2009/11/24 Andrus Adamchik <[email protected]> > I am ok with this as long as it doesn't break the ability for a to-many > relationship collections to (un)set reverse relationships when regular > collection add/remove methods are called. > > Sure
> > 1. Moving DO.addToManyTarget, etc to DataObjectUtils, taking Persistent >> instead of DO as an argument (actually I already did it locally) >> > > -1. I don't want to expose this API to the world, and also want to keep it > pluggable (which is not possible with a static class). > > We should sort out *what* should be pluggable here. Property injection in ever pluggable, and methods DataObject.setToOneTarget(...) (and I want it to be DataObjectUtils.setToOneTarget(Persistent, ...)) is just a helper method that calls those pluggable mechanisms. The need of setting reverse relationships is a paramdigm and has no need to become pluggable. Have you ever overridden DataObject.setToOneTarget()? Anyways, at the minimum, as i mentioned somewhere before, i'd like to see something helper like DataObjectUtils.readProperty(Persistent, String) for simple and unified access to DO's properties any any tier > Andrus > > > > > On Nov 24, 2009, at 12:42 PM, Andrey Razumovsky wrote: > > Okay, I've run into same situation when trying to allow PO subclasses in >> DC, >> but now I don't see context setting reverse arcs at user's change. The >> problem is in difference between CC and DC property change processing. >> Sorry >> about being so detailed around the code, but I don't see how i can explain >> it otherwise. >> >> So, what DC does when setting an arc: >> 1. Sets the value, which invokes context.propertyChanged() >> 2. GraphAction just registers property change, nothing else >> 3. Updates reverse arc >> 4. GraphAction just registers property change, nothing else >> >> What CC does when setting an arc: >> 1. Sets the value, which invokes context.propertyChanged() >> 2. GraphAction just registers property change and... >> 3. ...updates reverse arc >> 4. with those ugly threadlocals in CCGraghAction ensures there are no >> infinite cycles >> >> I am sure the first approach is much better. I suggest that we: >> 1. Moving DO.addToManyTarget, etc to DataObjectUtils, taking Persistent >> instead of DO as an argument (actually I already did it locally) >> 2. Make PersistentObjectHolder and other classes like that invoke >> context.propertChanged() should invoke those methods instead >> >> Ideally this will allow to: >> 1. Leave only OCGraphAction and get rid of DCGraphAction and CCGraphAction >> with its ugly threadLocal >> 2. Get rid of PropertyChangeProcessingStrategy >> 3. Maybe, make client and server persistent holders more similiar >> >> 2009/4/5 Andrus Adamchik <[email protected]> >> >> I committed the fix per CAY-1204. I encourage everybody using nested >>> contexts in ROP to test it and report any problems. >>> >>> The essence of the fix was to set a thread-local state for whatever >>> complex >>> sequence of events is going on when a relationship is changed, >>> determining >>> how the context should handle graph changes. Per new non-public >>> PropertyChangeProcessingStrategy enum, it is either of IGNORE, RECORD, >>> RECORD_AND_PROCESS_REVERSE_ARCS (default). This allows to handle 3 common >>> scenarios: >>> >>> "update from the DataChannel" >>> "update from the child context" >>> "update by the user" >>> >>> While this particular commit diverges client and server contexts further >>> from each other, contrary to our stated goal of merging them together, I >>> think the approach has a potential to become *the* way to do things >>> throughout the stack. Ideally this will eliminate the method pairs of >>> "doSomthing / doSomethingDirectly" from the API, which was a cornerstone >>> of >>> the graph management since Cayenne 1.0. >>> >>> Andrus >>> >>> >>> >>> On Apr 5, 2009, at 11:31 AM, Andrus Adamchik wrote: >>> >>> Cool. I got a further along in my investigation. I will put the details >>> in >>> >>>> a Jira and work on fixing it. >>>> >>>> Andrus >>>> >>>> >>>> On Apr 4, 2009, at 8:31 AM, Andrey Razumovsky wrote: >>>> >>>> Hi Andrus, >>>> >>>>> >>>>> I'm afraid I don't remember if it was done intentionally. I only know >>>>> that >>>>> the code is different from normal contexts' diff processing (that's why >>>>> I >>>>> feel a large refactoring is needed). Feel free to change the code, of >>>>> course. Alternatively, if you open a JIRA and post you JUnit there, >>>>> I'll >>>>> have a look. >>>>> >>>>> Andrey >>>>> >>>>> 2009/4/3 Andrus Adamchik <[email protected]> >>>>> >>>>> I am in the middle of debugging a problem with nested ROP contexts >>>>> >>>>>> losing >>>>>> arc changes when committing to a parent context. Since I was not >>>>>> involved in >>>>>> the ROP nested context work, I figured I'd post my thoughts here >>>>>> before >>>>>> I >>>>>> start changing the code. >>>>>> >>>>>> I noticed that per CAY-1119, there is a special subclass of >>>>>> ChildDiffLoader >>>>>> called CayenneContextChildDiffLoader that calls 'propertyChanged' on >>>>>> the >>>>>> parent context after syncing a simple property change. It seems like >>>>>> we >>>>>> need >>>>>> to do the same for relationships as well, to record arc changes in the >>>>>> parent diff list. >>>>>> >>>>>> Andrey, do you have any comments on that? I wonder if it was omitted >>>>>> intentionally. I will open a Jira (I think we don't have one for >>>>>> this), >>>>>> and >>>>>> add some tests with various relationships, but before I dig any deeper >>>>>> figured I'll need a sanity check. >>>>>> >>>>>> Thanks, >>>>>> Andrus >>>>>> >>>>>> >>>>>> >>>> >>>> >>> >> >> -- >> Andrey >> > > -- Andrey
