Model classes cannot have server-only types

Bug #744771 reported by Matt Giuca
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MUGLE
Fix Released
High
Prageeth Silva

Bug Description

In the current implementation of model classes, only client-visible types may be used. This is because all of the model classes are exposed to the client, so fail to compile if a non-client type is stored (such as Blob or User).

See discussion on bug #744035.

Tags: model
Revision history for this message
Matt Giuca (mgiuca) wrote :

Responding to Prageeth's comment on bug #744035:

> 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.

OK. But I still see a problem: the base class is going to need to have the wrapped type. For example, if the Data class has a Blob, then there will need to be a corresponding Object (deserialised) in the wrapped type. Therefore, the Data class will have a field for BOTH the deserialised Object AND the Blob. True, the deserialised Object could be null in the Data class, but then we're talking about a type A with a field that is never null, and a subclass B where the same field is always null -- technically B violates the constraint of A , because you cannot use B in place of A.

At this point we are into a very bad design: we are using subclasses ONLY to avoid code duplication and not for any other practical benefit which means they are being misused.

Couple that with the two-tables problem ... I still think we should just avoid this and go with a simpler model.

Rather than replacing this with two separate classes, I think it would be best if we just provide an API to read each individual field (after all, we have APIs for setting individual fields, why not getting as well?)

> 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).

I don't like this approach. The database should be treated a bit like the user interface -- it should not expose nasty things in the code. Much like a user interface, you should be able to refactor the code without having to change the underlying data representation. It isn't cool if our data store has a bunch of tables just there because of a way we wrote the code.

Revision history for this message
Matt Giuca (mgiuca) wrote :

OK I didn't do a very good job quoting there.

Revision history for this message
Prageeth Silva (prageethsilva) wrote :

OK, imagining we scrap everything off and have all our model classes to be server side ONLY; and providing api to access every field it will be very difficult to implement the UI.

I'm not saying it's impossible, but without doing filed duplication it's going to be very messy.

For example, the admin wanting to look at the list of users would have been easy if we had a User object to be passed to the client. We would simply return a list of User objects.

However, if we are to do single requests for each field, it'll be so many queries.

Once again, a work around this would be to build a table (or any HTML representation) in the server side ServiceImpl and pass it back. This would however limit us in using client side gwt feature in building a dynamic page.

I can't really think of a better way to get around the issue of not having to pass objects to the client side.

Any thoughts would be helpful.

Revision history for this message
Prageeth Silva (prageethsilva) wrote :

This has been fixed by having two different classes now in server and shared. ModelDataClass no longer inherits (extends) ModelClass. ModelClass is now an abstract class (earlier was an Interface) that has fields mapping to the field in the data class. Therefore, we can now have server only types in the data class and that shouldn't affect the shared classes.

Changed in mugle:
status: Triaged → Fix Committed
Matt Giuca (mgiuca)
Changed in mugle:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.