diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a2268212dde1..1f040805e1bd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -110,7 +110,7 @@ Please keep the following in mind when considering a code contribution: ### Your First Code Change For detailed information on getting started building and making code changes to -the SDK, refer to our [Working on the SDK][working-on-the-sdk] wiki page. +the SDK, refer to our [Working on the SDK](./docs/GettingStarted.md) doc ### Pull Request Readiness Before submitting your pull request, refer to the pull request readiness @@ -153,7 +153,7 @@ asked to [squash][git-rewriting-history] them into a single commit before it is merged in. ## Additional Resources -We maintain a [wiki][wiki] where information like design decisions, internal +We maintain [docs](docs/README.md) where information like design decisions, internal architecture, and style conventions are documented that you may find helpful when contributing to the SDK. @@ -166,8 +166,6 @@ when contributing to the SDK. [issues]: https://github.com/aws/aws-sdk-java-v2/issues [pull-requests]: https://github.com/aws/aws-sdk-java-v2/pulls [cla]: https://github.com/aws/aws-cla -[wiki]: https://github.com/aws/aws-sdk-java-v2/wiki -[working-on-the-sdk]: https://github.com/aws/aws-sdk-java-v2/wiki/Working-on-the-SDK [label-bug]: https://github.com/aws/aws-sdk-java-v2/labels/Bug [label-doc-issue]: https://github.com/aws/aws-sdk-java-v2/labels/Documentation%20Issue [label-feature-request]: https://github.com/aws/aws-sdk-java-v2/labels/Feature%20Request diff --git a/docs/ClientConfiguraion.md b/docs/ClientConfiguraion.md new file mode 100644 index 000000000000..da2dd3010b07 --- /dev/null +++ b/docs/ClientConfiguraion.md @@ -0,0 +1,107 @@ +# Client Configuration + +This page describes the structure and conventions used for client configuration objects. Client configuration objects are any objects used to configure an AWS client builder. + +#### Example + +This section walks through an example configuration class structure and describes each of its components at a high level. + +```Java +/** + * Configuration Description // (1) + */ +@Immutable +@ThreadSafe // (2) +public final class SdkConfiguration // (3) + implements ToCopyableBuilder { // (4) + private final String option; // (5) + + /** + * @see #builder() // (6) + */ + private SdkClientConfiguration(DefaultSdkConfigurationBuilder builder) { + this.option = builder.option; + } + + public static Builder builder() { + return new DefaultSdkConfigurationBuilder(); + } + + /** + * @see #Builder#option(String) // (7) + */ + public String option() { + return this.option; + } + + @Override + public ClientHttpConfiguration.Builder toBuilder() { + return builder().option(option); + } + + @NotThreadSafe + public interface Builder extends CopyableBuilder { // (8) + /** + * Configuration Option Description // (9) + */ + Builder option(String option); + } + + private static final class DefaultSdkConfigurationBuilder implements Builder { // (10) + private String option; + + @Override + public Builder option(String option) { // (11) + this.option = option; + return this; + } + + public void setOption(String option) { // (12) + this.option = option; + } + + @Override + public SdkConfiguration build() { + return new SdkConfiguration(this); + } + } +} +``` + +1. A detailed description should be given of what types of options the user might find in this object. +2. Configuration objects should be `@Immutable`, and therefore `@ThreadSafe`. +3. Configuration classes should be defined as `final` to prevent extension. +4. Configuration classes should extend `ToCopyableBuilder` to ensure they can be converted back to a builder object. +5. All configuration fields should be defined as `private final` to prevent reassignment. +6. Configuration constructors should be `private` to enforce creation by the `Builder` and refer curious eyes to the `builder()` method for creating the object. +7. One "get" method should be created for each configuration field. This method's name should exactly match the name of the field it is retrieving. +8. Each builder should have its own interface. This allows hiding certain public methods from auto-complete (see below). +9. A detailed description of each option should be given, including what it does, and **why** a user could want to change its default value. +10. A `private static final` implementation of the `Builder` interface should be created that is not exposed outside of the scope of the configuration class. +11. One "set" method should be created for each option to mutate the value in this builder. This method's name should exactly match the name of the field it is setting. +12. Each option should have a bean-style setter to allow configuring the object reflectively using `Inspector`-based frameworks, like spring XML. + +#### Configuration Fields + +This section details the semantics of configuration fields. + +1. Configuration fields must be **immutable**. + 1. Fields must be marked as `final`. + 2. Mutable types, like `List` and `Set` should be wrapped in a type that prevents their modification (eg. `Collections.unmodifiableList`) when referenced through the "get" method. + 3. Mutable types, like `List` and `Set` should be copied in the "set" method to prevent their modification by mutating the original object. +2. Configuration fields must be **reference types**. Primitive types like `boolean` or `int` should not be used because they can not convey the absence of configuration. +3. Configuration field names should **not start with a verb** (eg. `config.enableRedirect()` should instead be `config.redirectEnabled()`). This is to avoid confusing their "get" methods for a mutating action. + +Special notes for collection types, like `List`, `Set`, and `Map`: + +1. Collection type field names must be plural. +2. Collection types should be accompanied by an `addX` method on the builder that permits adding one item to the collection. This `addX` method should be singular. + +```Java +public interface Builder { + Builder options(List options); + Builder addOption(String option); + + Builder headers(Map headers); + Builder addHeader(String key, String value); +} diff --git a/docs/FavorStaticFactoryMethods.md b/docs/FavorStaticFactoryMethods.md new file mode 100644 index 000000000000..b36b6b8ffb40 --- /dev/null +++ b/docs/FavorStaticFactoryMethods.md @@ -0,0 +1,58 @@ +## Favor Static Factory Methods Over Constructors + +This page describes the structures and conventions used to initialize a class. + +### Static Factory Methods vs. Constructors +Static factory methods are preferable than constructors for the following reasons: +- Static factory methods provide meaningful names compared with constructors, which improves the readability of the codes by describing the objects being returned. +```java +// With static factory method, giving a hint that a foobar provider with default settings is being created. +FoobarProvider defaultProvider = FoobarProvider.defaultFoobarProvider(); + +// With constructor +FoobarProvider defaultProvider = new FoobarProvider(); +``` +- They are useful when work with immutable classes as we can reuse the same object instead of creating new object every time when it is invoked. +- Static factory methods can return any subtype of that class and this gives us flexibility to write a separate factory class to return different subclasses if needed. + +There are a few disadvantages of static factory methods: +- It might not be straightforward for the users to use static factory method to create new instances compared with constructors, but this can be improved by providing a set of common method names such as `create()` and smart IDEs such as IntelliJ and Eclipse can also give hints and help auto-completing. +- Classes without public or protected constructors cannot be subclassed, but this could encourage us to use composition instead of inheritance. + +In general, we should favor static factory methods over constructors. + +### Example +```java +public class DefaultCredentialsProvider implements AwsCredentialsProvider, SdkAutoCloseable { + + private static final DefaultCredentialsProvider DEFAULT_CREDENTIALS_PROVIDER = new DefaultCredentialsProvider(builder()); + + private DefaultCredentialsProvider(Builder builder) { + this.providerChain = createChain(builder); + } + + public static DefaultCredentialsProvider create() { + return DEFAULT_CREDENTIALS_PROVIDER; + } + + public static Builder builder() { + return new Builder(); + } + + public static final class Builder { + // ... + } + // ... +} + + +// There are two ways to create new instance +DefaultCredentialsProvider defaultCredentialsProvider1 = DefaultCredentialsProvider.create(); +DefaultCredentialsProvider defaultCredentialsProvider2 = DefaultCredentialsProvider.builder().build; +``` +### Naming Conventions +The naming conventions for the static factory methods: +- `create()`, `create(params)` when creating a new instance +eg: `DynamoDBClient.create()` +- `defaultXXX()` when returning an instance with default settings. +eg: `BackoffStrategy.defaultStrategy()` diff --git a/docs/GettingStarted.md b/docs/GettingStarted.md new file mode 100644 index 000000000000..d08c3889a90c --- /dev/null +++ b/docs/GettingStarted.md @@ -0,0 +1,94 @@ +# Working on the SDK + +# Things to Know +* The SDK is built on Java 8 +* [Maven][maven] is used as the build and dependency management system +* The majority of the service client code is auto-generated using the [code + generator][codegen] + +# Development Environment Setup Tips +If you use IntelliJ IDEA, we include some helpful config files that will make your development experience smoother: +- [intellij-codestyle.xml](https://github.com/aws/aws-sdk-java-v2/blob/master/build-tools/src/main/resources/software/amazon/awssdk/intellij-codestyle.xml) + + This will help ensure your code follows our code style guidelines. + +- [intellij-copyright-profile.xml](https://github.com/aws/aws-sdk-java-v2/blob/master/build-tools/src/main/resources/software/amazon/awssdk/intellij-copyright-profile.xml) + + This automatically inserts the license header to the top of source files that you create. + +If you have Checkstyle integrated with your IDE, we also recommend configuring it with our [Checkstyle config](https://github.com/aws/aws-sdk-java-v2/blob/master/build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml) so you can see any violations in line with the code. + +# Getting Around +At a high level, the SDK is comprised of two parts: the core runtime, and the +service clients, including their higher level abstractions. + +*TODO* + +# Building +Since the SDK is a normal Maven project, the usual `mvn package` and `mvn +install` commands are all you need to build the SDK. + +One important thing to note is that if you're working on the [code +generator][codegen], be sure to do a `mvn install` rather than a phase that +comes earlier such as `compile` or `test` so that the build uses the uses the +correct code generator JAR (i.e. the one including your changes). When in +doubt, just use `mvn package`. + +## Disabling Checkstyle/FindBugs +Normally Checkstyle and FindBugs scans run as part of the build process. +However, this slows down the build significantly so if you need to be able to +iterate quickly locally, you can turn either of them off by setting the +appropriate properties: + +```sh +# skips both Checkstyle and FindBugs +$ mvn install -Dfindbugs.skip=true -Dcheckstyle.skip=true +``` + +# Testing +## Unit Tests +As described in the project structure, tests are split between unit and +integration tests. During the normal `test` lifecycle phase, only the unit +tests are run. + +```sh +# runs the unit tests +mvn install +``` + +## Integration Tests +__Before running the integration tests, be aware that they require active AWS +IAM credentials, and because they will make actual calls to AWS, will incur a +cost to the owner of the account.__ + +If you're writing an integration test, try to see if it's possible to write it +as a set of unit tests with mocked responses instead. + +### Test Credentials + +As mentioned above, you will need to have active IAM credentials that the tests +will use to authenticate with AWS, and it will need to have an attached IAM +policy that is allowed to perform the actions the tests will be running. + +All integration tests are written to locate these credentials in +`$HOME/.aws/awsTestAccount.properties`: + +``` +$ cat $HOME/.aws/awsTestAccount.properties + +accessKey = ... +secretKey = ... +``` + +### Running the Integration Tests + +In order to run the integration tests along with the unit tests, you must +activate the `integration-tests` profile + +```sh +# runs both unit and integration tests +mvn install -P integration-tests +``` + +[maven]: https://maven.apache.org/ +[codegen]: https://github.com/aws/aws-sdk-java-v2/blob/master/codegen diff --git a/docs/NamingConventions.md b/docs/NamingConventions.md new file mode 100644 index 000000000000..be2a760f4979 --- /dev/null +++ b/docs/NamingConventions.md @@ -0,0 +1,15 @@ +## Naming Conventions + +This page describes the naming conventions, nouns and common terms + +- Abbreviations must follow the same conventions as any other word (eg. use `DynamoDbClient`, not `DynamoDBClient`) + +- Use Singular Enum Name + + For enum classes or "pseudo-enums" classes(classes with public static fields), we should use singular name. (eg. use `SdkSystemSetting`, not `SdkSystemSettings`) + +- Use of `Provider`, `Supplier` and `Factory` in the class name. + - For general supplier classes (loading/creating resources of the same kind), prefer `Provide` unless the class extends from Java `Supplier` class. (eg. `AwsCredentialsProvider`) + - For factories classes (creating resources of same or different kinds), prefer `Factory`. (eg. `AwsJsonProtocolFactory`) + + diff --git a/docs/README.md b/docs/README.md new file mode 100644 index 000000000000..22a4d0d271bf --- /dev/null +++ b/docs/README.md @@ -0,0 +1,13 @@ +# AWS SDK for Java 2.0 Docs + +## New Contributions +[GettingStarted](GettingStarted.md) + +## Style Conventions +- [Client Configuration](ClientConfiguraion.md) +- [Favor Static Factory Methods Over Constructors](FavorStaticFactoryMethods.md) +- [Use of Optional](UseOfOptional.md) +- [Naming Conventions](NamingConventions.md) + +## Key Design Decisions +- [Use of CompletableFuture](UseOfCompletableFuture.md) diff --git a/docs/UseOfCompletableFuture.md b/docs/UseOfCompletableFuture.md new file mode 100644 index 000000000000..02fd0734d2c9 --- /dev/null +++ b/docs/UseOfCompletableFuture.md @@ -0,0 +1,18 @@ +## Use of CompletableFuture + +Operations on the asynchronous clients return [`CompleteableFuture`][1] where `T` is the response type for the operation. This is somewhat curious in that [`CompleteableFuture`][1] is a concrete implementation rather than an interface. The alternative to returning a [`CompleteableFuture`][1] would be to return a [`CompletionStage`][2], an interface intended to allow chaining of asynchronous operations. + +The key advantage of [`CompleteableFuture`][1] is that it implements both the [`CompletionStage`][2] and [`Future`][3] interfaces - giving users of the SDK maximum flexibilty when it comes to handling responses from their asynchronous calls. + +Currently [`CompleteableFuture`][1] is the only implementation of [`CompletionStage`][2] that ships with the JDK. Whilst it's possible that future implementations will be added [`CompletionStage`][2] will always be tied to [`CompleteableFuture`][1] via the [`#toCompletableFuture`][4] method. Additionally, [`CompleteableFuture`][1] is not a `final` class and thus could be extended if there was a requirement to do so. + +One of the perceived risks with exposing [`CompleteableFuture`][1] rather than [`CompletionStage`][2] is that a user of the SDK may spuriously call [`#complete`](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#complete-T-) or [`#completeExceptionally`](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#completeExceptionally-java.lang.Throwable-) which could cause unintended consequences in their application and the SDK itself. However this risk is also present on the [`CompletionStage`][2] via the [`#toCompletableFuture`][4] method. + +If the [`CompletionStage`][2] interface did not have a [`#toCompletableFuture`][4] method the argument for using it would be a lot stronger, however as it stands the interface and its implementation are tightly coupled. + +Using [`CompleteableFuture`][1] gives users the best bang for their buck without much additional risk. + +[1]: https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html +[2]: https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletionStage.html +[3]: https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Future.html +[4]: https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletionStage.html#toCompletableFuture-- \ No newline at end of file diff --git a/docs/UseOfOptional.md b/docs/UseOfOptional.md new file mode 100644 index 000000000000..b0851f2aad59 --- /dev/null +++ b/docs/UseOfOptional.md @@ -0,0 +1,13 @@ +## Use of Optional + +This page describes general guidelines of how we use `Optional`. + +- Do not declare any instance variable of type Optional in public API. +- Do not use Optional in parameters in public API +- Do not use Optional in Builder classes +- Do not use Optional in POJOs. +- Use Optional for getters that access the field that's we know always going to be optional. +- Use Optional as a return type for any methods that have a result that's we know always going to be optional. + +References: +- http://blog.joda.org/2015/08/java-se-8-optional-pragmatic-approach.html