Skip to content
This repository has been archived by the owner on Jan 1, 2020. It is now read-only.

mixing mappers #58

Open
dbu opened this issue Nov 17, 2013 · 11 comments
Open

mixing mappers #58

dbu opened this issue Nov 17, 2013 · 11 comments

Comments

@dbu
Copy link
Collaborator

dbu commented Nov 17, 2013

it would be great to be able to use more than one mapper at the same time, so that i can edit objects coming from different sources. this could be different databases, but also different types like phpcr, orm and symfony translation strings (see liip/LiipTranslationBundle#21)

i guess the best approach would be to have a ChainMapper that decides by type name which of the mappers to use. we would need to make sure that when a request comes in from the frontend we also know the type.

if you would want to use several mappers of the same class for different databases, it would mean that each type may only come from one database, never be in one or another database. (e.g. doctrine orm you would make sure that each mapper contains a different subset of your orm mapped entities). the ChainMapper could support the mixed approach and cycle through all mappers knowing about a type name, but then when storing a new entity it does not know with which of the mappers to store it.

@flack
Copy link
Collaborator

flack commented Nov 17, 2013

I'm not entirely sure, but I think that the create case is actually not that difficult, because we get the typeof attribute from create.js and should be able to determine the correct mapper from it. What is more problematic IMO is delete, because on delete, create.js only sends us the identifier and nothing else, and we can't even do something like iterating over all mappers to see which one contains the entity, because identifiers would not necessarily be unique across different databases.

Anyways, here's the rough draft for a ChainMapper (lifted from #48 and slightly adjusted) to get the ball rolling:

abstract class ChainMapper implements RdfMapperInterface
{
    public function __construct(RdfMapperInterface $mapper)
    {
           $this->_defaultMapper = $mapper;
    }

    public function registerMapper(RdfMapperInterface $mapper, $typename = null)
    {
        if (is_null($typename))
        {
             $this->_defaultMapper = $mapper;
        }
        else
        {
              $this->_mappers[$typename] = $mapper;
        }
    }

    abstract function getMapperForObject($object);

    abstract function getMapperForRdfType($typeof);

    public function getPropertyValue($object, PropertyInterface $property)
    {
        return $this->getMapperForObject($object)->getPropertyValue($object, $property)
    }
    // and so on....
}

@dbu
Copy link
Collaborator Author

dbu commented Nov 17, 2013

afaik rdf identifiers must be globally unique (correct @bergie?)

so trying all possible mappers should be save if all behave as they should. its still a bit awkward, iterating over the mappers and trying to delete until there is no error, but should not be dangerous.

@flack
Copy link
Collaborator

flack commented Nov 17, 2013

You wouldn't have to go as far as trying to delete, trying to load (with getBySubject) would be sufficient: It that returns false, you pass to the next mapper.

If multiple mappers are a common use case, we might also go a bit further and add some sort of mapper ID to the generated identifier. F.x. we could have a ChainMapperAware interface with a method getMapperId or add the method to the RdfMapper interface directly. createSubject (or the code calling it) could then prepend the mapper ID. Figuring out which mapper to use would be trivial then, but I don't know how big the necessary changes would be, or if there are some hidden gotchas that I haven't thought of

@dbu
Copy link
Collaborator Author

dbu commented Nov 18, 2013

or the chainmapper could enumerate the mappers and prepend the number to the id :-) though that would not play well when you want semantical ids on your entities.

imho it would be the job of the mappers to make sure their id are unique. so i.e. the doctrine mapper should put the session name into the id if its not the default session. that way you won't have id collisions if the mapper in the lookup checks if there is this session information and if it matches the session it is connected to.

@flack
Copy link
Collaborator

flack commented Nov 18, 2013

True, having the prepend logic in the mapper would be a lot cleaner. We could then also allow passing the mapper ID on registerMapper, to give the developer full control.

I'm not sure how a mapper could make sure that its ID is unique. I mean, it doesn't know about how many and which other mappers are registered, or am I missing something?

@dbu
Copy link
Collaborator Author

dbu commented Nov 21, 2013

gave this some more thought: we could have the registerMapper method have a parameter for an id prefix to use with that mapper. then the individual mapper would not need to know anything about this, and using the same mapper class multiple times with different configuration is totally transparent. that way the chain mapper can handle everything transparently, except for createSubject and when storing new entities where it would need to figure out which mapper to use. i can think of 2 options:

  1. define the exception that must be thrown if a mapper can not handle an entity and do a try - catch until we find one.
  2. add a supportsObject($object) method, that would mean we can do createSubject with this, and a supportsCreation($type) meaning we can call prepareObject on this mapper with that type.

all other methods will only happen once the subject is resolved. the chain mapper will have to store the association between object and mapper in memory to know how to forward.

does that make sense to you? if so i can start to code something...

@dbu
Copy link
Collaborator Author

dbu commented Nov 25, 2013

@flack: what do you think?

@flack
Copy link
Collaborator

flack commented Nov 25, 2013

@dbu Sorry, this got a bit drowned in my mailbox. I think the id prefix approach sounds good, I'm somewhat unsure about the solutions you propose, but I can't think of better ones either right now.

Intuitively, I would go with solution 2, because it makes it more obvious what a mapper implementation has to actually do. Maybe we should call the second method supportsType instead, which seems a bit more generic. I wonder how supported types/objects are registered to the mapper BTW. Or are they determined automatically somehow (e.g. in Doctrine by seeing if ClassMetadata exists)? If it's the latter, then how would it work with the example you gave of having multiple instances of the same mapper with different configurations? Or do different configurations imply different storage backends?

@dbu
Copy link
Collaborator Author

dbu commented Dec 3, 2013

yeah, i think 2. is the more elegant option. we could call the methods supports and supportsCreate. a doctrine based mapper will check if the given object is managed by his doctrine instance. if there are several doctrine instances configured, each instance knows only about objects it manages (and it having several instances is the only reason to want several mappers in the first place). the supportsCreate case is because such objects will not yet be handled by doctrine and the mapper will have to figure out somewhat differently if it can support that (by checking if doctrine finds metadata for it). i think the separation of the cases has general value, for example if you need a custom mapper to create a new record for some of your models or other such scenarios. in the doctrine case, two mappers could load different instances of the same class from different repositories, but it would be possible to only create new instances through one repository. (say i have the same class loaded from a draft and a published table, but want to only create new objects in draft and have a custom workflow to push them into the published table)

@flack
Copy link
Collaborator

flack commented Dec 4, 2013

alright, so it seems like we're in agreement. Now the only thing that's missing is code :-)

@dbu
Copy link
Collaborator Author

dbu commented Dec 10, 2013

i will try to work on this while traveling to symfony con in warszawa...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants