Expando-style Map and/or @RawEntity support

[GoogleCodeExporter]

From thread:
- @Expando
http://groups.google.com/group/objectify-appengine/msg/fa8ec94382ec11b2
- @RawEntity
http://groups.google.com/group/objectify-appengine/msg/5028988170f34269

Original issue reported on code.google.com by spudbean on 4 Feb 2010 at 11:04

[GoogleCodeExporter]

Original comment by lhori...@gmail.com on 8 Feb 2010 at 5:38

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Will be very very useful.

Original comment by dab...@gmail.com on 4 Nov 2010 at 1:21

[GoogleCodeExporter]

See attachment for a possible patch. This was also sent to the mailing list, 
with the following comments:

A couple of explanations:

== Saving:

I had to change the code to make "path" for the savers into a dynamic
property that is passed along the way, as having a map in between a
saver and the root saver will change the path to something arbitrary.
That's the Path class and the changes to save(...) methods.

I decided to encode map values within the entity as
"mapfieldname.entryname.property1". I could also drop the mapfieldname
part, not sure what's the best way to do this. Comments? The current
way allows multiple maps in one class, without the map field name it
might look a bit more streamlined.

== Loading:

The name of the map entry to save to is currently passed as a field in
LoadContext, which is clearly an unsustainable hack (what about nested
maps within maps?). This has to be cleaned up, but I didn't want to
change all the save methods to pass around a (potential) map entry
name. Any idea?

I save the loaders name as "mapfieldname..property1",
"mapfieldname..propert2", and so on, note the double dots. Then when
loading a property called w.x.y.z where we cannot find a regular
loader, I look for w..y.z, w.x..z, i.e., all possible paths with one
element missing. Obvious problem is again that this doesn't support
nested maps (and might collide with strange field names ending in
dots).

So, generally speaking, this works, but will not work for nested maps
(the current code will silently fail and do horrible things for that
case). I think supporting proper nested maps will be hard given the
current code structure. If we'd want that, we'd have to move to a
system of nested Transmogs that each chop off a part of the path and
pass anything they cannot handle directly to sub-Transmogs for
embedded classes, maps, etc.

If you have a simple solution, please let me know.

Original comment by mar...@probst.io on 12 May 2011 at 7:01

Attachments:

• objectify-dynamic-properties.patch

[GoogleCodeExporter]

Thank you for sharing this patch.  Two quick comments: 

(1) I like having "mapfieldname" at the front of the entity key, because it 
essentially acts as a namespace to the map keys, and prevents entity key name 
collisions with other fields.

(2) I personally don't need support for nested maps.

Thanks again.

Original comment by g...@arakelian.com on 12 May 2011 at 7:17

[GoogleCodeExporter]

(1) I tentatively agree, though a solution would be to have actual declared 
fields take precedence over map entries.

(2) I think I came up with a better solution that would support nested maps 
(nested Transmogs that chop off prefixes). Will look into it tomorrow.

Original comment by martinpr...@google.com on 12 May 2011 at 7:38

[GoogleCodeExporter]

I have another patch that does support nested maps (through nested Transmogs) 
and removes the (hacky) "w..x.y.z" encoding. It still prevents map entries from 
having "." in them as that would make the lookup process too complex, IMHO.

I also added a whole bunch of tests for nested maps, embedded classes with 
maps, maps of primitive types, and some corner cases. I haven't considered 
performance at all in this.

I personally think this patch is a way cleaner solution than the earlier one.

Original comment by mar...@probst.io on 13 May 2011 at 11:29

Attachments:

• objectify-dynamic-properties-alternative.patch

[GoogleCodeExporter]

This should be fixed by r715 (though not yet incorporated in any release).

Original comment by mar...@probst.io on 18 May 2011 at 5:35

[GoogleCodeExporter]

Love the idea, found it after implementing my own solution using Entity.

Couple of issues.

- Clarity is key, 
  I would prefer an new annotation rather then reusing Embedded, as embed on a list/object does a serialization, is not searchable etc (basically semantically different).

- Flexibly, 
  Would be good if the delimiter is configurable, also the might want the prefix to be configurable or removable. If removable the question about have to deal with a nested map still remains. I would be happy if nested was disabled in certain cases.

- Functionality, 
  I haven't looked at the code closely enough but it doesn't seem to include querying for entities with property = value. I guess this isn't hard to achieve with the current query, but would need to know the prefix stuff.

A new annotation could solve these, at least the first two.
I don't mind Expando, but understand the objections.
How about something like @MapAsProperties(prefix="xzy",delimiter="$"), with the 
defaults being the field name and the delimiter being dot. Would like some way 
of specifying that there is not prefix and delimiter and then do conflict 
detection with other fields (reflection with caching could achieve this nicely).

Regards
Gary

Original comment by miller.garym on 28 Jul 2011 at 6:51

[GoogleCodeExporter]

Gary:

- Clarity:
@Embedded actually doesn't do Serialization on Lists, does it? It rather splits 
up the embedded object into collections for the individual fields, and querying 
works as expected. That's quite comparable to what this patch does.

- Functionality:
Querying should work. Fields of entities within a map are accessible through 
the prefix of the map, e.g. comparing on "mapField.mapKey.entityField". If it 
doesn't for you, please file a bug.

Regarding the flexibility, whether or not to have a prefix for the map fields 
was discussed on the mailing list, and this came out as the simpler solution 
(no conflicts). On the other hand, what'd be the use case?

Same for a custom delimiter. I don't really see the use case for that, but 
please  enlighten me if you have a good one :-)

In general, I'd rather leave it out than implement something of marginal value 
(as far as I can see) which might not even solve the real problem, and give us 
backwards compatibility headaches in the future.

Original comment by mar...@probst.io on 28 Jul 2011 at 7:41

• Changed state: Fixed

[GoogleCodeExporter]

Before releasing can we ensure that properties with dots can be queried. They 
can't from the admin console.

If Lists are queryable in GQL I don't know how. I've been playing with queries 
in the admin console with an number of interesting results.
1. (as stated) can't figure how to query contents of a collection. i.e. 
@Embedded list.
2. There are characters that are invalid in queries but valid property names. 
This includes . $ [ and ]. I haven't tested others _ is valid, assuming $ etc 
are also invalid.

My use case maybe over engineering the library, and I should just do 
substitutions on the keys. 

The optionality of the prefix came for me thinking about the expandos and how 
they would look in the datastore. It's probably not done often, but a single 
datastore can be used by Java and Python and it would be nice for compatibility 
in embedded Java maps and Python expando are compatible. Thinking about it 
valid queryable properties are probably also valid Python variable names.

Original comment by miller.garym on 29 Jul 2011 at 6:55

[GoogleCodeExporter]

There are several ofy test cases that cover querying properties with dots in 
them and they work as expected.  The GQL limitation is a defect of the admin 
console.  According to the GAE issue tracker, a fix is in progress.

Original comment by lhori...@gmail.com on 29 Jul 2011 at 7:04

[GoogleCodeExporter]

Gary: I see, the compatibility with Python would be an interesting feature. 
However I think if we want to do that, that's probably a bigger feature 
involving more changes. I think we should cross that bridge when we get there.

Original comment by mar...@probst.io on 29 Jul 2011 at 7:33