From 67e3f0826bc9bad0c6fca9464bb0d289bf43df31 Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Wed, 6 Apr 2016 18:18:48 -0700 Subject: [PATCH 1/6] Documentation on how to add a new client library --- ADDING_NEW_CLIENTS.md | 93 +++++++++++++++++++++++++++++++++++++++++++ CONTRIBUTING.md | 9 +++-- 2 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 ADDING_NEW_CLIENTS.md diff --git a/ADDING_NEW_CLIENTS.md b/ADDING_NEW_CLIENTS.md new file mode 100644 index 000000000000..0537ca2e7911 --- /dev/null +++ b/ADDING_NEW_CLIENTS.md @@ -0,0 +1,93 @@ +## How to Add a New Client Library + +### Overview + +This document outlines how to add a new client library to `gcloud-java`. New client libraries should be submodules located in a folder within the main repository, built using Maven. A client library should contain the following items: + +* An API layer, with which users will interact. This includes model objects and a service class. +* An SPI layer, which translates gcloud-java API calls into RPCs using an autogenerated client library. In almost all use cases, the user will not directly interact with this code. Separating this code from the API layer allows the API layer to remain stable despite changes to the autogenerated libraries used. +* A test helper class, which allows users to easily interact with a local emulator (if possible). If there is no emulator available and the service is too complex to create a mock, then this helper should facilitate separation of test data from other user data and enable easy cleanup. +* Tests, including unit tests and integration tests. +* A command line example application. +* Documentation, which is comprised of READMEs, Javadoc, and code snippets. + +### Components of a new client library + +#### API layer + +Before starting work on the API layer, write a design document and provide sample API code, either in the design document or as a pull request tagged with the "don't merge" label. As part of the design process, be sure to examine the Google Cloud service API and any implementations provided in other gcloud-* language libraries. Solicit feedback from other contributors to the repository. + +When possible, make classes immutable, providing builders when necessary. Commonly-used classes that contain metadata should also contain a subclass that provides functions on that metadata. For example, see `BlobInfo` (the metadata class) and `Blob` (the functional class). The builders for both objects should implement a common interface or abstract class, and the subclass should delegate to the metadata class builder. Make model object classes serializable. Also, make classes final when possible, except when the class contains functionality that cannot be fully tested by users without mocking the object. + +Notes/reminders: +* API layer classes should be located in the package `com.google.cloud.servicename`, where "servicename" corresponds to the name of the Cloud service. +* Override the `ServiceOptions.defaultRetryParams()` method in your service's options class to align with the Service Level Agreement (SLA) given by the underlying service. See #857 and #860 for context. +* See conventions about overriding the `equals` and `hashCode` methods in the discussion of #892. +* While not all fields for model objects need to be exposed to the user, `gcloud-java` clients should get and set all relevant fields when making RPC calls to the Cloud service's API. For example, since the `parent` field of Cloud Resource Manager Project objects is in alpha (at the time this is written) and not available to most users, `gcloud-java-resourcemanager` gets and sets the parent when interacting with the Cloud Resource Manager, but does not expose the parent to users. This avoids the user inadvertently attempting to unset the parent when updating a project. +* Be aware of differences in "update" behavior and name update/replace methods accordingly in your API. See #321 for context. + +#### SPI layer + +The SPI layer classes should be located in the package `com.google.cloud.servicename.spi`. In most cases, the SPI layer should contain at least three classes: +* An RPC factory interface (allows for the implementation to be loaded via the `java.util.ServiceLoader`). +* An RPC interface that contains all RPC methods. +* A default RPC implementation. + +#### Test helpers + +Test helper classes should be located in the package `com.google.cloud.servicename.testing`. The naming convention for test helpers is `[Local|Remote][Service]Helper.java`. For example, the local test helper for `gcloud-java-datastore` is named `LocalDatastoreHelper` and the remote test helper for `gcloud-java-storage` is named `RemoteStorageHelper`. All test helpers should contain public `create` and `options` methods, and local helpers should contain `start` and `stop` methods. See existing test helpers for information on what each of those methods should do. + +There are three types of test helpers: +* When a local emulator is already available, your test helper should launch that emulator and return service options to connect to that local emulator. This enables both users and our own library to run unit tests easily. An example of this type of helper is `LocalDatastoreHelper`. Google Cloud Datastore provides a script that launches a local datastore, so `LocalDatastoreHelper` launches that script in a separate process when the user calls `start()`. + +* When there is no local emulator but the service is simple enough to write an emulator, you should do so. The emulator should listen to a port for requests, process those requests, and send responses back, being as true to the actual service as possible. Be sure to document differences between your emulator and the actual service. Examples of this type of test helper are `LocalResourceManagerHelper` and `LocalDnsHelper`. + +* When there is no local emulator and the service is too complex to write a solid emulator, the test helper should contain methods to get options and separate test data from other user data. `RemoteStorageHelper` is an example of this type of test helper, since there is no local emulator for Google Cloud Storage (at the time that this is written) and because the Google Cloud Storage API is complex. `RemoteStorageHelper` has methods to: + * Get service options settings. + * Create a test bucket with a sufficiently obscure name (to separate the bucket from any of the users other data). + * Clear up data left over from tests in that test bucket. + + +#### Tests + +API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. + +Simple service-related tests should be added to [GoogleCloudPlatform/gcloud-java-examples](https://github.com/GoogleCloudPlatform/gcloud-java-examples/tree/master/test-apps). To test releases and platform-specific bugs, it's valuable to deploy the apps in that repository on App Engine, Compute Engine, and from your own desktop. + +#### Example application + +The example application should be a simple command line interface for the service. It should use common library use patterns so that users see good examples of how to use `gcloud-java` when viewing the source code. Be sure to keep the examples up to date if/when there are updates that make the API cleaner and more concise. See examples of applications under the `gcloud-java-examples` folder. The example application should be in the package `com.google.cloud.examples.servicename`. + +#### Documentation + +* Include a summary of the service and code snippets on the main repository's README. These snippets should be simple and cover a few common usage patterns. The README snippets should also be added to `gcloud-java-examples` in the package `com.google.cloud.examples.servicename.snippets`. Placing snippet code in the repository ensures that the snippet code builds when Travis CI is run. For this purpose, README snippets and the snippet code in `gcloud-java-examples` should be kept in sync. As of yet, we do not have unit tests for snippets, so the snippets should be tested periodically, especially after any relevant library updates. +* Create a README in the client library's folder. This README should mimic the structure of other client libraries' READMEs. In particular, you should create a step-by-step "Getting Started" guide. See [`gcloud-java-datastore`'s README](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/README.md) for reference. All code in that step-by-step guide should also be included in the `gcloud-java-examples` snippets package. +* The API and test helper packages should have `package-info.java` files. These files should contain descriptions of the packages as well as simple example code and/or links to code snippets. +* Public methods, classes, and builders should contain meaningful Javadoc. Document both unchecked and checked exceptions. +* Update [`TESTING`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/TESTING.md) with how to run tests using the test helper. +* Update the [`gcloud-java-examples` README](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-examples/README.md) with instructions on how to run your example application. + +Notes/reminders: +* Clearly document which APIs must be enabled in the Developers Console's API Manager. +* Versioning in documentation is automatically updated by the script `utilities/update_docs_version.sh`. Be sure to examine that script to make sure any version-dependent documentation will be updated properly upon release. + +### Workflow + +New services should be created in a branch based on `master`. The branch name should include "alpha". For example, while developing `gcloud-java-pubsub`, all Pub/Sub related work should be done in `pubsub-alpha`. All code should be submitted through pull requests from a branch on a forked repository. Limiting pull request size is very helpful for reviewers. All code that is merged into the branch should be standalone and well-tested. Any todo comments in the code should have an associated Github issue number for tracking purposes. You should periodically pull updates from the master branch, especially if there are project-wide updates or if relevant changes have been made to the core utilities library, `gcloud-java-core`. + +Create at least two milestones (stable and future) for your service and an issue tag with the service name. Create issues for any to-do items and tag them appropriately. This keeps an up-to-date short-term to-do list and also allows for longer term roadmaps. + +Be sure you've configured the base folder's `pom.xml` correctly. + * Add your module to the base directory's `pom.xml` file under the list of modules. + * Add your module to the javadoc packaging settings. See PR #802 for an example. + * Add your example to the assembler plugin. PR #839 includes examples of using appassembler. + +When your client library is complete, contact the service owners to get a review. The primary purpose of this review is to make sure that the gcloud-java client interacts with the Cloud service properly. Present the reviewers with a link to the Github repository, as well as your (updated) design document that details the API. + +### Closing remarks + +* Efforts should be made to maintain the current style of the repository and a consistent style between gcloud-java client libraries. + * We anticipate that people will often use multiple `gcloud-java` clients, so we don't want differences in conventions from library to library. Look at existing `gcloud-java` clients to see coding and naming conventions. + * Codacy is configured to report on pull requests about style issues. Whenever possible, those comments should be addressed. Coders and reviewers should also run a linter on pull requests, because the Codacy tool may not catch all style errors. +* When weighing which client libraries to add, consider that a hand-crafted `gcloud-java` client library is especially useful if it can abstract away and/or make java-idiomatic significant parts of a service's autogenerated API. + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3d93f2d032c7..0bfee7150bc8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,9 +2,9 @@ Contributing ============ 1. **Please sign one of the contributor license agreements below.** -1. Fork the repo, develop and test your code changes, add docs. -1. Make sure that your commit messages clearly describe the changes. -1. Send a pull request. +2. Fork the repo, develop and test your code changes, add docs. +3. Make sure that your commit messages clearly describe the changes. +4. Send a pull request. Here are some guidelines for hacking on gcloud-java. @@ -43,6 +43,9 @@ The feature must work fully on Java 7 and above. The feature must not add unnecessary dependencies (where "unnecessary" is of course subjective, but new dependencies should be discussed). +Adding a New Client Library +--------------------------- +See [ADDING_NEW_CLIENTS](./ADDING_NEW_CLIENTS.md) for guidelines on how to add a new client library to `gcloud-java`. Coding Style ------------ From d9c83f64f1c7d2d2d4fc2c1ca7524801a6aa9d6f Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Thu, 14 Apr 2016 14:37:39 -0700 Subject: [PATCH 2/6] Add more gcloud-java-core concepts + fixes for comments --- ADDING_NEW_CLIENTS.md | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/ADDING_NEW_CLIENTS.md b/ADDING_NEW_CLIENTS.md index 0537ca2e7911..e049e9753dae 100644 --- a/ADDING_NEW_CLIENTS.md +++ b/ADDING_NEW_CLIENTS.md @@ -15,16 +15,31 @@ This document outlines how to add a new client library to `gcloud-java`. New cl #### API layer -Before starting work on the API layer, write a design document and provide sample API code, either in the design document or as a pull request tagged with the "don't merge" label. As part of the design process, be sure to examine the Google Cloud service API and any implementations provided in other gcloud-* language libraries. Solicit feedback from other contributors to the repository. +Before starting work on the API layer, write a design document and provide sample API code, either in the design document or as a pull request tagged with the "don't merge" label. As part of the design process, be sure to examine the Google Cloud service API and any implementations provided in other gcloud-* language libraries. Solicit feedback from other contributors to the repository. Client libraries should be low-level while minimizing boilerplate code needed by API users. The library should also be flexible enough to be used by higher-level libraries. For example, Objectify should be able to use `gcloud-java-datastore`. -When possible, make classes immutable, providing builders when necessary. Commonly-used classes that contain metadata should also contain a subclass that provides functions on that metadata. For example, see `BlobInfo` (the metadata class) and `Blob` (the functional class). The builders for both objects should implement a common interface or abstract class, and the subclass should delegate to the metadata class builder. Make model object classes serializable. Also, make classes final when possible, except when the class contains functionality that cannot be fully tested by users without mocking the object. +The library should contain: +* A subclass of the [`ServiceOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceOptions.java) class. The `ServiceOptions` class contains important information for communicating with the Google Cloud service, such as the project ID, authentication, and retry handling. Subclasses should add/override relevant methods as necessary. Example: [`DatastoreOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java) +* An interface extending the [`Service`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/Service.java) interface (example: [`Datastore`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/Datastore.java)) and an implentation of that interface (example: [`DatastoreImpl`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java)). +* An interface extending the [`ServiceFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceFactory.java) interface. Example: [`DatastoreFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreFactory.java) +* A runtime exception class that extends [`BaseServiceException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java), which is used to wrap service-related exceptions. Example: [`DatastoreException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java) +* Classes representing model objects and request-related options. Commonly-used classes that contain metadata should also have a subclass that provides functions related to that metadata. For example, see [`BlobInfo`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-storage/src/main/java/com/google/cloud/storage/BlobInfo.java) (the metadata class) and [`Blob`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-storage/src/main/java/com/google/cloud/storage/Blob.java) (the functional class). The builders for both objects should implement a common interface or abstract class, and the functional subclass builder should delegate to the metadata class builder. + +In general, make classes immutable whenever possible, providing builders as necessary. Make model object classes `java.util.Serializable`. Also, make classes final when possible, except when the class contains functionality that cannot be fully tested by users without mocking the object. If a class cannot be made final, then `hashCode` or `equals` overrides should be made final if possible. + +`gcloud-java-core` provides functionality for code patterns used across `gcloud-java` libraries. The following are some important core concepts: +* Paging: Cloud services often expose page-based listing using page tokens. The [`Page`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/Page.java) interface should be used for page-based listing. A `Page` contains an iterator over results in that page, as well as methods to get the next page and all results in future pages. `Page` requires a `NextPageFetcher` implementation (see the `NextPageFetcher` interface in [`PageImpl`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/PageImpl.java)). This implementation should delegate constructing request options to the `nextRequestOptions` method in `PageImpl`. + +* Exception handling: we have the notion of retryable vs non-retryable exceptions and idempotent vs non-idempotent exceptions. These are encapsulated by `BaseServiceException`. Retryable error codes should be listed in the client's subclass of `BaseServiceException`, specifying whether exception is idempotent. The subclass should also provide methods to translate the exceptions given by the underlying autogeneraged client library. The [`ExceptionHandler`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ExceptionHandler.java) class intercepts and decides how to handle exceptions (based on the set of retryable error codes) when making RPC calls. + +* Batching: The [`BatchResult`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/dns-alpha-batch/gcloud-java-core/src/main/java/com/google/cloud/BatchResult.java) class provides a simple way for users to combine RPC calls for performance enhancement. Clients for services that provide batching should provide a batch class that contains methods similar to the methods in the `Service.java` subclass, but instead return a subclass of `BatchResult`. Also provide an SPI-layer class to collect batch requests and submit the batch. Notes/reminders: * API layer classes should be located in the package `com.google.cloud.servicename`, where "servicename" corresponds to the name of the Cloud service. * Override the `ServiceOptions.defaultRetryParams()` method in your service's options class to align with the Service Level Agreement (SLA) given by the underlying service. See #857 and #860 for context. * See conventions about overriding the `equals` and `hashCode` methods in the discussion of #892. -* While not all fields for model objects need to be exposed to the user, `gcloud-java` clients should get and set all relevant fields when making RPC calls to the Cloud service's API. For example, since the `parent` field of Cloud Resource Manager Project objects is in alpha (at the time this is written) and not available to most users, `gcloud-java-resourcemanager` gets and sets the parent when interacting with the Cloud Resource Manager, but does not expose the parent to users. This avoids the user inadvertently attempting to unset the parent when updating a project. +* While not all fields for model objects need to be exposed to the user, `gcloud-java` clients should get and set all relevant fields when making RPC calls to the Cloud service's API. For example, since the `parent` field of Cloud Resource Manager Project objects is in alpha (at the time this is written) and not available to most users, `gcloud-java-resourcemanager` gets and sets the parent when interacting with the Cloud Resource Manager, but does not expose the parent to users. As a result, the user won't accidentally unset the parent when updating a project. * Be aware of differences in "update" behavior and name update/replace methods accordingly in your API. See #321 for context. +* Do not expose third party libraries in the API. This has been a design choice from the beginning of the project, and all existing clients adhere to this convention. #### SPI layer @@ -50,7 +65,7 @@ There are three types of test helpers: #### Tests -API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. +API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. All unit tests run for pull requests, but integration tests are only run upon merging the pull request. We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork. Simple service-related tests should be added to [GoogleCloudPlatform/gcloud-java-examples](https://github.com/GoogleCloudPlatform/gcloud-java-examples/tree/master/test-apps). To test releases and platform-specific bugs, it's valuable to deploy the apps in that repository on App Engine, Compute Engine, and from your own desktop. @@ -73,21 +88,20 @@ Notes/reminders: ### Workflow -New services should be created in a branch based on `master`. The branch name should include "alpha". For example, while developing `gcloud-java-pubsub`, all Pub/Sub related work should be done in `pubsub-alpha`. All code should be submitted through pull requests from a branch on a forked repository. Limiting pull request size is very helpful for reviewers. All code that is merged into the branch should be standalone and well-tested. Any todo comments in the code should have an associated Github issue number for tracking purposes. You should periodically pull updates from the master branch, especially if there are project-wide updates or if relevant changes have been made to the core utilities library, `gcloud-java-core`. +New services should be created in a branch based on `master`. The branch name should include the suffix "-alpha". For example, while developing `gcloud-java-pubsub`, all Pub/Sub related work should be done in `pubsub-alpha`. All code should be submitted through pull requests from a branch on a forked repository. Limiting pull request size is very helpful for reviewers. All code that is merged into the branch should be standalone and well-tested. Any todo comments in the code should have an associated Github issue number for tracking purposes. You should periodically pull updates from the master branch, especially if there are project-wide updates or if relevant changes have been made to the core utilities library, `gcloud-java-core`. Create at least two milestones (stable and future) for your service and an issue tag with the service name. Create issues for any to-do items and tag them appropriately. This keeps an up-to-date short-term to-do list and also allows for longer term roadmaps. Be sure you've configured the base folder's `pom.xml` correctly. * Add your module to the base directory's `pom.xml` file under the list of modules. * Add your module to the javadoc packaging settings. See PR #802 for an example. - * Add your example to the assembler plugin. PR #839 includes examples of using appassembler. + When your client library is complete, contact the service owners to get a review. The primary purpose of this review is to make sure that the gcloud-java client interacts with the Cloud service properly. Present the reviewers with a link to the Github repository, as well as your (updated) design document that details the API. ### Closing remarks * Efforts should be made to maintain the current style of the repository and a consistent style between gcloud-java client libraries. - * We anticipate that people will often use multiple `gcloud-java` clients, so we don't want differences in conventions from library to library. Look at existing `gcloud-java` clients to see coding and naming conventions. + * We anticipate that people will sometimes use multiple `gcloud-java` clients, so we don't want differences in conventions from library to library. Look at existing `gcloud-java` clients to see coding and naming conventions. * Codacy is configured to report on pull requests about style issues. Whenever possible, those comments should be addressed. Coders and reviewers should also run a linter on pull requests, because the Codacy tool may not catch all style errors. * When weighing which client libraries to add, consider that a hand-crafted `gcloud-java` client library is especially useful if it can abstract away and/or make java-idiomatic significant parts of a service's autogenerated API. - From d4d4d5bcbcc04a82064713487c2f176506e52be1 Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Thu, 14 Apr 2016 15:59:14 -0700 Subject: [PATCH 3/6] Rename doc + address comments --- CONTRIBUTING.md | 6 +++--- ...NG_NEW_CLIENTS.md => SUPPORTING_NEW_SERVICES.md | 14 ++++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) rename ADDING_NEW_CLIENTS.md => SUPPORTING_NEW_SERVICES.md (93%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0bfee7150bc8..b4f98acd3f1f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,9 +43,9 @@ The feature must work fully on Java 7 and above. The feature must not add unnecessary dependencies (where "unnecessary" is of course subjective, but new dependencies should be discussed). -Adding a New Client Library ---------------------------- -See [ADDING_NEW_CLIENTS](./ADDING_NEW_CLIENTS.md) for guidelines on how to add a new client library to `gcloud-java`. +Adding Support for a New Service +-------------------------------- +See [SUPPORTING_NEW_SERVICES](./SUPPORT_NEW_SERVICES.md) for guidelines on how to add a new client library to `gcloud-java`. Coding Style ------------ diff --git a/ADDING_NEW_CLIENTS.md b/SUPPORTING_NEW_SERVICES.md similarity index 93% rename from ADDING_NEW_CLIENTS.md rename to SUPPORTING_NEW_SERVICES.md index e049e9753dae..151e83538b10 100644 --- a/ADDING_NEW_CLIENTS.md +++ b/SUPPORTING_NEW_SERVICES.md @@ -1,4 +1,4 @@ -## How to Add a New Client Library +## Supporting New Services ### Overview @@ -18,7 +18,7 @@ This document outlines how to add a new client library to `gcloud-java`. New cl Before starting work on the API layer, write a design document and provide sample API code, either in the design document or as a pull request tagged with the "don't merge" label. As part of the design process, be sure to examine the Google Cloud service API and any implementations provided in other gcloud-* language libraries. Solicit feedback from other contributors to the repository. Client libraries should be low-level while minimizing boilerplate code needed by API users. The library should also be flexible enough to be used by higher-level libraries. For example, Objectify should be able to use `gcloud-java-datastore`. The library should contain: -* A subclass of the [`ServiceOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceOptions.java) class. The `ServiceOptions` class contains important information for communicating with the Google Cloud service, such as the project ID, authentication, and retry handling. Subclasses should add/override relevant methods as necessary. Example: [`DatastoreOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java) +* A subclass of the [`ServiceOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceOptions.java) class. The `ServiceOptions` class contains important information for communicating with the Google Cloud service, such as the project ID, authentication, and retry handling. Subclasses should add/override relevant methods as necessary. Example: [`DatastoreOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java). * An interface extending the [`Service`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/Service.java) interface (example: [`Datastore`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/Datastore.java)) and an implentation of that interface (example: [`DatastoreImpl`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java)). * An interface extending the [`ServiceFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceFactory.java) interface. Example: [`DatastoreFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreFactory.java) * A runtime exception class that extends [`BaseServiceException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java), which is used to wrap service-related exceptions. Example: [`DatastoreException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java) @@ -29,7 +29,7 @@ In general, make classes immutable whenever possible, providing builders as nece `gcloud-java-core` provides functionality for code patterns used across `gcloud-java` libraries. The following are some important core concepts: * Paging: Cloud services often expose page-based listing using page tokens. The [`Page`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/Page.java) interface should be used for page-based listing. A `Page` contains an iterator over results in that page, as well as methods to get the next page and all results in future pages. `Page` requires a `NextPageFetcher` implementation (see the `NextPageFetcher` interface in [`PageImpl`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/PageImpl.java)). This implementation should delegate constructing request options to the `nextRequestOptions` method in `PageImpl`. -* Exception handling: we have the notion of retryable vs non-retryable exceptions and idempotent vs non-idempotent exceptions. These are encapsulated by `BaseServiceException`. Retryable error codes should be listed in the client's subclass of `BaseServiceException`, specifying whether exception is idempotent. The subclass should also provide methods to translate the exceptions given by the underlying autogeneraged client library. The [`ExceptionHandler`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ExceptionHandler.java) class intercepts and decides how to handle exceptions (based on the set of retryable error codes) when making RPC calls. +* Exception handling: we have the notion of retryable vs non-retryable exceptions and idempotent vs non-idempotent exceptions. These are encapsulated by `BaseServiceException`. Retryable error codes should be listed in the client's subclass of `BaseServiceException`. An exception should be considered retryable if it makes sense to retry (e.g. if there was a transient error, not a fundamentally invalid request) and if the exception is idempotent. The subclass should also provide methods to translate the exceptions given by the underlying autogeneraged client library. The [`ExceptionHandler`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ExceptionHandler.java) class intercepts and retries RPC calls when retryable exceptions are encountered. * Batching: The [`BatchResult`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/dns-alpha-batch/gcloud-java-core/src/main/java/com/google/cloud/BatchResult.java) class provides a simple way for users to combine RPC calls for performance enhancement. Clients for services that provide batching should provide a batch class that contains methods similar to the methods in the `Service.java` subclass, but instead return a subclass of `BatchResult`. Also provide an SPI-layer class to collect batch requests and submit the batch. @@ -40,6 +40,8 @@ Notes/reminders: * While not all fields for model objects need to be exposed to the user, `gcloud-java` clients should get and set all relevant fields when making RPC calls to the Cloud service's API. For example, since the `parent` field of Cloud Resource Manager Project objects is in alpha (at the time this is written) and not available to most users, `gcloud-java-resourcemanager` gets and sets the parent when interacting with the Cloud Resource Manager, but does not expose the parent to users. As a result, the user won't accidentally unset the parent when updating a project. * Be aware of differences in "update" behavior and name update/replace methods accordingly in your API. See #321 for context. * Do not expose third party libraries in the API. This has been a design choice from the beginning of the project, and all existing clients adhere to this convention. +* Member variable getters and builder setters should not use the prefixes "get" or "set". +* Any service-generated IDs for model objects should be named `generatedId()`. #### SPI layer @@ -55,7 +57,7 @@ Test helper classes should be located in the package `com.google.cloud.servicena There are three types of test helpers: * When a local emulator is already available, your test helper should launch that emulator and return service options to connect to that local emulator. This enables both users and our own library to run unit tests easily. An example of this type of helper is `LocalDatastoreHelper`. Google Cloud Datastore provides a script that launches a local datastore, so `LocalDatastoreHelper` launches that script in a separate process when the user calls `start()`. -* When there is no local emulator but the service is simple enough to write an emulator, you should do so. The emulator should listen to a port for requests, process those requests, and send responses back, being as true to the actual service as possible. Be sure to document differences between your emulator and the actual service. Examples of this type of test helper are `LocalResourceManagerHelper` and `LocalDnsHelper`. +* When there is no local emulator but the service is simple enough to write an emulator, you should do so. The emulator should listen to a port for requests, process those requests, and send responses back, being as true to the actual service as possible. Dependencies in this mock should be as lightweight as possible. Be sure to document differences between your emulator and the actual service. Examples of this type of test helper are `LocalResourceManagerHelper` and `LocalDnsHelper`. * When there is no local emulator and the service is too complex to write a solid emulator, the test helper should contain methods to get options and separate test data from other user data. `RemoteStorageHelper` is an example of this type of test helper, since there is no local emulator for Google Cloud Storage (at the time that this is written) and because the Google Cloud Storage API is complex. `RemoteStorageHelper` has methods to: * Get service options settings. @@ -65,7 +67,7 @@ There are three types of test helpers: #### Tests -API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. All unit tests run for pull requests, but integration tests are only run upon merging the pull request. We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork. +API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. All unit tests run for pull requests, but integration tests are only run upon merging the pull request. We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork. For this purpose, integration test file names must be prefixed with "IT" (e.g. `ITDnsTest`). Simple service-related tests should be added to [GoogleCloudPlatform/gcloud-java-examples](https://github.com/GoogleCloudPlatform/gcloud-java-examples/tree/master/test-apps). To test releases and platform-specific bugs, it's valuable to deploy the apps in that repository on App Engine, Compute Engine, and from your own desktop. @@ -93,7 +95,7 @@ New services should be created in a branch based on `master`. The branch name s Create at least two milestones (stable and future) for your service and an issue tag with the service name. Create issues for any to-do items and tag them appropriately. This keeps an up-to-date short-term to-do list and also allows for longer term roadmaps. Be sure you've configured the base folder's `pom.xml` correctly. - * Add your module to the base directory's `pom.xml` file under the list of modules. + * Add your module to the base directory's `pom.xml` file under the list of modules (in alphabetical order). * Add your module to the javadoc packaging settings. See PR #802 for an example. From 4d00a2849c2bd418b2e93e977b5193928e1ad4d0 Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Thu, 14 Apr 2016 17:13:23 -0700 Subject: [PATCH 4/6] remove client naming + other discussed fixes --- CONTRIBUTING.md | 2 +- SUPPORTING_NEW_SERVICES.md | 41 +++++++++++++++++++------------------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b4f98acd3f1f..3ed827deb5af 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -45,7 +45,7 @@ but new dependencies should be discussed). Adding Support for a New Service -------------------------------- -See [SUPPORTING_NEW_SERVICES](./SUPPORT_NEW_SERVICES.md) for guidelines on how to add a new client library to `gcloud-java`. +See [SUPPORTING_NEW_SERVICES](./SUPPORTING_NEW_SERVICES.md) for guidelines on how to add support for a new Google Cloud service to `gcloud-java`. Coding Style ------------ diff --git a/SUPPORTING_NEW_SERVICES.md b/SUPPORTING_NEW_SERVICES.md index 151e83538b10..56ebf073a13c 100644 --- a/SUPPORTING_NEW_SERVICES.md +++ b/SUPPORTING_NEW_SERVICES.md @@ -2,44 +2,44 @@ ### Overview -This document outlines how to add a new client library to `gcloud-java`. New client libraries should be submodules located in a folder within the main repository, built using Maven. A client library should contain the following items: +This document outlines how to add support for a new service in `gcloud-java`. New services should be submodules located in a folder within the main repository, built using Maven. A new service should contain the following items: * An API layer, with which users will interact. This includes model objects and a service class. -* An SPI layer, which translates gcloud-java API calls into RPCs using an autogenerated client library. In almost all use cases, the user will not directly interact with this code. Separating this code from the API layer allows the API layer to remain stable despite changes to the autogenerated libraries used. -* A test helper class, which allows users to easily interact with a local emulator (if possible). If there is no emulator available and the service is too complex to create a mock, then this helper should facilitate separation of test data from other user data and enable easy cleanup. +* An SPI layer, which translates gcloud-java API calls into RPCs using an autogenerated client library. In almost all use cases, the user will not directly interact with this code. Separating this code from the API layer provides two benefits. First, it allows the API layer to remain stable despite changes to the autogenerated libraries used. Second, it makes testing easier, since the RPC implementation can be substituted with a mock. +* A test helper class, which allows users to easily interact with a local service emulator (if possible). If there is no emulator available and the service is too complex to create a mock, then a remote test helper should be provided to separate test data from other user data and enable easy cleanup. * Tests, including unit tests and integration tests. * A command line example application. * Documentation, which is comprised of READMEs, Javadoc, and code snippets. -### Components of a new client library +### Components of a new service #### API layer -Before starting work on the API layer, write a design document and provide sample API code, either in the design document or as a pull request tagged with the "don't merge" label. As part of the design process, be sure to examine the Google Cloud service API and any implementations provided in other gcloud-* language libraries. Solicit feedback from other contributors to the repository. Client libraries should be low-level while minimizing boilerplate code needed by API users. The library should also be flexible enough to be used by higher-level libraries. For example, Objectify should be able to use `gcloud-java-datastore`. +Before starting work on the API layer, write a design document and provide sample API code. Sample API code should either be included in the design document or as a pull request tagged with the "don't merge" label. As part of the design process, be sure to examine the Google Cloud service API and any implementations provided in other gcloud-* language libraries. Solicit feedback from other contributors to the repository. `gcloud-java` services should be low-level while minimizing boilerplate code needed by API users. They should also be flexible enough to be used by higher-level libraries. For example, [Objectify](https://github.com/objectify/objectify) should be able to use `gcloud-java-datastore`. The library should contain: * A subclass of the [`ServiceOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceOptions.java) class. The `ServiceOptions` class contains important information for communicating with the Google Cloud service, such as the project ID, authentication, and retry handling. Subclasses should add/override relevant methods as necessary. Example: [`DatastoreOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java). * An interface extending the [`Service`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/Service.java) interface (example: [`Datastore`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/Datastore.java)) and an implentation of that interface (example: [`DatastoreImpl`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java)). * An interface extending the [`ServiceFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceFactory.java) interface. Example: [`DatastoreFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreFactory.java) * A runtime exception class that extends [`BaseServiceException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java), which is used to wrap service-related exceptions. Example: [`DatastoreException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java) -* Classes representing model objects and request-related options. Commonly-used classes that contain metadata should also have a subclass that provides functions related to that metadata. For example, see [`BlobInfo`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-storage/src/main/java/com/google/cloud/storage/BlobInfo.java) (the metadata class) and [`Blob`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-storage/src/main/java/com/google/cloud/storage/Blob.java) (the functional class). The builders for both objects should implement a common interface or abstract class, and the functional subclass builder should delegate to the metadata class builder. +* Classes representing model objects and request-related options. Model objects that correspond to service resources should have a subclass that provides functions related to that resource. For example, see [`BlobInfo`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-storage/src/main/java/com/google/cloud/storage/BlobInfo.java) (the metadata class) and [`Blob`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-storage/src/main/java/com/google/cloud/storage/Blob.java) (the functional class). The builders for both objects should implement a common interface or abstract class, and the functional subclass builder should delegate to the metadata class builder. -In general, make classes immutable whenever possible, providing builders as necessary. Make model object classes `java.util.Serializable`. Also, make classes final when possible, except when the class contains functionality that cannot be fully tested by users without mocking the object. If a class cannot be made final, then `hashCode` or `equals` overrides should be made final if possible. +In general, make classes immutable whenever possible, providing builders as necessary. Make model object classes `java.util.Serializable`. Prefer making classes final, with the following exceptions: (1) functional objects and (2) classes in which the user cannot set all attributes. If a class cannot be made final, then `hashCode` or `equals` overrides should be made final if possible. `gcloud-java-core` provides functionality for code patterns used across `gcloud-java` libraries. The following are some important core concepts: * Paging: Cloud services often expose page-based listing using page tokens. The [`Page`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/Page.java) interface should be used for page-based listing. A `Page` contains an iterator over results in that page, as well as methods to get the next page and all results in future pages. `Page` requires a `NextPageFetcher` implementation (see the `NextPageFetcher` interface in [`PageImpl`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/PageImpl.java)). This implementation should delegate constructing request options to the `nextRequestOptions` method in `PageImpl`. -* Exception handling: we have the notion of retryable vs non-retryable exceptions and idempotent vs non-idempotent exceptions. These are encapsulated by `BaseServiceException`. Retryable error codes should be listed in the client's subclass of `BaseServiceException`. An exception should be considered retryable if it makes sense to retry (e.g. if there was a transient error, not a fundamentally invalid request) and if the exception is idempotent. The subclass should also provide methods to translate the exceptions given by the underlying autogeneraged client library. The [`ExceptionHandler`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ExceptionHandler.java) class intercepts and retries RPC calls when retryable exceptions are encountered. +* Exception handling: we have the notion of retryable versus non-retryable operations. These are encapsulated by `BaseServiceException`. Retryable error codes should be listed in the service's subclass of `BaseServiceException`. An operation should be considered retryable if it makes sense to retry (e.g. if there was a transient service error, not a fundamentally invalid request) and if the operation that triggered the exception is idempotent. Exceptions also contain information regarding whether the service rejected the request, meaning the operation was not applied. The `BaseServiceException` subclass should also provide methods to translate the exceptions given by the underlying autogeneraged client library. The [`ExceptionHandler`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ExceptionHandler.java) class intercepts and retries RPC calls when retryable exceptions are encountered. Note that some exceptions are masked in the SPI layer. For example, `get` and `delete` operations often return "404 Not Found" if the resource doesn't exist. Instead of throwing an exception in these cases, we often return `null` for `get` and `false` for `delete`. -* Batching: The [`BatchResult`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/dns-alpha-batch/gcloud-java-core/src/main/java/com/google/cloud/BatchResult.java) class provides a simple way for users to combine RPC calls for performance enhancement. Clients for services that provide batching should provide a batch class that contains methods similar to the methods in the `Service.java` subclass, but instead return a subclass of `BatchResult`. Also provide an SPI-layer class to collect batch requests and submit the batch. +* Batching: The [`BatchResult`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/dns-alpha-batch/gcloud-java-core/src/main/java/com/google/cloud/BatchResult.java) class provides a simple way for users to combine RPC calls for performance enhancement. API for services that support batching should implement a batch class that contains methods similar to the methods in the `Service.java` subclass, but instead return a subclass of `BatchResult`. Also provide an SPI-layer class to collect batch requests and submit the batch. Notes/reminders: * API layer classes should be located in the package `com.google.cloud.servicename`, where "servicename" corresponds to the name of the Cloud service. * Override the `ServiceOptions.defaultRetryParams()` method in your service's options class to align with the Service Level Agreement (SLA) given by the underlying service. See #857 and #860 for context. * See conventions about overriding the `equals` and `hashCode` methods in the discussion of #892. -* While not all fields for model objects need to be exposed to the user, `gcloud-java` clients should get and set all relevant fields when making RPC calls to the Cloud service's API. For example, since the `parent` field of Cloud Resource Manager Project objects is in alpha (at the time this is written) and not available to most users, `gcloud-java-resourcemanager` gets and sets the parent when interacting with the Cloud Resource Manager, but does not expose the parent to users. As a result, the user won't accidentally unset the parent when updating a project. +* While not all fields for model objects need to be exposed to the user, `gcloud-java` services should get and set all relevant fields when making RPC calls to the Cloud service's API. For example, since the `parent` field of Cloud Resource Manager Project objects is in alpha (at the time this is written) and not available to most users, `gcloud-java-resourcemanager` gets and sets the parent when interacting with the Cloud Resource Manager, but does not expose the parent to users. As a result, the user won't accidentally unset the parent when updating a project. * Be aware of differences in "update" behavior and name update/replace methods accordingly in your API. See #321 for context. -* Do not expose third party libraries in the API. This has been a design choice from the beginning of the project, and all existing clients adhere to this convention. +* Do not expose third party libraries in the API. This has been a design choice from the beginning of the project, and all existing `gcloud-java` services adhere to this convention. * Member variable getters and builder setters should not use the prefixes "get" or "set". * Any service-generated IDs for model objects should be named `generatedId()`. @@ -62,12 +62,11 @@ There are three types of test helpers: * When there is no local emulator and the service is too complex to write a solid emulator, the test helper should contain methods to get options and separate test data from other user data. `RemoteStorageHelper` is an example of this type of test helper, since there is no local emulator for Google Cloud Storage (at the time that this is written) and because the Google Cloud Storage API is complex. `RemoteStorageHelper` has methods to: * Get service options settings. * Create a test bucket with a sufficiently obscure name (to separate the bucket from any of the users other data). - * Clear up data left over from tests in that test bucket. - + * Clean up data left over from tests in that test bucket. #### Tests -API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. All unit tests run for pull requests, but integration tests are only run upon merging the pull request. We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork. For this purpose, integration test file names must be prefixed with "IT" (e.g. `ITDnsTest`). +API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. All unit tests run for pull requests, but integration tests are only run upon merging the pull request. We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork. To separate integration tests from unit tests,prefix integration test file names with "IT" (e.g. `ITDnsTest`). Simple service-related tests should be added to [GoogleCloudPlatform/gcloud-java-examples](https://github.com/GoogleCloudPlatform/gcloud-java-examples/tree/master/test-apps). To test releases and platform-specific bugs, it's valuable to deploy the apps in that repository on App Engine, Compute Engine, and from your own desktop. @@ -78,9 +77,9 @@ The example application should be a simple command line interface for the servic #### Documentation * Include a summary of the service and code snippets on the main repository's README. These snippets should be simple and cover a few common usage patterns. The README snippets should also be added to `gcloud-java-examples` in the package `com.google.cloud.examples.servicename.snippets`. Placing snippet code in the repository ensures that the snippet code builds when Travis CI is run. For this purpose, README snippets and the snippet code in `gcloud-java-examples` should be kept in sync. As of yet, we do not have unit tests for snippets, so the snippets should be tested periodically, especially after any relevant library updates. -* Create a README in the client library's folder. This README should mimic the structure of other client libraries' READMEs. In particular, you should create a step-by-step "Getting Started" guide. See [`gcloud-java-datastore`'s README](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/README.md) for reference. All code in that step-by-step guide should also be included in the `gcloud-java-examples` snippets package. +* Create a README in the service's folder. This README should mimic the structure of other services' READMEs. In particular, you should create a step-by-step "Getting Started" guide. See [`gcloud-java-datastore`'s README](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/README.md) for reference. All code in that step-by-step guide should also be included in the `gcloud-java-examples` snippets package. * The API and test helper packages should have `package-info.java` files. These files should contain descriptions of the packages as well as simple example code and/or links to code snippets. -* Public methods, classes, and builders should contain meaningful Javadoc. Document both unchecked and checked exceptions. +* Public methods, classes, and builders should contain meaningful Javadoc. When possible, copy docs from the service's `cloud.google.com` API documentation and provide `@see` links to relevant Google Cloud web pages. Document both unchecked and checked exceptions. * Update [`TESTING`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/TESTING.md) with how to run tests using the test helper. * Update the [`gcloud-java-examples` README](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-examples/README.md) with instructions on how to run your example application. @@ -97,13 +96,13 @@ Create at least two milestones (stable and future) for your service and an issue Be sure you've configured the base folder's `pom.xml` correctly. * Add your module to the base directory's `pom.xml` file under the list of modules (in alphabetical order). * Add your module to the javadoc packaging settings. See PR #802 for an example. - + [comment]: # (TODO: uncomment when gcs-nio branch is merged into master) * Add your example to the assembler plugin. PR #839 includes examples of using appassembler. -When your client library is complete, contact the service owners to get a review. The primary purpose of this review is to make sure that the gcloud-java client interacts with the Cloud service properly. Present the reviewers with a link to the Github repository, as well as your (updated) design document that details the API. +When your service is complete, contact the service owners to get a review. The primary purpose of this review is to make sure that the gcloud-java service interacts with the Cloud service properly. Present the reviewers with a link to the Github repository, as well as your (updated) design document that details the API. ### Closing remarks -* Efforts should be made to maintain the current style of the repository and a consistent style between gcloud-java client libraries. - * We anticipate that people will sometimes use multiple `gcloud-java` clients, so we don't want differences in conventions from library to library. Look at existing `gcloud-java` clients to see coding and naming conventions. +* Efforts should be made to maintain the current style of the repository and a consistent style between gcloud-java services. + * We anticipate that people will sometimes use multiple `gcloud-java` services, so we don't want differences in conventions from library to library. Look at existing `gcloud-java` services to see coding and naming conventions. * Codacy is configured to report on pull requests about style issues. Whenever possible, those comments should be addressed. Coders and reviewers should also run a linter on pull requests, because the Codacy tool may not catch all style errors. -* When weighing which client libraries to add, consider that a hand-crafted `gcloud-java` client library is especially useful if it can abstract away and/or make java-idiomatic significant parts of a service's autogenerated API. +* When weighing which services to add, consider that a hand-crafted `gcloud-java` service API is especially useful if it can abstract away and/or make java-idiomatic significant parts of a service's autogenerated API. From 666c3d48e2d5744307ce1c1afeb4b06f801f054b Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Fri, 15 Apr 2016 10:14:36 -0700 Subject: [PATCH 5/6] Incorporate feedback --- SUPPORTING_NEW_SERVICES.md | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/SUPPORTING_NEW_SERVICES.md b/SUPPORTING_NEW_SERVICES.md index 56ebf073a13c..868347242e9d 100644 --- a/SUPPORTING_NEW_SERVICES.md +++ b/SUPPORTING_NEW_SERVICES.md @@ -23,24 +23,28 @@ The library should contain: * An interface extending the [`ServiceFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceFactory.java) interface. Example: [`DatastoreFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreFactory.java) * A runtime exception class that extends [`BaseServiceException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java), which is used to wrap service-related exceptions. Example: [`DatastoreException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java) * Classes representing model objects and request-related options. Model objects that correspond to service resources should have a subclass that provides functions related to that resource. For example, see [`BlobInfo`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-storage/src/main/java/com/google/cloud/storage/BlobInfo.java) (the metadata class) and [`Blob`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-storage/src/main/java/com/google/cloud/storage/Blob.java) (the functional class). The builders for both objects should implement a common interface or abstract class, and the functional subclass builder should delegate to the metadata class builder. +* Optional, request-related options classes, located in the interface that extends `Service.java`. Methods should accept these options as arguments when appropriate. The options classes should provide static methods to create instances with specific options settings. A common option is to request that only specific fields of a model object should be included in a response. Typically such an option is created via a `fields(...)` method which accepts a vararg of type `Field` enum. The enum should implement the [FieldSelector](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/FieldSelector.java) interface. In general, make classes immutable whenever possible, providing builders as necessary. Make model object classes `java.util.Serializable`. Prefer making classes final, with the following exceptions: (1) functional objects and (2) classes in which the user cannot set all attributes. If a class cannot be made final, then `hashCode` or `equals` overrides should be made final if possible. `gcloud-java-core` provides functionality for code patterns used across `gcloud-java` libraries. The following are some important core concepts: -* Paging: Cloud services often expose page-based listing using page tokens. The [`Page`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/Page.java) interface should be used for page-based listing. A `Page` contains an iterator over results in that page, as well as methods to get the next page and all results in future pages. `Page` requires a `NextPageFetcher` implementation (see the `NextPageFetcher` interface in [`PageImpl`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/PageImpl.java)). This implementation should delegate constructing request options to the `nextRequestOptions` method in `PageImpl`. +* Paging: Google Cloud services often expose page-based listing using page tokens. The [`Page`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/Page.java) interface should be used for page-based listing. A `Page` contains an iterator over results in that page, as well as methods to get the next page and all results in future pages. `Page` requires a `NextPageFetcher` implementation (see the `NextPageFetcher` interface in [`PageImpl`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/PageImpl.java)). This implementation should delegate constructing request options to the `nextRequestOptions` method in `PageImpl`. * Exception handling: we have the notion of retryable versus non-retryable operations. These are encapsulated by `BaseServiceException`. Retryable error codes should be listed in the service's subclass of `BaseServiceException`. An operation should be considered retryable if it makes sense to retry (e.g. if there was a transient service error, not a fundamentally invalid request) and if the operation that triggered the exception is idempotent. Exceptions also contain information regarding whether the service rejected the request, meaning the operation was not applied. The `BaseServiceException` subclass should also provide methods to translate the exceptions given by the underlying autogeneraged client library. The [`ExceptionHandler`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ExceptionHandler.java) class intercepts and retries RPC calls when retryable exceptions are encountered. Note that some exceptions are masked in the SPI layer. For example, `get` and `delete` operations often return "404 Not Found" if the resource doesn't exist. Instead of throwing an exception in these cases, we often return `null` for `get` and `false` for `delete`. -* Batching: The [`BatchResult`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/dns-alpha-batch/gcloud-java-core/src/main/java/com/google/cloud/BatchResult.java) class provides a simple way for users to combine RPC calls for performance enhancement. API for services that support batching should implement a batch class that contains methods similar to the methods in the `Service.java` subclass, but instead return a subclass of `BatchResult`. Also provide an SPI-layer class to collect batch requests and submit the batch. +* Batching: The [`BatchResult`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/dns-alpha-batch/gcloud-java-core/src/main/java/com/google/cloud/BatchResult.java) class provides a simple way for users to combine RPC calls for performance enhancement. APIs for services that support batching should implement a batch class that contains methods similar to the methods in the `Service.java` subclass. A batch operation's return type should be a subclass of `BatchResult`. Also provide an SPI-layer class to collect batch requests and submit the batch. A batch instance should be created by the service API, preferably via a method called `batch()`. The batch should be submitted using the batch instance itself, preferably using a method named `submit()`. + +* IAM Policies: If the Google Cloud service supports [IAM](https://cloud.google.com/iam/docs/), you should provide a subclass of [`IamPolicy`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/IamPolicy.java) in your API. [`Policy`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-resourcemanager/src/main/java/com/google/cloud/resourcemanager/Policy.java) can be used to set default access control lists on Google Cloud projects as a whole. However, if users want to set policies on specific resources within a project, they will need to use the subclass you provide in your API. Notes/reminders: -* API layer classes should be located in the package `com.google.cloud.servicename`, where "servicename" corresponds to the name of the Cloud service. +* API layer classes should be located in the package `com.google.cloud.servicename`, where "servicename" corresponds to the name of the Google Cloud service. * Override the `ServiceOptions.defaultRetryParams()` method in your service's options class to align with the Service Level Agreement (SLA) given by the underlying service. See #857 and #860 for context. +* Override the `ServiceOptions.projectIdRequired()` method to return false when the service does not require requests to be associated with a specific project. * See conventions about overriding the `equals` and `hashCode` methods in the discussion of #892. -* While not all fields for model objects need to be exposed to the user, `gcloud-java` services should get and set all relevant fields when making RPC calls to the Cloud service's API. For example, since the `parent` field of Cloud Resource Manager Project objects is in alpha (at the time this is written) and not available to most users, `gcloud-java-resourcemanager` gets and sets the parent when interacting with the Cloud Resource Manager, but does not expose the parent to users. As a result, the user won't accidentally unset the parent when updating a project. +* While not all fields for model objects need to be exposed to the user, `gcloud-java` services should get and set all relevant fields when making RPC calls to the Google Cloud service's API. For example, since the `parent` field of Cloud Resource Manager Project objects is in alpha (at the time this was written) and not available to most users, `gcloud-java-resourcemanager` gets and sets the parent when interacting with the Cloud Resource Manager, but does not expose the parent to users. As a result, the user won't accidentally unset the parent when updating a project. * Be aware of differences in "update" behavior and name update/replace methods accordingly in your API. See #321 for context. * Do not expose third party libraries in the API. This has been a design choice from the beginning of the project, and all existing `gcloud-java` services adhere to this convention. -* Member variable getters and builder setters should not use the prefixes "get" or "set". +* Member variable getters and builder setters should not use the JavaBean get/set prefix style. * Any service-generated IDs for model objects should be named `generatedId()`. #### SPI layer @@ -59,24 +63,24 @@ There are three types of test helpers: * When there is no local emulator but the service is simple enough to write an emulator, you should do so. The emulator should listen to a port for requests, process those requests, and send responses back, being as true to the actual service as possible. Dependencies in this mock should be as lightweight as possible. Be sure to document differences between your emulator and the actual service. Examples of this type of test helper are `LocalResourceManagerHelper` and `LocalDnsHelper`. -* When there is no local emulator and the service is too complex to write a solid emulator, the test helper should contain methods to get options and separate test data from other user data. `RemoteStorageHelper` is an example of this type of test helper, since there is no local emulator for Google Cloud Storage (at the time that this is written) and because the Google Cloud Storage API is complex. `RemoteStorageHelper` has methods to: +* When there is no local emulator and the service is too complex to write a solid emulator, the test helper should contain methods to get options and to help isolate test data from production data. `RemoteStorageHelper` is an example of this type of test helper, since there is no local emulator for Google Cloud Storage (at the time that this was written) and because the Google Cloud Storage API is complex. `RemoteStorageHelper` has methods to: * Get service options settings. * Create a test bucket with a sufficiently obscure name (to separate the bucket from any of the users other data). * Clean up data left over from tests in that test bucket. #### Tests -API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. All unit tests run for pull requests, but integration tests are only run upon merging the pull request. We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork. To separate integration tests from unit tests,prefix integration test file names with "IT" (e.g. `ITDnsTest`). +API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. All unit tests run for pull requests, but integration tests are only run upon merging the pull request. We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork. Prefix integration test file names with "IT" (e.g. `ITDnsTest`) to separate integration tests from unit tests. Simple service-related tests should be added to [GoogleCloudPlatform/gcloud-java-examples](https://github.com/GoogleCloudPlatform/gcloud-java-examples/tree/master/test-apps). To test releases and platform-specific bugs, it's valuable to deploy the apps in that repository on App Engine, Compute Engine, and from your own desktop. #### Example application -The example application should be a simple command line interface for the service. It should use common library use patterns so that users see good examples of how to use `gcloud-java` when viewing the source code. Be sure to keep the examples up to date if/when there are updates that make the API cleaner and more concise. See examples of applications under the `gcloud-java-examples` folder. The example application should be in the package `com.google.cloud.examples.servicename`. +The example application should be a simple command line interface for the service. It should use common library usage patterns so that users can have good examples of how to use the service. Be sure to keep the examples up to date if/when there are updates that make the API cleaner and more concise. See examples of applications under the `gcloud-java-examples` folder. The example application should be in the package `com.google.cloud.examples.servicename`. #### Documentation -* Include a summary of the service and code snippets on the main repository's README. These snippets should be simple and cover a few common usage patterns. The README snippets should also be added to `gcloud-java-examples` in the package `com.google.cloud.examples.servicename.snippets`. Placing snippet code in the repository ensures that the snippet code builds when Travis CI is run. For this purpose, README snippets and the snippet code in `gcloud-java-examples` should be kept in sync. As of yet, we do not have unit tests for snippets, so the snippets should be tested periodically, especially after any relevant library updates. +* Include a summary of the service and code snippets on the main repository's README. These snippets should be simple and cover a few common usage patterns. The README snippets should also be added to `gcloud-java-examples` in the package `com.google.cloud.examples.servicename.snippets`. Placing snippet code in the repository ensures that the snippet code builds when Travis CI is run. For this purpose, README snippets and the snippet code in `gcloud-java-examples` should be kept in sync. Issue #753 suggests autogenerating READMEs, which would be useful for keeping snippet code in sync. As of yet, we do not have unit tests for snippets, so the snippets should be tested periodically, especially after any relevant library updates. * Create a README in the service's folder. This README should mimic the structure of other services' READMEs. In particular, you should create a step-by-step "Getting Started" guide. See [`gcloud-java-datastore`'s README](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/README.md) for reference. All code in that step-by-step guide should also be included in the `gcloud-java-examples` snippets package. * The API and test helper packages should have `package-info.java` files. These files should contain descriptions of the packages as well as simple example code and/or links to code snippets. * Public methods, classes, and builders should contain meaningful Javadoc. When possible, copy docs from the service's `cloud.google.com` API documentation and provide `@see` links to relevant Google Cloud web pages. Document both unchecked and checked exceptions. @@ -89,20 +93,22 @@ Notes/reminders: ### Workflow -New services should be created in a branch based on `master`. The branch name should include the suffix "-alpha". For example, while developing `gcloud-java-pubsub`, all Pub/Sub related work should be done in `pubsub-alpha`. All code should be submitted through pull requests from a branch on a forked repository. Limiting pull request size is very helpful for reviewers. All code that is merged into the branch should be standalone and well-tested. Any todo comments in the code should have an associated Github issue number for tracking purposes. You should periodically pull updates from the master branch, especially if there are project-wide updates or if relevant changes have been made to the core utilities library, `gcloud-java-core`. +New services should be created in a branch based on `master`. The branch name should include the suffix "-alpha". For example, while developing `gcloud-java-pubsub`, all Pub/Sub related work should be done in `pubsub-alpha`. All code should be submitted through pull requests from a branch on a forked repository. Limiting pull request size is very helpful for reviewers. All code that is merged into the branch should be standalone and well-tested. Any todo comments in the code should have an associated Github issue number for tracking purposes. You should periodically pull updates from the master branch, especially if there are project-wide updates or if relevant changes have been made to the core utilities library, `gcloud-java-core`. This PR should only contain commits related to the merge to ease code review. Create at least two milestones (stable and future) for your service and an issue tag with the service name. Create issues for any to-do items and tag them appropriately. This keeps an up-to-date short-term to-do list and also allows for longer term roadmaps. Be sure you've configured the base folder's `pom.xml` correctly. * Add your module to the base directory's `pom.xml` file under the list of modules (in alphabetical order). * Add your module to the javadoc packaging settings. See PR #802 for an example. - [comment]: # (TODO: uncomment when gcs-nio branch is merged into master) * Add your example to the assembler plugin. PR #839 includes examples of using appassembler. -When your service is complete, contact the service owners to get a review. The primary purpose of this review is to make sure that the gcloud-java service interacts with the Cloud service properly. Present the reviewers with a link to the Github repository, as well as your (updated) design document that details the API. + + +When your service is complete, contact the service owners to get a review. The primary purpose of this review is to make sure that the gcloud-java service interacts with the Google Cloud service properly. Present the reviewers with a link to the Github repository, as well as your (updated) design document that details the API. ### Closing remarks * Efforts should be made to maintain the current style of the repository and a consistent style between gcloud-java services. - * We anticipate that people will sometimes use multiple `gcloud-java` services, so we don't want differences in conventions from library to library. Look at existing `gcloud-java` services to see coding and naming conventions. - * Codacy is configured to report on pull requests about style issues. Whenever possible, those comments should be addressed. Coders and reviewers should also run a linter on pull requests, because the Codacy tool may not catch all style errors. -* When weighing which services to add, consider that a hand-crafted `gcloud-java` service API is especially useful if it can abstract away and/or make java-idiomatic significant parts of a service's autogenerated API. + * We anticipate that people will sometimes use multiple `gcloud-java` services, so we don't want differences in conventions from one service API to another. Look at existing `gcloud-java` services to see coding and naming conventions. + * Codacy is configured to report on pull requests about style issues. Whenever possible, those comments should be addressed. Coders and reviewers should also run a linter on pull requests, because the Codacy tool may not catch all style errors. There is a [Checkstyle configuration](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/checkstyle.xml) provided in the repository. +* When weighing which services to add, consider that a hand-crafted `gcloud-java` service API is especially useful if it can simplify the usability of the autogenerated client. From 7cefdfed737fd3da99c736dd811b6c003094671a Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Fri, 15 Apr 2016 14:58:25 -0700 Subject: [PATCH 6/6] fixes --- SUPPORTING_NEW_SERVICES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SUPPORTING_NEW_SERVICES.md b/SUPPORTING_NEW_SERVICES.md index 868347242e9d..7eba222336de 100644 --- a/SUPPORTING_NEW_SERVICES.md +++ b/SUPPORTING_NEW_SERVICES.md @@ -23,7 +23,7 @@ The library should contain: * An interface extending the [`ServiceFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceFactory.java) interface. Example: [`DatastoreFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreFactory.java) * A runtime exception class that extends [`BaseServiceException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java), which is used to wrap service-related exceptions. Example: [`DatastoreException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java) * Classes representing model objects and request-related options. Model objects that correspond to service resources should have a subclass that provides functions related to that resource. For example, see [`BlobInfo`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-storage/src/main/java/com/google/cloud/storage/BlobInfo.java) (the metadata class) and [`Blob`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-storage/src/main/java/com/google/cloud/storage/Blob.java) (the functional class). The builders for both objects should implement a common interface or abstract class, and the functional subclass builder should delegate to the metadata class builder. -* Optional, request-related options classes, located in the interface that extends `Service.java`. Methods should accept these options as arguments when appropriate. The options classes should provide static methods to create instances with specific options settings. A common option is to request that only specific fields of a model object should be included in a response. Typically such an option is created via a `fields(...)` method which accepts a vararg of type `Field` enum. The enum should implement the [FieldSelector](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/FieldSelector.java) interface. +* Request-related options classes. Operations should accept these options as varargs when appropriate. Supplying options as varargs allows for supporting more advanced use cases without affecting the method signature. The options classes should provide static methods to create instances with specific options settings. A common option is to request that only specific fields of a model object should be included in a response. Typically such an option is created via a `fields(...)` method which accepts a vararg of type `Field` enum. The enum should implement the [FieldSelector](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/FieldSelector.java) interface. In general, make classes immutable whenever possible, providing builders as necessary. Make model object classes `java.util.Serializable`. Prefer making classes final, with the following exceptions: (1) functional objects and (2) classes in which the user cannot set all attributes. If a class cannot be made final, then `hashCode` or `equals` overrides should be made final if possible. @@ -70,7 +70,7 @@ There are three types of test helpers: #### Tests -API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. All unit tests run for pull requests, but integration tests are only run upon merging the pull request. We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork. Prefix integration test file names with "IT" (e.g. `ITDnsTest`) to separate integration tests from unit tests. +API-level functionality should be well-covered by unit tests. Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested. As of now, `gcloud-java` relies on integration tests to test the SPI layer. Unit tests for the API layer should be located in the package `com.google.cloud.servicename`. Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs. Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`. All unit tests run for pull requests, but integration tests are only run upon merging the pull request. We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork. Prefix integration test file names with "IT" (e.g. `ITDnsTest`) to separate them from regular unit tests. Simple service-related tests should be added to [GoogleCloudPlatform/gcloud-java-examples](https://github.com/GoogleCloudPlatform/gcloud-java-examples/tree/master/test-apps). To test releases and platform-specific bugs, it's valuable to deploy the apps in that repository on App Engine, Compute Engine, and from your own desktop.