KeyValuePair needs to store a Blob not a generic Object

[Matt Giuca]

The current KeyValuePair class has its 'value' as an Object. This will only work for Objects that have been specially marked for JDO persistence. We want to allow any plain-old-Java serializable objects to go in here.

Therefore, change the 'value' field to a com.google.appengine.api.datastore.Blob, and manually serialise and deserialise the Objects going into and out of the public interface to this class.

[Matt Giuca (mgiuca)]

I've got a problem with this, and it kind of casts doubt on the whole "database classes are the same ones exposed to the client" concept. I tried changing the 'value' field of KeyValuePair to a Blob, but of course, I am now getting a gwtc compile error -- there is no Blob class in JavaScript!

Really, the client version of the KeyValuePair should have an Object (already deserialised), while the database/server version should have a Blob (serialised for the database storage). With the current architecture, there is no way for me to ever use any type in the database which is not a standard Java type (such as Blob, Text, User, etc). This makes me nervous about the whole concept. Perhaps we should just be providing a much simpler API (similar to the dev-api) which simply lets you query individual properties of each object, rather than taking a copy of the whole thing.

In any event, I don't know how to solve this present problem. Any thoughts?

[Prageeth Silva (prageethsilva)]

OK now I remember why I came up with the concept of having two classes for each model class.
In cases as mentioned above, I have a way around IF we continue to have the current concept.

What I would do is, basically have two extra retrieve and update (basically get and set object itself) methods in the ModelDataClass interface. This way, the ModelClass (exposed to the client) can actually have the Object type value. However, the ModelDataClass implementation of KeyValuePair (KeyValuepairData) would have an extra field which is the blob.

So all you do is, override the two methods mentioned above that handles the translation from Object to Blob and vice versa. And, in other model classes, we can simply return the same value without doing any operations?

This hides the blob from the client and the client classes can have any fields we want; but the data classes will have JDO persistence fields.

I can do this tonight, but Matt would have to wait till I finish implementing this; I'll take about 30mins to completely implement it and get it working that way.

But this still brings rise to the issue of having two tables per model class. I personally think this adds more features to the model and we can live with having 10 empty tables that are never used (not much of a space or performance issue).

[Matt Giuca (mgiuca)]

I see I see. I move this discussion to bug #744771.

[Scott Ritchie (sritchie)]

Related to bug #774677

Ive found that KeyValuePairs currently don't work. At compile time (when pressing the populate datastore button) i get:

00:02:15.472 [ERROR] javax.servlet.ServletContext log: Exception while dispatching incoming RPC call com.google.gwt.user.server.rpc.UnexpectedException: Service method 'public abstract java.lang.String au.edu.unimelb.csse.mugle.client.DataTestService.populateDatastore()' threw an unexpected exception: org.datanucleus.exceptions.NucleusUserException: Field "au.edu.unimelb.csse.mugle.server.model.KeyValuePairData.value" is declared as a reference type (interface/Object) but no implementation classes of "java.io.Serializable" have been found!
...
at au.edu.unimelb.csse.mugle.server.DataTestServiceImpl.populateDatastore(DataTestServiceImpl.java:53)

which is the line:
"pm.makePersistentAll(users)"

Since the relationships are owned between User, UserGameProfile and then KeyValuePair this comes up here - but is a bigger (unrelated) problem

Our keyvaluepair storage wont work as is. I tried to change the Object being stored from a straight Object to Serializable (error with Object instead of Serializable is essentially the same)

However what i think is happening here based on the error above and the example at:
https://code.google.com/appengine/docs/java/datastore/jdo/dataclasses.html#Serializable_Objects

It's searching OUR code for classes which implement Serializable - and not finding any, which is why there's an error.
So i think we'll need to serialize the objects ourselves.

[Scott Ritchie (sritchie)]

Also note you won't see this error as of revision 119 as the relations are now unowned, but the problem still remains and will arise in the future.

[Prageeth Silva (prageethsilva)]

I have completely removed Serializable from the KeyValuePairData class.
Now it's set to take in an Object (given that only serializable objects can be passed to the server anyhow).
In the setValue(Object) method, it's automatically converted to a byte array (please note that this is valid for any Object, whether it's serializable or not).
Similarly getValue() automatically converts the byte array to an Object and returns the object.
This preserves the client API interface as it takes an Object and returns and Object.
In terms of performance, I doubt it'll be an issue as I've used Streams to convert the object back and forth and the overheads can be negligible.

However, another major bug has appeared (most probably due to the some runtime error after modifying the custom annotations).

[Prageeth Silva (prageethsilva)]

The above mentioned bug has also been fixed, it was a bug in the ModelWrapper casting.

[Prageeth Silva (prageethsilva)]

Just realised that saving a Blob or byte[] does not work during runtime. As far as I can see, I've added a second test after populating the database. It basically retrieves the correct KeyValuePair object with respect to the KeyValyePair-Key via a query, but for some reason it's value (at the moment set as Blob) seems to be null.

[Scott Ritchie (sritchie)]

As of revision 326 i have reason enough to believe that the current implementation of Key Value Pairs works

Ive tested the putting and getting of a Serializable custom class KVPtester in the datastore.
The output text for testing the KeyValuePair storage can be seen in stdout when the Populate Datastore button is pressed

This however is put and gotten from the server side implementation. Im not sure if the Class will survive transportation across the GWT RPC layer if transported from the client side, but it is my understanding that any object marked Serializable/IsSerializable will - and that our problem was the actual storage of the serializable class in the KVP itself

[Prageeth Silva (prageethsilva)]

I agree that the server side is working as well.
The way I implemented (that's in r326) is by converting the object into a byte array and that's only possible with serializable AFAIK. However, the current interface given to the student uses Object and I think we should check if the object received is either Serializable/IsSerializable before trying to store it. If it's not, we can simply throw and exception as they have been told that RPC only works for those types.

[Matt Giuca (mgiuca)]

Note: Opinion means "won't fix; due to a matter of opinion". Basically don't use that status. I'm setting back to Fix Committed.

I am not as confident as you that it will survive the transport over the RPC. I just tested it on the dummy server (which is using the same client code) and sadly, a user-defined type seems not to work over RPC. But we can open a different bug on that. This one (the database one) appears fixed.