Preserve same entity in nested pass-through (_elseNested). #342
Conversation
* Fixes #338 (see test `shouldHandleUnmatchedLiteralsAndEntitiesInElseNestedSource`; e4c6fe5). * Doesn't work with partially handled entities (see test `shouldHandlePartiallyUnmatchedLiteralsAndEntitiesInElseNestedSource`). * It should either coordinate with `Entity` w.r.t. starting/ending entities. * Or separate pass-through entities from explicitly specified entities.
blackwinter
changed the title
_elseNested).
There was a problem hiding this comment.
Nice solution. The ensuing bug is no blocker for the PR, since the PR does fixes the issue and does not introduce new bugs (you just anticipate issues which may or may not arise in the real world) - so move forward.
Re discussing fix for the "ensuing bug": will do that later.
| o.get().literal("Hawaii", "Aloha"); | ||
| if (!coordinatesWithEntity) { | ||
| o.get().endEntity(); | ||
| o.get().startEntity("USA2"); |
There was a problem hiding this comment.
If we decide to leave it in, yes. But for the time being it was only intended to illustrate the issue (and the different results, depending on the potential resolution).
Well, if the use case is to "change some literals or entities in a record while passing the remainder of a record untouched", then it's almost useless if it can't deal with partially handled entities. And we'd almost certainly need the first solution ("coordinate with For the supposedly limited use case (incompletely?) presented in #338, it would probably suffice. But I'm hesitant to call this feature ( |
|
Thinking more about this, I'm not so sure there can even be any straightforward solution. Because, after all, the input entity and the output entity are totally independent from each other. So if they happen to be named differently in the morph, there's nothing for us to merge. (Not to mention how Which is to say, I'm leaning more towards option 2 (separation) now; at least that'd be consistent. Or should it be configurable? Anyway, maybe we should really look at the actual use case... @hagbeck: What is it you're trying to achieve here? Can you provide us with a complete example or test case? We may have to accept the fact that this is an experimental feature and the details will have to be fleshed out in practice. |
This reimplemets commit f009e28: "Data which is handled by metamorph rules will NOT be passed through (hence the aptly named "_else"). If you want to use data in the morph AND pass it through, you have to add an explicit rule for this, as usual." This behaviour is not the default, though. It can be achieved by this morph rule: <entity name='$name' sameEntity='true' reset='true' flushWith='$name' flushIncomplete='true'>" where $name is the name of the entity you want to treat. See #338.
|
@blackwinter I commited a proposal that shows my understanding of the issue. I assume that a mix of handling and dropping data is a common scenario when one wants to also pass some data untouched. |
|
@dr0i: As long as we're clear that this
@hagbeck, @fsteeg: Please feel free to chime in. I still think that we would benefit from an actual use case. |
|
I've tried to build an example use case in https://github.com/hagbeck/metafacture-sandbox/tree/master/else-nested-entities The current branch seems to solve the issue. |
|
Thanks! I've turned your example into a test so we avoid future regressions. Please let us know if you encounter further issues or come up with additional and/or more complex use cases. We're going to let this sit for a while. |
E.g. JSON-LD keywords. Fixes #343.
Escape feedback char in _else data.
|
We've been using this for months in OERSI without issues. I think we can merge this and release it with 5.2.0 (see #318). @blackwinter: Do you want to assign @dr0i for final review & merge? |
|
Great to hear, thanks. |
Fixes #338 (see test
shouldHandleUnmatchedLiteralsAndEntitiesInElseNestedSource; e4c6fe5).Doesn't work with partially handled entities, though (see test
shouldHandlePartiallyUnmatchedLiteralsAndEntitiesInElseNestedSource):Entityw.r.t. starting/ending entities.coordinatesWithEntityset totrue.separatesFromEntityset totrue.I'd like a review of the actual fix and also some feedback concerning the ensuing bug (which remedy would be preferrable? is this considered a blocker for the PR to move forward?).