This section covers practices that are encouraged or discouraged. Reading this section will help guide development in a way that keeps the tenets of jclouds in-tact, and review comments to a minimum.
Start In many cases, we are already practicing things mentioned as Start, just not consistently. Start is mentioned here to keep people on the same page.
Stop Stop means stop propagating, ex. things we should change, but not necessarily backport. For example, we should not employ practice named Stop in any labs api or anything promoted out of labs. However, if something mentioned as Stop exists in a current non-labs api or provider, we should more carefully plan, and possibly allow `Stop` practice to persist for sake of consistency with other code in that api
Undo Undo are unconditionally something we should work on, in any maintained version of jclouds, as these things limit our ability to progress the entire codebase.
General Practice
(Stop) silo'ing fixes!
(Stop) using Set for everything!
(Undo) putting `@Singleton` on everything!
@Singleton indicates you are holding shared state not given to you in the constructor. Putting @Singleton on everything waters down the concept and slows the runtime of jclouds.
(Stop) Using guava for user-visible types, especially simple ones.
Api interfaces
(Stop) Cargo-Culting aka blindly copy/pasting annotations
There are numerous cases where people have copy/pasted @Fallback annotations which either don't make sense, don't make sense for the api, or actually cause problems. For example, I've seen fallbacks to false on methods that return void, or a list. Also, we fallback on 404 to list methods whose documentation say 404 is not possible. Read the api documentation before making a fallback. If you have a fallback, test it.
Value (Domain) Types
(Start) Using Auto-Value for all value types
(Stop) Declaring top-level types for subordinate classes.
If you have a status object that's only valid for Server, make it a static inner class of Server. Similarly for single-field types. This makes it easier to understand the top-level, most important types, and prevents us from having to use naming conventions like ServerStatus to disambiguate.
(Undo) Writing builders for output-only types
(Continue) Writing builders for user-input objects with more that 3 parameters.
(Stop) Writing complex builders
Tests
(Continue) Writing MockWebServer (MockTests)
MWS basic setup
MWS advanced setup
Handling links in server responses
protected String stringFromResource(String resourceName) {
try {
return toStringAndClose(getClass().getResourceAsStream(resourceName))
.replace("https://www.googleapis.com/compute/v1/", url("/"));
} catch (IOException e) {
throw propagate(e);
}
}
MWS do's and don'ts
(Undo) Testing default fallbacks
Tests that cover fallback annotations you didn't add clutters the code, and drowns out important test cases. Only test for code you added.
(Undo) Subclassing BaseRestApiExpectTest
(Undo) Subclassing BaseRestAnnotationProcessingTest
Pagination
For each method that returns paginated results, you will need two methods in the api: one without parameters that returns the entire list, and the other one, with the pagination parameters, that is able to return a single page. In jclouds:
- The first one, returning a single page, must return an
IterableWithMarker<T>
, which is just an Iterable with a marker that points to the next page. - The second one, the entire list, must return a
PagedIterable<T>
, which is an Iterable that has a single page (anIterableWithMarker<T>
) and knows how to get the next page given the next page marker.
Having this in mind, you need to figure out two things:
- How to build the
IterableWithMarker<T>
so it contains a marker with all the info required to fetch the next page. - How to build the
PagedIterable<T>
to automatically fetch pages given a marker.
Building the first one should be pretty straightforward. You just need to create a class that extends theIterableWithMarker<T>
abstract class and make sure the returned marker has all the info to fetch the next page. In your example, the "meta" fields could serve this purpose. Once you have your implementation, you just need to create a ResponseParser that builds your class given an HttpResponse. That's what the ParseImages class in Glance does.
Next thing you have to do, is build a PagedIterable<T>
given an IterableWithMarker<T>
. Since the APIs always return a single page, in the end, the HttpResponses for the "listAll" and "listSinglePage" methods will be the same. That's why most "listAll" methods use the same response parser than above, and then use a transformation function to transform the resulting IterableWithMarker<T>
into a PagedIterable<T>
(see the ImageApi annotations).
So, that ToPagedIterable
is just a function that transforms an IterableWithMarker<T>
into aPagedIterable<T>
. This can be done in many ways, depending on the API. This functions usually build the iterable using the helper methods in the PagedIterables class, such as the advance one. If you take a look at that method you'll see that the PagedIterable is build given an IterableWithMarker<T>
and an advancing function. That advancing function transforms an Object
to an IterablewithMarker<T>
where the Object is the current marker to the next page, and the result should be the next page. That advancing function will only be invoked if the marker to the next page is present.
So, in the end, you just need to create a function to transform an IterableWithMarker<T>
into aPagediterable<T>
. How you build it is up to you. Here are a few examples you can take a look at, to get the whole idea:
- In Abiquo, for example, each page returns a link to the next one, so there is a generic function that simply performs a GET call to fetch the next page, given the marker (the link).
- In Glance, there is the ParseImages#ToPagedIterable that extends an existing helper class to build a new API call. The class it extends populates the caller arguments to the function, but this might not be the best example in your case and could bring confusion.
- There is a helper class that you could also use (and perhaps this is what I'd recommend if there is no generic way to do pagination for all API calls). You could create a function that extends the existing ArgsToPagedIterable. That class already has the common logic and you only have to implement its abstract method. That method must return a function that returns the next page given an
IterableWithMarker<T>
, but also receives as a parameter the list of arguments (the arguments of the invoked java method) used in the previous API call, which can be useful to build the call to the next page. You can take the ParseImages#ToPagedIterable as an example to get the idea, but apply that to the ArgsToPagedIterable class.