vlucas / Spot

[DEPRECATED - use v2] Simple DataMapper ORM for PHP 5.3+
75 stars 14 forks source link

Full type support #42

Closed pnomolos closed 10 years ago

pnomolos commented 11 years ago

Right now type support is good, but adding custom types is a bit of a chore. I think we need to extend the type interface to add a fourth method - serialize or something along those lines - to have the type mapped to the internal database structure (since that may be different than how it's handled elsewhere).

I'm thinking that we should have something like the following: Have all the Types extend from a new class - perhaps something like 'Noop'. It would be the basis for all types, and would define a _serialize and serialize methods, which would look something like the following:

public static function _serialize($value, $adapter = null)
{
    if (isset($serializeHandlers[$adapter][get_called_class()])) {
        return $serializeHandlers[$adapter][get_called_class()]($value);
    }
    return static::serialize($value);
}

// This would be overridden by the types to properly cast/serialize a value for DB storage.
public static function serialize($value) {
    return $value;
}

Then the adapters would call $type::_serialize($value, get_class($this)) to get the value.

I believe (if I haven't missed anything) that this should allow for enough flexibility to do what we want to do when saving back to the database.

pnomolos commented 11 years ago

Additional thoughts

We may/probably will need a matching set of _unserialize and unserialize methods to do the database back to type conversion. I also realized that my usage of coalesce was incorrect so I've changed the names accordingly.

There are also going to ramifications for the migrations currently in place for the MySQL adapter. Perhaps custom types should also specify a 'base type' from a white-list that has been chosen as being compatible across all systems? I'm thinking that such a list would consist of (at least) the following:

pnomolos commented 11 years ago

I've put together a basic Type class with the ideas of what I'm thinking: https://gist.github.com/pnomolos/5209527

vlucas commented 11 years ago

I do recognize the need for this in Spot, and I had an idea for a slightly different approach:

public static function internalType() {
    return 'string'; // Return internal type from the "base types" whitelist we use
}

So basically the type would return the internal type that needs to be used to store the value. The adapters would then be responsible for storing and retrieving all the internal types. The adapters would call: $type::internalType() and handle the value or cast it accordingly from there.

Would that work?

pnomolos commented 11 years ago

The problem with that is for the cases where a custom type is available natively for some datastores but not others. I'm thinking json as an example for Postgres, but it would be stored as a string for other RDBMSes. With your method I'm not sure how one would go about doing that (besides hardcoding an exception for the json type).

vlucas commented 11 years ago

Yeah, you're right - I guess we have to let the adapter handle it, and maybe throw an exception if it doesn't know how.

pnomolos commented 11 years ago

Yeah, something along those lines. I was also wondering - the current ::set() and ::get() methods take the Entity as a parameter as well, but none of them use that. I'm wondering if there was any reason to doing that?

vlucas commented 11 years ago

The original idea behind it was to check validaton rules on set, but it does have some other interesting applications. It could get any other information from the Entity and run some calculations. For instance, Rails has a counter_cache column type (example) that is a calculated value. It could also be used in cases where people might make a password type, and you need to retrieve the salt from the user or other information to build a hash. Basically, it could allow almost any custom type to do some pretty interesting things.

pnomolos commented 11 years ago

OK, that shouldn't be needed in dump and load, though, should it (I used the Ruby Datamapper project as an inspiration for naming)

vlucas commented 11 years ago

I'm not too keen on the names, but I won't complain about it too loudly. :)

pnomolos commented 11 years ago

I did think about serialize and unserialize but I felt that was too close to PHP's native methods and I didn't want any confusion. I could change them as well :)

pnomolos commented 11 years ago

@vlucas Is this closable now? :)

vlucas commented 11 years ago

Yeah, I think we have this solved, at least for now. If there are any issues that bubble up, we will tackle them on a case-by-case basis.

pnomolos commented 11 years ago

Actually I'm going to leave this open until tests for the new types are in :)