Regarding FLUID-3824, preserve policy

[Antranig Basman]

Thanks for this report, Harris. I think that, at length, we need to 
consider that this is a bug in the "preserve" policy that was 
implemented in the options merging system during this last release 
cycle. At present, the directive "preserve" simply causes the right-hand 
("user") reference to the model to be copied into the component option 
without change - this also "inadvertently" has the effect of what we 
would call "replace".
This is at odds with the expectations of the pager component, or at 
least the demo, which expects that models are capable of participating 
in merging their VALUES like the rest of the options structure. 
Certainly, before "preserve" was invented this used to happen by 
default, since the model was not treated any differently to the rest of 
the configuration.
I debated with Colin this afternoon whether models really SHOULD 
participate in merging. I think we decided that, at length, they 
probably should - there are two shades of this issue, firstly that, for 
example, in components like the pager, the "model" really is a form of 
"options" despite being modifiable at runtime. Secondly, in light of our 
discussion in IRC a week back about the potential for implementing 
components with "modifiable options" - an idea for this was to attach to 
such components a changeApplier targgeted at its options structure, 
together with some schematic information regarding which options were 
successfully modifiable at runtime.

In any case, meeting these two requirements at the same time now 
requires a somewhat tricky implementation. Rather than the simple 
reference copying that we have now, we need the following sequence:
i) copy RHS to temporary object
ii) clear LHS, and copy RHS to LHS using "copyModel" semantics
iii) merge temporary onto RHS using the same
The final two seem to have a natural implementation in terms of 
changeApplier "ADD" and "MERGE" requests targetted at "" although it has 
to be mentioned that this is an area of incompletely thought-out 
functionality and is pending a set of successful ideas about
http://issues.fluidproject.org/browse/FLUID-3674
which we will not be able to address for the 1.3 release. However the 
idiom for changes to "" seem at least somewhat stable.

I will commit fixes to Fluid.js and DataBinding.js now which seem to 
resolve FLUID-3824 together with some test cases to show the expected 
behaviour for complex cases of "preserve". This however has the effect 
of creating a dependence on DataBinding.js for some users that was not 
there before. In particular, a use of the "preserve" strategy will now 
demand an implementation of fluid.makeChangeApplier. This "seems 
reasonable" since without the use of the data binding infrastructure, 
there is not really any value to the "model reference stability 
semantics" that the "preserve" policy implements. However this is 
something that needs to be communicated to users and has required the 
test cases to be redistributed. Although I'm worried about making a 
change this fundamental close to code freeze, the impact of it looks to 
be small from what I can see right now.

Cheers,
Antranig.

On 11/11/2010 09:04, Wong, Harris wrote:
> Hi,
>
> I was working on FLUID-3824 (http://issues.fluidproject.org/browse/FLUID-3824) and I came across one problem with the preserve policy
>
> After some debugging, the Sakai demo (infusion/src/webapp/integration-demos/sakai/html/pager-site-setting.html) fails to run because the pageSize is not set.  I found that it's not set because the default values from Pager.js line 765-769 didn't get merged with the demo's constructor model.
>
> Pager.js line 765-769
>   model: {
>              pageIndex: undefined,
>              pageSize: 10,
>              totalRange: undefined
>          },
>
> site_setting_members.js line 86-88
> model: {
>              pageIndex: 3
>          },
>
>
> The merge should become
>   model: {
>              pageIndex: 3,
>              pageSize: 10,
>              totalRange: undefined
>          },
>
> but is actually returning
>   model: {
>              pageIndex: 3
>          },
>
> I think this is caused by the preserve policy which Fluid.js condition going false on line 536.
>
> Should I fix this by changing:
> site_setting_members.js line 86-88
> model: {
>              pageSize: 10,
>              pageIndex: 3
>          },
> or is there another way to do it regarding to the preserve policy?
>
>
> Thanks,
> Harris
> _______________________________________________________
> fluid-work mailing list - fluid-work at fluidproject.org
> To unsubscribe, change settings or access archives,
> see http://fluidproject.org/mailman/listinfo/fluid-work