#1174 ✓resolved
David Masover

Need custom primitives (per-adapter) to support custom types.

Reported by David Masover | January 4th, 2010 @ 11:31 AM

I've just started hacking on the AppEngine adapter (dm-appengine). The AppEngine adapter does include some custom types, but for the most part, these are simply extensions of String. I was adding native support for keys.

See, AppEngine actually uses a type exposed as a native Java object -- AppEngine::Datastore::Key -- which includes an id or a name, and a possible parent key. The key of a given Entity is, as far as I can tell, immutable, and every Entity has one.

The important thing here is that it is, again, a native Java object. It really and truly IS a primitive, as far as this adapter is concerned. So, here's what I've come up with:

module DataMapper
  module Types
    [...]
    class Key < Type
      primitive AppEngine::Datastore::Key
      [...]
    end

    original_primitives = Property::PRIMITIVES
    Property::PRIMITIVES = (original_primitives.dup << AppEngine::Datastore::Key).freeze

    module Java
      module ComGoogleAppengineApiDatastore
      end
    end
    Java::ComGoogleAppengineApiDatastore::Key = Key

  end
end

Other types are somewhat easier, because they don't actually have to be used as keys. But this one is brutal. Yes, AppEngine::Datastore::Key.name resolves to "Java::ComGoogleAppengineApiDatastore::Key", probably a Java-to-JRuby thing. But more disturbing is how difficult it is to add a new primitive type at all.

I have no idea where to actually start fixing this, but that's where I'm at. It's not bad, as hacks go, but... forget public/private APIs, I'm pretty sure I shouldn't be changing a constant that was deliberately frozen like that.

I'm also thinking that while loading stuff into DataMapper::Types can sometimes be a good thing, Property::PRIMITIVES should be per-adapter (probably as a method), and there should be adapter-specific types. Some databases only natively support Strings; some databases natively support arbitrary Ruby objects, and I don't think dm-core needs to know about it -- it can provide types, sure, but it shouldn't say which are primitive.

Also: Why does child_key resolve the primitive, rather than the key type? From associations/many_to_one.rb:

def child_key
  [...]
    type = parent_property.send(parent_property.type == DataMapper::Types::Boolean ? :type : :primitive)
  [...]
end

It seems to me that the only reason to do this is because of things like Serial. And it seems like that could be fixed by instead using something like:

  property :id, Integer, :serial => true

...and maybe use Serial as syntactic sugar for that.

Comments and changes to this ticket

  • Piotr Solnica (solnic)

    Piotr Solnica (solnic) January 4th, 2010 @ 01:20 PM

    • State changed from “new” to “unconfirmed”

    +1 to that, however there's an idea of unifying Property and Type APIs. Take a look at http://wiki.github.com/datamapper/dm-core/roadmap which sounds like a better way

  • David Masover

    David Masover January 4th, 2010 @ 01:49 PM

    Sounds good, but I'm not sure it addresses this issue. Specifically:

    • There's a constant (frozen!) list of primitive types.
    • All types are declared in the DataMapper::Types namespace. Some types (especially primitives) really are per-adapter.
    • Adding a primitive type is clumsy.

    Specifically, the logic of looking for a constant inside DataMapper::Types seems to be crying out to be a hash of some sort. Something like:

    {
      ::Object => DataMapper::Types::Object,
      ...
      AppEngine::Datastore::Key => DataMapper::Types::Key
    }
    

    It would help -- I'd be able to override Property#primitive? and other things -- but I think this is still valid.

    Also, is there a better way (currently) to do what I'm doing? Here's the rest of the relevant code:

    http://code.google.com/r/ninja-dm-appengine-keys/source/browse/dm-a...

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) May 22nd, 2010 @ 01:38 AM

    • State changed from “unconfirmed” to “resolved”
    • Assigned user set to “Piotr Solnica (solnic)”

    @David: Piotr actually completed the Type/Property refactor and I think it addresses some of your problems. You can see the work on edge DM and/or with DM 1.0.0 RC2 which was released a few days ago.

    The new system basically just defines one core property (DM::Property::Objec) which all properties inherit from. Adapters are only required to implement serialization for that one class, but they can go further and support the properties defined in dm-core. In general I encourage adapters to store data in a native format if they can.

    In your specific case you can look over the Property subclasses in dm-core and dm-types, and inherit from something specific, or if there isn't anything, inheriting from DM::Property::Object.

    Please look over the work and if you have any suggestions for improvement, or any blockers please let us know.

    I am going to mark this ticket as resolved for now, since I think most of the issues you're having won't be applicable, or at least will be completely different from what you were dealing with. If you have any further issues please either add a comment here and I will reopen the ticket, or open a new ticket (although the latter is preferred with a link back to this ticket for backstory).

  • Piotr Solnica (solnic)

    Piotr Solnica (solnic) May 22nd, 2010 @ 05:47 AM

    With new Property API you can use whatever class you want as primitive, you can also override primitive? in your custom property types if you need to, so it seems like all the problems are gone.

    I've got one comment though - people shouldn't use custom property types to define mapping between ruby primitives and the datastore primitives as this should be handled by the adapter. For instance if you have a property DateTime (with primitive ::DateTime) and the datastore supports only Time as the primitive then it is the adapter which has to do the DateDate=>Time mapping and perform correct typecasting when persisting/retrieving values. Property API is used by the Resource layer, adapter layer is something that sits below it and shouldn't be mixed together.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Pages