Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation on supporting new services #923

Merged
merged 6 commits into from
Apr 15, 2016
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions ADDING_NEW_CLIENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

This comment was marked as spam.

This comment was marked as spam.


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)

This comment was marked as spam.

* 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)).

This comment was marked as spam.

* 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.

This comment was marked as spam.


* 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.

This comment was marked as spam.

This comment was marked as spam.

* 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.

This comment was marked as spam.

* 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.

This comment was marked as spam.

#### SPI layer

Expand All @@ -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.

This comment was marked as spam.


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.

Expand All @@ -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`.

This comment was marked as spam.

This comment was marked as spam.


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.

This comment was marked as spam.

* 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.
<!--- (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.

### 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.