Joerg Heinicke wrote:
On 30.03.2004 00:24, Marc Portier wrote:
Towards the larger user-base I think we should unify views, clean out the code and provide appropriate documentation.
So, here is an attempt to bring the sheep back into the heard, and lay down a better foundation for the RepeaterBinding implementation as well as its use (and thus its documentation)
+1
looking into the <fb:repeater> syntax we see these mappings back in the names of the attributes:
@id points to the repeater
@parent-path points to the matching contextbean
the repeater's rows each map to one item found at @row-path within the contextbean
the nested child-widgets will map @id of widget to @path of actual property.
I found the (at that time) wb:repeater syntax really confusing when using it the first time. There were 5 attributes and there usage was not obvious to me: @id, @parent-path, @row-path, @unique-id, @unique-row-path. That was why I liked Antonio's move of the last two attrs into elements so much.
What's still confusing IMO is the name parent-path - the path of which parent? For consistency this should also be named @path as for all other binding elements, the name @row-path avoids any further confusion I think.
thx for chiming in with my name change proposal upfront :-)
[case 1]
DESCRIPTION:
- the form handling (actions or events) will not reorder the rows
- no rows will be added or deleted
- item-properties could be sparsly bound to row-widgets (meaning: not all properties are mapped to actual widgets since they're not part of the foreseen end-user interaction scheme, of course the ones that were not bound should not be cleared on save just because the form-model isn't holding a value for them)
In this case we don't really speak of identity of the items, and we don't really need it either: the position of each item in the list (it's index) serves as an implicit identifier, and since no adding/deleting or reordering on the rows can happen we know that the row-indexes will just keep in sync.
PROPOSED SYNTAX: <fb:repeater id="rep-id" parent-path="." row-path="item"> <!-- no fb:identity --> <!--required elements for this case --> <fb:on-bind>...</fb:on-bind> <!--optional elements for this case --> <fb:on-insert>..</fb:on-insert> <fb:on-delete>..</fb:on-delete> </fb:repeater>
EXPECTATION DETECTION:
- no nested <fb:identity> element tells us we don't have an explicit identity, and we should just trust the index as a identity correlator.
STRATEGY on 'save': - foreach row in repeater - use the nested 'on-bind' binding
A straightforward extension seems to be - allow for add/delete of rows (by adding <on-insert> <on-delete>) - at the cost of not surviving sparse binding of items.
Then the Strategy becomes: - foreach row in repeater with matching index in the items - use the nested 'on-bind' binding - for excess rows in the repeater - use on-insert and on-bind - for excess items in the context - use on-delete
-o0o-
[case 2] (aka current SimpleRepeaterBinding) DESCRIPTION: - no fluff, just stuff - the simplicity of 'load' ported over to 'save'
- we can just remove all the items before binding them from the form (thus no expected support for sparse binding) - form-model can be changed at will
PROPOSED SYNTAX: <fb:repeater id="rep-id" parent-path="." row-path="item" clear-items-on-save="true"> <!-- no fb:identity --> <!--required elements for this case --> <fb:on-bind>...</fb:on-bind> <fb:on-insert>..</fb:on-insert> <fb:on-delete>..</fb:on-delete> </fb:repeater>
EXPECTATION DETECTION: @clear-items-on-save="true"
STRATEGY on 'save': - remove all items using 'on-delete' - do as extended-case-1
REMARK:
This strategy obviously needs to delete and re-create all items. Thus it requires the presence of <fb:insert> and <fb:delete> bindings to operate.
However, taking into account the current operation, and some typing-economics for the most logical/frequent use we should probably agree on some default behaviour for those and request explicit removal of the insert and delete bindings by adding the empty configuration element.
Maybe it's just to late for thinking, but what's the difference between 1 (especially with "straightforward extension") and 2?
none!
In fact I hope to be wrapping all these up in one imlementation algorithm: if we add the current indexes (of row and item) as arguments to the isIdentical() method in the IdentityBinding, then we could have a default identity-matching thingy implementing the interface in those cases where there was no fb:identity...
but that will become clear as I start coding
the counter-argument would be that detecting the strategy at build time and then installing the most appropriate algorithm probably will lead to simpler code, easier extensibility at the cost of some more classes...
[case 3] DESCRIPTION: - items have an explicit identity and can be sparsly bound - form-model can add/remove/reorder the rows - the sequence editing of the rows need to be reflected onto the items
PROPOSED SYNTAX: <fb:repeater id="rep-id" parent-path="." row-path="item" > <!--required elements for this case --> <fb:identity>...</fb:identity> <fb:on-bind>...</fb:on-bind> <fb:on-insert>..</fb:on-insert> <fb:on-delete>..</fb:on-delete> </fb:repeater>
EXPECTATION DETECTION: <fb:identity> is present
STRATEGY on 'save': - backup all items to a list OriginalItems - foreach row in the repeater - if row-identity-match in OriginalItems - move item back into the context and bind - else - use on-insert and on-bind to create and bind - for the items still left in the OriginalItems - use the on-delete
in fact, this approach makes a question pop up to the hibernate and ojb experts out there:
will this moving to buffer collection just work or am I to consider some constraints while doing so?
-o0o-
[case 4] (aka current RepeaterBinding)
DESCRIPTION:
- items have an explicit identity and can be sparsly bound
- form-model can add/remove but should not allow reordering of the rows
- the original sequence of the items is to be maintained at all times (on-insert really is more of an on-append: no new ones can be inserted in between the old ones)
PROPOSED SYNTAX: not clear yet, I tend towards:
<fb:repeater id="rep-id" parent-path="." row-path="item" row-path-insert="newItem"> <!--required elements for this case --> <fb:identity>...</fb:identity> <fb:on-bind>...</fb:on-bind> <fb:on-insert>..</fb:on-insert> <fb:on-delete>..</fb:on-delete> </fb:repeater>
EXPECTATION DETECTION:
fb:identity plus
the presense of a @row-path-insert seems to indicate that the new ones need to go into a different space then the exisiting ones, and thus denying the approach expressed in case3
(i.e. even if it would read the same as row-path! so there would be a difference between explicitely stating it as the same, and defaultly assuming it is the same)
STRATEGY on 'save': - foreach row in repeater - if identity match found in items - bind to that - add it to the set of updatedItems - else - add it to some list of rowsToInsert - run through items - if NOT found in updatedItems - add to list of toDeleteItems (ndx will do) - register the on-insert as factory - foreach row in rowsToInsert - create and bind it - foreach ndx in toDeleteItems in reverse order - use on-delete to remove/mark
3 vs. 4: IMO @row-path-insert makes the strategy not really obvious. Wouldn't it be better to specify explicitely whether reordering is allowed/needed or not?
yeah, that is the alternative which still makes me doubth
For completion:
implementation of the above assumes the refactoring of the current identity approach towards the earlier mentioned IdentityBinding interface.
see: http://marc.theaimsgroup.com/?l=xml-cocoon-dev&m=107955523607012&w=2
(Joerg, if you're ok, I'll start doing it when this discussion (if any) converges)
Of course, you only remove another lame excuse for starting with my diploma thesis :)
oh, joerg, sorry man, I didn't know it was that important to you ;-)
While writing up this proposal I end up questioning a number of (historically chosen) names that could change:
- repeater/@parent-path --> repeater/@path for consistency with the order bindings
Have I written this somewhere above? :-)
- repeater/@row-path --> repeater/@item-path since it is pointing to items, not rows
Indeed, so it's reasonable.
but it kinda requires me adding the above to the docos
which makes me think: I'm planning on only patching cforms (not woody) for this... we still don't have cforms docs in the wiki, right?
- on-insert is actually more of a on-create ('insert' seems to exclude 'append', while create is more nutral to exactly where the newly created is added, it is also more in sync with the actual factory registration IMHO )
Not that important, but I'm also ok with this.
- repeater/@row-path-insert --> repeater/@new-item-path which would be syncing up with the last two changes and arguments
What do we need @row-path-insert/@new-item-path for if we have on-insert?
well, that is part of the confusion of on-insert
on-insert isn't really used to insert stuff, it just registers a factory to create nodes when that would be needed
this means that it doesn't make sense to add path information on that element, since it will not actively use that. (it is the repeater deciding and doing that)
in any case, we added the new-item-path already some time ago (called row-path-insert) for those use cases were the new rows in a repeater needed to be added into a separate section
this use seems to collide with this case4, so that's why I figured to let it be it's trigger
regards, -marc= -- Marc Portier http://outerthought.org/ Outerthought - Open Source, Java & XML Competence Support Center Read my weblog at http://blogs.cocoondev.org/mpo/ [EMAIL PROTECTED] [EMAIL PROTECTED]
