jclouds-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ignasi Barrera <notificati...@github.com>
Subject Re: [jclouds/jclouds] Async interface changes for putBlob (#1114)
Date Thu, 06 Jul 2017 07:01:55 GMT
nacx commented on this pull request.

>        super(backend, backendType);
       this.consistencyModel = checkNotNull(consistencyModel, "consistencyModel");
       this.blobStore = checkNotNull(blobStore, "blobStore");
       this.utils = checkNotNull(utils, "utils");
       this.blobRequestSigner = checkNotNull(blobRequestSigner, "blobRequestSigner");
+      Binding<AsyncBlobStore> asyncBlobStoreBinding = injector.getExistingBinding(Key.get(AsyncBlobStore.class));
+      if (asyncBlobStoreBinding != null) {
+         asyncBlobStore = asyncBlobStoreBinding.getProvider().get();
+      } else {
+         asyncBlobStore = null;
+      }

Apologies for the late reply.

The idea is to create a base module class for all blobstore context modules. Something like:

public abstract class BaseBlobStoreServiceContextModule extends AbstractModule {
   protected void configure() {

   protected final Optional<AsyncBlobStore> provideAsyncBlobStore(Injector i) {
      Binding<AsyncBlobStore> binding = i.getExistingBinding(Key.get(AsyncBlobStore.class));
      return binding == null ? Optional.<AsyncBlobStore> absent() : Optional.of(binding.getProvider().get());
Then you can have the `BlobStoreContextImpl` directly inject the `Optional<AsyncBlobStore>`
like this. The previous module populates an optional to the Guice injector based on the presence
of a binding for the `AyncBlobStore`. Whether is present or not, the base module class configures
the optional in a generic way, and you can have it directly injected in your class, without
having to resolve the binding in the constructor:

public class BlobStoreContextImpl extends BaseView implements BlobStoreContext {
   private final Optional<AsyncBlobStore> asyncBlobStore;

   public BlobStoreContextImpl(@Provider Context backend, @Provider TypeToken<? extends
Context> backendType,
         Utils utils, ConsistencyModel consistencyModel, BlobStore blobStore, BlobRequestSigner
         Optional<AsyncBlobStore> asyncBlobStore) {
      this.asyncBlobStore = asyncBlobStore;
This configuration will cause the `BlobStoreContextImpl` to get an `absent()` injected for
all providers. The only thing that has to be done in those providers where we support async,
is to bind the async implementation. In Azure:
public class AzureBlobStoreContextModule extends BaseBlobStoreServiceContextModule {
   protected void configure() {

So, with the proposed change:

* We have a Guice module that configures the `Optional<AsyncBlobStore>` by default.
* Modules that support async only have to define the binding. A proper optional will be always
injected to the `BlobStoreContextImpl` (modules not defining the binding will get an `absent`

The only implication of this approach is that **all blobstore modules, of all providers**
(whether they use async or not) need to be changed to extend the new `BaseBlobStoreServiceContextModule`
module since it defines the Guice binding for the optional, which is now needed by the `BlobStoreContextImpl`
This implies small changes in all blobstore providers but IMO it is a cleaner implementation
and it makes sense to have a base class for all blobstore Guice modules where we can define
common behavior.

Does this make sense?

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
View raw message