Skip to content

Commit

Permalink
Add initial architecture rules (#5741)
Browse files Browse the repository at this point in the history
* Add initial architecture tests

* Address feedback

* Update version

* Allowlist new violations

* Update test code to log error message
  • Loading branch information
zoewangg authored Dec 11, 2024
1 parent d6296b2 commit 2bddb0c
Show file tree
Hide file tree
Showing 63 changed files with 1,548 additions and 106 deletions.
3 changes: 2 additions & 1 deletion .brazil.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@
"third-party-slf4j-api": { "skipImport": true },
"v2-migration-tests": {"skipImport": true},
"crt-unavailable-tests": { "skipImport": true },
"bundle-shading-tests": { "skipImport": true }
"bundle-shading-tests": { "skipImport": true },
"architecture-tests": {"skipImport": true}
},

"dependencies": {
Expand Down
12 changes: 12 additions & 0 deletions bom-internal/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,18 @@
<version>${reactive-streams.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.tngtech.archunit</groupId>
<artifactId>archunit</artifactId>
<version>${archunit.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.tngtech.archunit</groupId>
<artifactId>archunit-junit5</artifactId>
<version>${archunit.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.jimfs</groupId>
<artifactId>jimfs</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion buildspecs/release-javadoc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ phases:
commands:
- python ./scripts/doc_crosslinks/generate_cross_link_data.py --apiDefinitionsBasePath ./services/ --apiDefinitionsRelativeFilePath src/main/resources/codegen-resources/service-2.json --templateFilePath ./scripts/doc_crosslinks/crosslink_redirect.html --outputFilePath ./scripts/crosslink_redirect.html
- mvn install -P quick -T1C
- mvn clean install javadoc:aggregate -B -Ppublic-javadoc -Dcheckstyle.skip -Dspotbugs.skip -DskipTests -Ddoclint=none -pl '!:protocol-tests,!:protocol-tests-core,!:codegen-generated-classes-test,!:sdk-benchmarks,!:s3-benchmarks,!:module-path-tests,!:test-utils,!:http-client-tests,!:tests-coverage-reporting,!:sdk-native-image-test,!:ruleset-testing-core,!:old-client-version-compatibility-test,!:crt-unavailable-tests,!:bundle-shading-tests,!:v2-migration,!:v2-migration-tests'
- mvn clean install javadoc:aggregate -B -Ppublic-javadoc -Dcheckstyle.skip -Dspotbugs.skip -DskipTests -Ddoclint=none -pl '!:protocol-tests,!:protocol-tests-core,!:codegen-generated-classes-test,!:sdk-benchmarks,!:s3-benchmarks,!:module-path-tests,!:test-utils,!:http-client-tests,!:tests-coverage-reporting,!:sdk-native-image-test,!:ruleset-testing-core,!:old-client-version-compatibility-test,!:crt-unavailable-tests,!:bundle-shading-tests,!:v2-migration,!:v2-migration-tests,!:architecture-tests'
- RELEASE_VERSION=`mvn -q -Dexec.executable=echo -Dexec.args='${project.version}' --non-recursive exec:exec`
-
- aws s3 sync target/site/apidocs/ $DOC_PATH/$RELEASE_VERSION/ --acl="public-read"
Expand Down
2 changes: 1 addition & 1 deletion buildspecs/release-to-maven.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ phases:
awk 'BEGIN { var=ENVIRON["SDK_SIGNING_GPG_KEYNAME"] } { gsub("\\$SDK_SIGNING_GPG_KEYNAME", var, $0); print }' > \
$SETTINGS_XML
mvn clean deploy -B -s $SETTINGS_XML -Ppublishing -DperformRelease -Dspotbugs.skip -DskipTests -Dcheckstyle.skip -Djapicmp.skip -Ddoclint=none -pl !:protocol-tests,!:protocol-tests-core,!:codegen-generated-classes-test,!:sdk-benchmarks,!:module-path-tests,!:tests-coverage-reporting,!:stability-tests,!:sdk-native-image-test,!:auth-tests,!:s3-benchmarks,!:region-testing,!:old-client-version-compatibility-test,!:crt-unavailable-tests,!:bundle-shading-tests,!:v2-migration-tests -DautoReleaseAfterClose=true -DstagingProgressTimeoutMinutes=30 -Dmaven.wagon.httpconnectionManager.ttlSeconds=120 -Dmaven.wagon.http.retryHandler.requestSentEnabled=true
mvn clean deploy -B -s $SETTINGS_XML -Ppublishing -DperformRelease -Dspotbugs.skip -DskipTests -Dcheckstyle.skip -Djapicmp.skip -Ddoclint=none -pl !:protocol-tests,!:protocol-tests-core,!:codegen-generated-classes-test,!:sdk-benchmarks,!:module-path-tests,!:tests-coverage-reporting,!:stability-tests,!:sdk-native-image-test,!:auth-tests,!:s3-benchmarks,!:region-testing,!:old-client-version-compatibility-test,!:crt-unavailable-tests,!:bundle-shading-tests,!:v2-migration-tests,!:architecture-tests -DautoReleaseAfterClose=true -DstagingProgressTimeoutMinutes=30 -Dmaven.wagon.httpconnectionManager.ttlSeconds=120 -Dmaven.wagon.http.retryHandler.requestSentEnabled=true
else
echo "This version was already released."
fi
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@

import java.util.Optional;
import java.util.function.Supplier;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.core.SdkSystemSetting;
import software.amazon.awssdk.profiles.ProfileFile;
import software.amazon.awssdk.profiles.ProfileProperty;
import software.amazon.awssdk.utils.OptionalUtils;

@SdkInternalApi
@SdkProtectedApi
public final class AccountIdEndpointModeResolver {

private static final AccountIdEndpointMode SDK_DEFAULT_MODE = AccountIdEndpointMode.PREFERRED;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.awscore.AwsExecutionAttribute;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.core.interceptor.Context;
Expand All @@ -35,7 +35,7 @@
* This interceptor will monitor for {@link UnknownHostException}s and provide the customer with additional information they can
* use to debug or fix the problem.
*/
@SdkInternalApi
@SdkProtectedApi
public final class HelpfulUnknownHostExceptionInterceptor implements ExecutionInterceptor {
@Override
public Throwable modifyException(Context.FailedExecution context, ExecutionAttributes executionAttributes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
package software.amazon.awssdk.awscore.interceptor;

import java.util.Optional;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.awscore.internal.interceptor.TracingSystemSetting;
import software.amazon.awssdk.core.interceptor.Context;
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
Expand All @@ -28,7 +28,7 @@
* The {@code TraceIdExecutionInterceptor} copies the trace details to the {@link #TRACE_ID_HEADER} header, assuming we seem to
* be running in a lambda environment.
*/
@SdkInternalApi
@SdkProtectedApi
public class TraceIdExecutionInterceptor implements ExecutionInterceptor {
private static final String TRACE_ID_HEADER = "X-Amzn-Trace-Id";
private static final String LAMBDA_FUNCTION_NAME_ENVIRONMENT_VARIABLE = "AWS_LAMBDA_FUNCTION_NAME";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,20 @@

package software.amazon.awssdk.awscore.internal;

import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.core.SdkProtocolMetadata;
import software.amazon.awssdk.utils.ToString;
import software.amazon.awssdk.utils.builder.CopyableBuilder;
import software.amazon.awssdk.utils.builder.ToCopyableBuilder;

/**
* Contains AWS-specific protocol metadata. Implementation of {@link SdkProtocolMetadata}.
*
* <p>
* Implementation notes: this class should've been outside internal package,
* but we can't fix it due to backwards compatibility reasons.
*/
@SdkInternalApi
@SdkProtectedApi
public final class AwsProtocolMetadata implements SdkProtocolMetadata, ToCopyableBuilder<AwsProtocolMetadata.Builder,
AwsProtocolMetadata> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,13 @@

package software.amazon.awssdk.awscore.internal;

import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.annotations.SdkProtectedApi;

@SdkInternalApi
/**
* Implementation notes: this class should've been outside internal package,
* but we can't fix it due to backwards compatibility reasons.
*/
@SdkProtectedApi
public enum AwsServiceProtocol {
EC2("ec2"),
AWS_JSON("json"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
package software.amazon.awssdk.awscore.internal.defaultsmode;

import java.util.Optional;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.awscore.defaultsmode.DefaultsMode;
import software.amazon.awssdk.core.SdkSystemSetting;
import software.amazon.awssdk.regions.Region;
Expand All @@ -29,8 +29,12 @@
/**
* This class attempts to discover the appropriate {@link DefaultsMode} by inspecting the environment. It falls
* back to the {@link DefaultsMode#STANDARD} mode if the target mode cannot be determined.
*
* <p>
* Implementation notes: this class should've been outside internal package,
* but we can't fix it due to backwards compatibility reasons.
*/
@SdkInternalApi
@SdkProtectedApi
public class AutoDefaultsModeDiscovery {
private static final String EC2_METADATA_REGION_PATH = "/latest/meta-data/placement/region";
private static final DefaultsMode FALLBACK_DEFAULTS_MODE = DefaultsMode.STANDARD;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@
package software.amazon.awssdk.awscore.internal.useragent;

import java.util.Optional;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.awscore.endpoints.AccountIdEndpointMode;
import software.amazon.awssdk.core.useragent.BusinessMetricFeatureId;

@SdkInternalApi
/**
* Implementation notes: this class should've been outside internal package,
* but we can't fix it due to backwards compatibility reasons.
*/
@SdkProtectedApi
public final class BusinessMetricsUtils {
private BusinessMetricsUtils() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@
package software.amazon.awssdk.protocols.json;

import java.io.IOException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.http.SdkHttpFullResponse;
import software.amazon.awssdk.protocols.jsoncore.JsonNode;
import software.amazon.awssdk.protocols.jsoncore.JsonNodeParser;
import software.amazon.awssdk.thirdparty.jackson.core.JsonFactory;
import software.amazon.awssdk.utils.IoUtils;
import software.amazon.awssdk.utils.Logger;

/**
* Simple struct like class to hold both the raw json string content and it's parsed JsonNode
Expand All @@ -32,7 +31,7 @@
//TODO Do we need this? It isn't well encapsulated because of storing non-copied arrays.
public class JsonContent {

private static final Logger LOG = LoggerFactory.getLogger(JsonContent.class);
private static final Logger LOG = Logger.loggerFor(JsonContent.class);

private final byte[] rawContent;
private final JsonNode jsonNode;
Expand All @@ -58,7 +57,7 @@ public static JsonContent createJsonContent(SdkHttpFullResponse httpResponse,
try {
return IoUtils.toByteArray(c);
} catch (IOException e) {
LOG.debug("Unable to read HTTP response content", e);
LOG.debug(() -> "Unable to read HTTP response content", e);
}
return null;
}).orElse(null);
Expand All @@ -73,7 +72,7 @@ private static JsonNode parseJsonContent(byte[] rawJsonContent, JsonFactory json
JsonNodeParser parser = JsonNodeParser.builder().jsonFactory(jsonFactory).build();
return parser.parse(rawJsonContent);
} catch (Exception e) {
LOG.debug("Unable to parse HTTP response content", e);
LOG.debug(() -> "Unable to parse HTTP response content", e);
return JsonNode.emptyObjectNode();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

import java.util.Optional;
import java.util.function.Function;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.Response;
import software.amazon.awssdk.core.SdkPojo;
Expand All @@ -32,6 +30,7 @@
import software.amazon.awssdk.http.SdkHttpFullResponse;
import software.amazon.awssdk.protocols.query.unmarshall.XmlElement;
import software.amazon.awssdk.utils.IoUtils;
import software.amazon.awssdk.utils.Logger;

/**
* Unmarshalls an HTTP response into either a successful response POJO, or into a (possibly modeled) exception based
Expand All @@ -42,7 +41,7 @@
*/
@SdkInternalApi
public class AwsXmlPredicatedResponseHandler<OutputT> implements HttpResponseHandler<Response<OutputT>> {
private static final Logger log = LoggerFactory.getLogger(AwsXmlPredicatedResponseHandler.class);
private static final Logger log = Logger.loggerFor(AwsXmlPredicatedResponseHandler.class);

private final Function<SdkHttpFullResponse, SdkPojo> pojoSupplier;
private final Function<AwsXmlUnmarshallingContext, OutputT> successResponseTransformer;
Expand Down Expand Up @@ -173,7 +172,7 @@ private void closeInputStreamIfNeeded(SdkHttpFullResponse httpResponse,
if (didRequestFail || !needsConnectionLeftOpen) {
Optional.ofNullable(httpResponse)
.flatMap(SdkHttpFullResponse::content) // If no content, no need to close
.ifPresent(s -> IoUtils.closeQuietly(s, log));
.ifPresent(s -> IoUtils.closeQuietlyV2(s, log));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@

/**
* Identify a {@link RetryStrategy} that has the capacity to work with sets of default retry predicates.
*
* <p>
* Implementation notes: this class should've been outside internal package,
* but we can't fix it due to backwards compatibility reasons.
*/
@SdkProtectedApi
public interface DefaultAwareRetryStrategy extends RetryStrategy {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@

/**
* The set of retry predicates that are by default added to a retry strategy.
*
* <p>
* Implementation notes: this class should've been outside internal package,
* but we can't fix it due to backwards compatibility reasons.
*/
@SdkProtectedApi
public interface RetryStrategyDefaults {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@

import java.net.URI;
import java.time.Instant;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.utils.builder.CopyableBuilder;
import software.amazon.awssdk.utils.builder.ToCopyableBuilder;

@SdkInternalApi
@SdkProtectedApi
public final class EndpointDiscoveryEndpoint implements
ToCopyableBuilder<EndpointDiscoveryEndpoint.Builder, EndpointDiscoveryEndpoint> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,13 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.utils.Logger;

@SdkProtectedApi
public class EndpointDiscoveryProviderChain implements EndpointDiscoveryProvider {

private static final Logger log = LoggerFactory.getLogger(EndpointDiscoveryProviderChain.class);
private static final Logger log = Logger.loggerFor(EndpointDiscoveryProviderChain.class);

private final List<EndpointDiscoveryProvider> providers;

Expand All @@ -40,7 +39,8 @@ public boolean resolveEndpointDiscovery() {
try {
return provider.resolveEndpointDiscovery();
} catch (Exception e) {
log.debug("Unable to load endpoint discovery from {}:{}", provider.toString(), e.getMessage());
log.debug(() -> String.format("Unable to load endpoint discovery from %s:%s", provider.toString(),
e.getMessage()));
}
}
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,21 @@

package software.amazon.awssdk.core.internal.compression;

import static software.amazon.awssdk.utils.IoUtils.closeQuietly;
import static software.amazon.awssdk.utils.IoUtils.closeQuietlyV2;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.zip.GZIPOutputStream;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.SdkBytes;
import software.amazon.awssdk.utils.Logger;

@SdkInternalApi
public final class GzipCompressor implements Compressor {

private static final String COMPRESSOR_TYPE = "gzip";
private static final Logger log = LoggerFactory.getLogger(GzipCompressor.class);
private static final Logger log = Logger.loggerFor(GzipCompressor.class);

@Override
public String compressorType() {
Expand All @@ -49,7 +48,7 @@ public SdkBytes compress(SdkBytes content) {
} catch (IOException e) {
throw new UncheckedIOException(e);
} finally {
closeQuietly(gzipOutputStream, log);
closeQuietlyV2(gzipOutputStream, log);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

import java.io.IOException;
import java.util.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.Response;
import software.amazon.awssdk.core.exception.RetryableException;
Expand All @@ -30,6 +28,7 @@
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
import software.amazon.awssdk.http.SdkHttpFullResponse;
import software.amazon.awssdk.utils.IoUtils;
import software.amazon.awssdk.utils.Logger;

/**
* Unmarshalls an HTTP response into either a successful response POJO, or into a (possibly modeled) exception based
Expand All @@ -40,7 +39,7 @@
*/
@SdkInternalApi
public class CombinedResponseHandler<OutputT> implements HttpResponseHandler<Response<OutputT>> {
private static final Logger log = LoggerFactory.getLogger(CombinedResponseHandler.class);
private static final Logger log = Logger.loggerFor(CombinedResponseHandler.class);

private final HttpResponseHandler<OutputT> successResponseHandler;
private final HttpResponseHandler<? extends SdkException> errorResponseHandler;
Expand Down Expand Up @@ -143,7 +142,7 @@ private void closeInputStreamIfNeeded(SdkHttpFullResponse httpResponse,
if (didRequestFail || !successResponseHandler.needsConnectionLeftOpen()) {
Optional.ofNullable(httpResponse)
.flatMap(SdkHttpFullResponse::content) // If no content, no need to close
.ifPresent(s -> IoUtils.closeQuietly(s, log));
.ifPresent(s -> IoUtils.closeQuietlyV2(s, log));
}
}
}
Loading

0 comments on commit 2bddb0c

Please sign in to comment.