better support for item creation#28
Conversation
There was a problem hiding this comment.
@flack: wdyt? the thing is that we should never use short names. maybe we should make the type name expanded in the first place and shorten it in rendering?
There was a problem hiding this comment.
I haven't really checked yet, but was this function moved here or copied? Having the same funtion twice in the lib is of course very ugly, so a separate helper class would be a way out of this.
Originally, I had a JsonLd class in the repo that wrapped this functionality, but at some point, this became impractical. The big headache for any solution is imo to keep track of all possible namespaces. I would say that this screams for a global registry, but unfortunately, global state was declared evil at some point...
Expanding the names during type construction sounds like a good idea, though. If we can make sure that all rendering functions have access to the namespace map for their type, this might work. And I think it could also help us to avoid rendering unused namespaces, if we manage to track which namespaces have been "reverse resolved" during a given render call.
There was a problem hiding this comment.
maybe we even should push expanding namespaces out to the rdf drivers (so the xml driver would expand them while parsing xml, and the array parser would process the passed-in array).
in #29 i propose the topic of improving the rendering, i think this is related. but for now i think the approach would be to expand everything and then make sure the entity has its namespace mapping so it can declare them and map back url to prefix back when rendering.
|
@adou600 with these changes, a mapping as in |
There was a problem hiding this comment.
I'm not really familiar with the components used here, but I find it a bit confusing that for a class that has to do with doctrinePhpCr, an exception class from symfony is imported. Especially since in the code below, two time PHP's vanilla \RunTimeException is used, and then one time the RuntimException from symfony.
Like I said, I don't really know how this all fits together, but I was always under the impression that doctrine and symfony were two sepearate entities, i.e. that you could use doctrine (and therefore potentially this class) without symfony. If that's the case, then creating an interdependency with the exception class seems unnecessary
There was a problem hiding this comment.
totally right, this runtimeexception use statement should read use \RuntimeException, we create a needless dependency on a symfony component otherwise. and additionally it would be wrong to throw this. if any we could use this idea of extending a base exception for createphp to allow catching it specifically. don't think we need to, but if then it should be a separate PR.
|
oh wow, i think i will write a proposal what exactly we could do regarding the whole type/rev story and then do a PR to just fix that. this becomes too big. |
|
we should wait with this PR until #34 is done, then merge master into here and solve the undoubtly occuring merge conflicts and finalize object creation here. |
|
#34 seems to be merged. |
|
indeed. But right now, this PR cannot be automatically merged, I suppose it needs syncing with the latest changes |
|
indeed, it needs sync as i changed quite fundamental things about item creation to make it possible to do this PR in a clean way. but first we need to sort out the problems we have with non-entity collections and see if that has any impact: #37 |
@adou600 did refactorings to support object creation.
i open this PR and will start reviewing. more feedback welcome!
basically this adds: