Should Blobs and Buckets be stateless or keep a reference to a StorageService ?

@aozarov

Right now, it looks like our Blob and Bucket objects are stateless, in that they don't keep a reference to a StorageService object. What this means is, given we have the following:

StorageService storage = ... // Get a storage service (irrelevant how here)
Bucket bucket = storage.get("bucket-name");
Blob blob = storage.get("bucket-name", "blobname");

The only way to interact with these objects is by keeping a reference to the StorageService:

BlobReadChannel channel = storage.reader("bucket", "blob");
// or 
BlobReadChannel channel = storage.reader(blob); // I think?

There's no way to do:

BlobReadChannel channel = blob.getReadChannel(); // Or anything like this.

The purpose of this issue is to discuss whether or not these objects should hold a reference to the StorageService which would make things like blob.getReadChannel() or blob.delete() possible rather than always calling storage.getReadChannel(blob) and storage.delete(blob).

The benefits of this are friendlier-looking code (IMO at least....). The downsides are (I think) serialization becomes a bit more confusing, and what does it mean to send one "StorageService-aware" Blob into another StorageService method, ie:

StorageService storageA = ... // Authenticated with read permissions only.
StorageService storageB = ... // Authenticated with read *and* write permissions.
Blob blobA = storageA.get("bucket-name", "blob");
blobA.delete(); // This should fail: blobA is tied to storageA, which has read-only permissions.
storageB.delete(blobA); // Should this "override" the `StorageService`?

I have no idea what the right answer is, but wanted to open the floor for discussion.

/cc @aozarov @jboynes

I think that this is nice but consider it in the level of a syntactic sugar. I don't think an extra reference to Storage service would be a problem.

Two more things to consider are:

consistency with the Datastore API
This would either make the blobs/bucket mutable or operation would have to return a new copy

@jgeewax

I am taking over on this.
It would be nice for functional Bucket to return functional Blob objects (in methods like get or list) rather than BucketInfo objects (so that users do not need to do further conversions as new Blob(bucket.get("someBlob"))).

This is perfectly doable for get methods. For list method instead we use the underlying storage.list wich returns a ListResult<BlobInfo>. ListResult is an iterable that handles pagination and is also serializable. ListResult therefore takes only seriablizable types. Because of that we can not instantiate ListResult<Blob>. I see two solutions here given that we want to preserve the fact that storage works with serializable types (and I think we should):

Define a BlobList class which wraps ListResult and is an Iterable<Blob>
Let bucket methods return BlobInfo rather than functional Blob

I would personally prefer solution 1.
@jgeewax @aozarov @ajkannan thoughs?

I also think that Bucket should return (functional) Blob.

Another option would be remove the constrain that T needs to be Serializable but
document that the results param must be Serializable. and then pass results<Blob>
that keeps a transient reference to storageService and convert BlobInfo to Blob on the fly.

Now the question is how do we create storageService upon de-serialization of that implementation
of results. For that I think we can do the following (probably should be done as a prior PR):

I think it is reasonable to ask XXXFactory to be serialize.
with bothXXXOptions and XXXFactory serializable we can always construct the implementation
for XXX service.
We could pass both factory and option to the service impl (maybe even have part of the service contract) getFactory as we have now getOptions.

If we do so, we could technically even make the functional Blob and Bucket Serializable and the
keep the signature of ListResult as is...

What do you think?

I am not sure I would want functional classes as Blob, Bucket or XXX Service to be serializable. I think having serializable BlobInfo and BucketInfo is good as the classes are semantically containers or metadata. But for XXX Service, Blob and Bucker I struggle to find reasons for making them serializable.

Yes, we don't have to make or declare them as serializable, the other approach was to transform them on the fly and remove the requirement of having T serializable. Any opinion on that?