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:
```java
public abstract class BaseBlobStoreServiceContextModule extends AbstractModule {
@Override
protected void configure() {
}
@Provides
@Singleton
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:
```java
@Singleton
public class BlobStoreContextImpl extends BaseView implements BlobStoreContext {
...
private final Optional<AsyncBlobStore> asyncBlobStore;
@Inject
public BlobStoreContextImpl(@Provider Context backend, @Provider TypeToken<?
extends Context> backendType,
Utils utils, ConsistencyModel consistencyModel, BlobStore blobStore,
BlobRequestSigner 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:
```java
public class AzureBlobStoreContextModule extends
BaseBlobStoreServiceContextModule {
@Override
protected void configure() {
...
bind(AsyncBlobStore.class).to(AsyncAzureBlobStore.class).in(Scopes.SINGLETON);
}
}
```
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` injected).
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`
constructor.
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:
https://github.com/jclouds/jclouds/pull/1114#discussion_r125824375
