refactor the reverse relation mapping handling to prepare for object creation

[dbu]

after the discussion in #28, i propose we do this refactoring first to have the "rev" handled in the place we actually need it.

as soon as @bergie has fixed bergie/create#122 we would then be able to finish #28 by using the actual information we have.

the tests run (at least locally for me) but there might still be mistakes somewhere.

and for the metadata of collections, this is a BC break, once merged projects will have to update their mappings.

some of this code is actually by @adou600 but it was too complicated to extract commits out of the other PR so i recreated that.

[dbu]

this will be needed when we create new entities as well, otherwise we would have the chicken-and-egg problem

[flack]

I won't have time to review this today (at least not before the evening), so I thought I'd send at least a "message received" message :-)

skimming over the changes, I kind of wondered if there is a reason why we don't initialize the array here, i.e. $rev = array();, but like i said, any serious comments will be tonight or tomorrow.

[dbu]

correct, we get a strict warning otherwise on using [] without initializing.

[flack]

Ok, I've got it working in my testbed now with remarkably little effort, and so far, everything I tried works. Good job!

Apart from the few comments I made, I'm good for merging this

[dbu]

comment was swallowed by github: we do need this i think as this is a mixed collection and we can't know the type of the child here.

[flack]

I wonder if we could make this a bit more flexible: If we would pass the $child object to getType, then each driver could implement its own strategy for determining the correct RDF type (and classname mapping would be the default implementation). Or do you think that this would be overkill?

[dbu]

the problem is that sometimes we will have the name and sometimes a real object. i just pushed a big commit that makes this explicit and lets the factory use the driver to translate. getTypeByName / getTypeByObject / getTypeByRdf

[dbu]

i updated the PR. what is left is

• question how we name the allowed child types in the collection
• how to bind a collection to real data when it is a mixed collection.

[flack]

I will do some testing later on, and I still have to think a bit about the mixed collection thing. But just as a quick notice: Travis tells me, the unittests are failing for this PR with two fatal errors like this:

PHP Fatal error:  Call to a member function getVocabularies() on a non-object in /home/travis/builds/flack/createphp/src/Midgard/CreatePHP/Entity/Collection.php on line 193

Do you see that in your setup, too?

[dbu]

unfortunately yes, seems i broke something. once we are clear about the naming i will go over the change again and clean it up and fix the tests. and make sure i actually test the stuff we are changing here...

[flack]

I wonder if this class map is really needed. I mean, each type has a mandatory "typeof" attribute that already holds the rdf name. So if this function is called with e.g. "sioc:post", wouldn't it be enough if the driver iterates over all types and returns the first one where typeof == 'sioc:post'?

[dbu]

we could, but then we would need to instantiate all types which could be a performance bottleneck. i think the way we do it now is not too bad - we could easily cache this map in the driver.

btw, we would never ask for sioc:Post but "http://rdfs.org/sioc/ns#Post" - we should not use short names here as we lack the context of the namespace mapping.

[dbu]

when you look at the implementation of the array driver, i build the map by reading the rdf name from the xml/array, its just in a different place. but a different driver might use a different strategy to build the list. if you insist we can also do a driver::getAllTypes method and build the list here and eventually cache this list (though the driver might be in a better position to judge if the cache is still valid)

[flack]

BTW, a quick janitorial question: Since you mention that this PR includes code from #28, can #28 be closed, then?

[dbu]

no, #28 does a lot more than this PR. here we create the base so that the other PR can do things in a proper way.

we have a hackday for the cmf at liip in zurich on thursday - i hope we can make progress both on this PR and on item creation then.

[dbu]

ok, so the tests work locally again and i think it makes more sense. i changed typeFactory::getType to be explicit whether we want by name or by object so the factory can implement that with the help of the driver.

also, i changed the property "type" attribute to "nodeType" and the types of collection to "childtypes" (resp "childtype" in xml, as there its just a list)

makes sense like this?

[flack]

Hm, I get the feeling that github swallowed some comments again, at least I have some email notifications that don't correspond to anything in the changeset view.

But AFAICT, we're in agreement, so I'm merging now. I still have to look at this getTypeBy[Name|Rdf|Object] thing, but that can be discussed separately.

[dbu]

i checked on all flack discussed an outdated diff 7 days ago Show outdated diff things if we missed anything and it seems to be fine.

thanks for merging. i think the separation of getTypeBy allows for more flexibility in the factory. we definitely have use cases for all 3