Thanks for raising this Oscar, I've never really understood why the viewers are responsible for validation but never took the time to dive into it.
Cheers, Jeroen On Tue, Aug 6, 2013 at 8:15 AM, Dan Haywood <[email protected]>wrote: > Oscar, > > You've convinced me... we should keep our entities consistent across the > layers. More discussion inline below.... > Dan > > > On 5 August 2013 17:00, GESCONSULTOR - Óscar Bou <[email protected] > >wrote: > > > > But I presume there was a reason that you put your call to > > > AbstractXMSDomainObjectRepositoryAndFactory#.doFindByProp(...) in this > > > closure. Can you tell me explain further for me? > > > > > > The reason is that we are adapting our current viewer (user interface) to > > Isis. There is common functionality to all screens (such as filtering, > > searching, saving a domain object, refresh, etc.) that work with methods > > defined on a base repository > (AbstractXMSDomainObjectRepositoryAndFactory). > > > > As all Isis interaction is made inside those methods, we surrounded all > > that logic within a Isis transaction, thinking that would allow as to > avoid > > custom web session filters, etc. > > It was the "closest" place to the code interacting with Isis. Can you > > explain a bit the advantages about why should it be done on a "higher > > layer" - the web session - (perhaps if not done through a web session > > filter it can introduce some "session management" problems (seeing > > different object sets or different Isis sessions depending on the > "session" > > returned by Tomcat - in different states/update time - , etc.). > > > > > Part of the reason is in keeping the responsibilities of the various > implementations consistent with each other. The Wicket viewer and the RO > viewer now both set up transactions before doing any work, in other words > we are using "session/transaction in view" design. > > I only added the IsisTransactionFilterForRestfulObjects very recently; > until then it was also relying on the lazy creation of transactions down in > the PersistenceManager. But what it also means is that any queries for > objects are done without a transaction in place (I guess that the JDO/DN > and/or the RDBMS creates one implicitly). I could see in the future that > we might want to allow the user to use isolation level 3 rather than level > 1, ie guarantee that a transaction exists and is present. > > > > > > > > > At the moment we only ever apply such validation at the UI layer, > > mediating > > > between the viewer layer and the domain object model/layer. > > > > This was not our expectation. And I think this is not the expected > > behavior in other "validation" frameworks. I'll further detail this. > > > > You've convinced me... we should keep our entities consistent across the > layers. I don't know that I really thought otherwise, was just aware that > this was an area where the current implementation of Isis was less than > ideal. > > [snip] > > > > So, at least, I could expect: > > - After any action execution, all domain objects are in compliance with > > all defined invariants. > > - During action execution, I could understand that objects are not > > conformant with invariants. > > - If a property is modified on a viewer, when the object changes are > > saved/persisted (sent to Isis and, through it, to the database) all > > invariants are in place. > > > > > I've been mulling on how we can implement easily implement this. I'm not > sure that FrameworkSynchronizer is the right place, because that will be > called multiple times throughout the xactn. Instead, in IsisTransaction > itself we have several enlistXxx() methods, already called by the JDO > objectstore/FrameworkSynchronizer, originally added for auditing and > publishing purposes. > > So, it seems it would be relatively trivial to additionally spin through > all enlisted objects, and fire the validation checks on them, just prior to > commit. This would be a bit like a pre-commit trigger. > > Another benefit of this design is that we know that at this point JDO/DN > will have locked these objects exclusively, so there is no chance of the > objects being changed under our feet. > > I think this is what the ticket you've raised, ISIS-487, should now > implement. > > > > > > If we were on a CQRS architecture, all property changes would be > > considered a command and the DDD domain invariants would be guaranteed > > before and after executing a command. And perhaps that would be the > meaning > > of an "Isis Transaction", differenciating it from a database transaction > or > > an "application layer transaction" (such as in SAP, where it's referred > as > > an operations requiring work in one or more than one views, as in a > > wizard)). > > > > I don't see an IsisTransaction as having a different scope to a database > transaction; it's a single short-lived transaction in force for the > duration of the invocation of an object action (or property edit). But > what it can/does provide is this ability to find the enlisted objects and > do useful things to/with them. > > Appreciate you spending the time to make these arguments; good stuff. > > Dan >
