You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by Adam Hardy <ah...@cyberspaceroad.com> on 2008/05/01 00:42:23 UTC
Re: Fundamental flaw in Model-Driven?
Adam Hardy on 26/04/08 10:42, wrote:
> Jeromy Evans on 26/04/08 04:51, wrote:
>> Eric D Nielsen wrote:
>>> I've been investigating some interesting behavior regarding model-driven
>>> actions and found a past thread that covers the situation from late last
>>> year:
>>>
>>> http://www.nabble.com/ModelDriven-CRUD-validation-failure-still-causes-JPA-update-td12987242.html
>>> ...
>>>
>>> It would seem to be a way forward exists, however. Model Driven can be
>>> tweaked as follows:
>>> The model driven interceptor would need to
>>> a) capture the raw request parameters into some storage
>>> b) skip the second params interceptor
>>> c) create a copy of the getModel object (to ensure its detached from
>>> any
>>> persistence session -- need to make sure this works for other common
>>> persistence engines)
>>> d) run params/validation on this copy
>>> e) if no validation errors, run params on the orginal (still attached
>>> to the
>>> persistence session), and proceed as normal
>>> f) if validation errors, intercept calls to the getters for values in
>>> the raw
>>> request for redisplay -- need to worry about XSS issues here
>>>
>>> There's a lot of moving pieces, and I'm sure I'm missing some even more
>>> subtle interactions. However I do think something needs to be done.
>>> I'm interesting in tackling this, and am hoping for some feedback on
>>> the above
>>> outline of required changes.
>>>
>> Hi Eric,
>> I haven't had an opportunity to absorb your suggestion properly yet
>> but thought I'd mention I agree with your line of thinking that the
>> validation mechanism in particular needs to be improved. However, this
>> is a general problem that also applies to rich clients; that is
>> responsibility for rolling back changes to a model, and various
>> patterns have developed over the years. A temporary copy is a simple
>> implementation, however within a JPA-environment automatically
>> creating a clone is often infeasible or undesirable. For example, if
>> it's attached to a session, this process may cause hydration of the
>> entire object graph. Unless the framework is provided hints, it won't
>> know what to safely/efficiently clone.
>>
>> Having the framework maintain dirty flags or proxy for the model also
>> seems ineffective as the JPA provider performs the exact same task,
>> only better.
>>
>> The option to write straight to the model (or DTO) and performing
>> validation of the model (or DTO) is a distinguishing feature of Struts
>> 2, but also the source of such complications. Anyway, I don't have a
>> solution, but I do intend to start resolving the numerous validation
>> issues in JIRA in the near future and this one is the list.
>
> I also stumbled across this problem.
>
> One solution could be to use a TypeConverter to instantiate new,
> unmanaged entities during the ParamsInterceptor invocation.
>
> Such a TypeConverter could pass back instantiated, blank entities (with
> nothing but the ID) for ParamsInterceptor to collate, and against which
> ValidationInterceptor can operate. E.g. for param 'bean=69', the type
> converter instantiates Bean and calls setId(69)
>
> Once Validation succeeds, ModelDriven and ParamsInterceptor should be
> invoked again to update the managed entities.
>
> The unmanaged entities would just disappear out of scope and be garbage
> collected.
>
> Such a strategy would require modification to ParamsInterceptor to tell
> it to use the initial TypeConverter on the first call, but not on the
> second. Whether the algorithm that controls that is accessible for
> modification in the ParamsInterceptor or whether it is out-of-reach in
> OGNL somewhere, I'm not sure.
I am pulling in what I wrote earlier because I should correct what I said about
letting entities slip out of scope. Following this situation:
- Model-driven builds the model
- conversion to type and setting incoming HTTP param into Model
- validation occurs and fails
the problem was spurious JPA updates - which can be avoided by putting an
interceptor in between the Validation and the Workflow interceptors: all it
should do is call EntityManager.clear().
if (validationAwareAction.hasErrors()) getEntityManager().clear();
Using the JPA extended persistence context (not EJB), the clear() will eliminate
any changes to the model before JPA comes to flush to the db. I assume it would
work in an EJB container too.
Does that fit your situation, Eric?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Fundamental flaw in Model-Driven?
Posted by Adam Hardy <ah...@cyberspaceroad.com>.
Chase on 01/05/08 06:04, wrote:
>
>> Adam Hardy wrote:
>>
>> the problem was spurious JPA updates - which can be avoided by putting
>> an interceptor in between the Validation and the Workflow
>> interceptors: all it should do is call EntityManager.clear().
>>
>> if (validationAwareAction.hasErrors()) getEntityManager().clear();
>>
>>
>> Using the JPA extended persistence context (not EJB), the clear() will
>> eliminate any changes to the model before JPA comes to flush to the
>> db. I assume it would work in an EJB container too.
>>
>> Does that fit your situation, Eric?
> Then wouldn't the following work?
>
> public void validate() {
> //any actual validation code here
> if(hasErrors()) {
> //clear EntityManager or fail transaction
> }
> }
Yes it would work, although I haven't tried it yet.
Actually I think you have to do it this way.
My idea of a 'ValidationFailure' interceptor placed before the workflow
interceptor would not work when Action.validate() fails the validation.
NB it's counter-intuitive for WorkflowInterceptor to call Action.validate() - I
thought ValidationInterceptor did that. What was the rationale behind that design?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Fundamental flaw in Model-Driven?
Posted by Chase <ch...@osdev.org>.
> Adam Hardy wrote:
>
> the problem was spurious JPA updates - which can be avoided by putting
> an interceptor in between the Validation and the Workflow
> interceptors: all it should do is call EntityManager.clear().
>
> if (validationAwareAction.hasErrors()) getEntityManager().clear();
>
>
> Using the JPA extended persistence context (not EJB), the clear() will
> eliminate any changes to the model before JPA comes to flush to the
> db. I assume it would work in an EJB container too.
>
> Does that fit your situation, Eric?
Then wouldn't the following work?
public void validate() {
//any actual validation code here
if(hasErrors()) {
//clear EntityManager or fail transaction
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Fundamental flaw in Model-Driven?
Posted by Alexander Snaps <al...@gmail.com>.
Don't really have the context indeed, I haven't been looking into the
Model-Driven and had no clue where this is happening. I just think this is
more correct than doing everything within the tx demarcation. And indeed
many people don't know about the tx.begin - commit in a row thing. I also
keeps entities managed and hence still enable you to navigate association
lazily... whether or not state got flushed to the database or not. Just
wanted to share with you guys.Still my $.02, make them $.03 in the meantime
;)
On Thu, May 1, 2008 at 1:18 PM, Adam Hardy <ah...@cyberspaceroad.com>
wrote:
> Alexander Snaps on 01/05/08 12:02, wrote:
>
> Entity myEntity = em.find(Entity.class, someId);
> >
> > oldValue = myEntity.getSomeField(); // this is only for further down in
> > the
> > mail ;)
> > myEntity.setSomeField(someValue);
> >
> > if(!hasError) {
> > EntityTransaction tx = em.getTransaction();
> > tx.begin();
> > tx.commit(); // Flushes state to DB
> > } else {
> > // do whatever _outside_ of any transaction, like re-ask user to
> > re-enter
> > data
> > }
> >
> > >
> > > Indeed calling the clear operation on the entity manager also results
> > > in
> > >
> > > > all managed entities becoming detached. Beyond many operations on
> > > > the current persistence context might result in the entity manager flushing
> > > > state to the database. Yet having it behave differently because of
> > > > updating managed entities outside of the tx also sometimes have the
> > > > persistence context not behave as one would expect...
> > > >
> > > > I'm afraid I can't quite follow you there. (Use of 'beyond'?) The
> > > clear
> > > operation is exactly what I want to do - with the result you mention.
> > > This
> > > is the start of a request (HTTP param conversion and calling setters
> > > on
> > > model properties), with a new EntityManager, so anything that goes
> > > wrong
> > > should cause everything to be cleared.
> > >
> > >
> > > An example of what I mean is if now you'd query for every Entity with
> > someField == oldValue (see above), you'd still get the same instance as
> > myEntity, which would have myEntity.getSomeField().equals(oldValue) ==
> > false!
> >
>
> Yes true but you're not really making use of the struts interceptor
> framework here, which should keep from doing things like that.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
--
Alexander Snaps <al...@gmail.com>
http://www.jroller.com/page/greenhorn
http://www.linkedin.com/in/alexandersnaps
Re: Fundamental flaw in Model-Driven?
Posted by Adam Hardy <ah...@cyberspaceroad.com>.
Alexander Snaps on 01/05/08 12:02, wrote:
> Entity myEntity = em.find(Entity.class, someId);
>
> oldValue = myEntity.getSomeField(); // this is only for further down in the
> mail ;)
> myEntity.setSomeField(someValue);
>
> if(!hasError) {
> EntityTransaction tx = em.getTransaction();
> tx.begin();
> tx.commit(); // Flushes state to DB
> } else {
> // do whatever _outside_ of any transaction, like re-ask user to re-enter
> data
> }
>>
>> Indeed calling the clear operation on the entity manager also results in
>>> all managed entities becoming detached. Beyond many operations on the
>>> current persistence context might result in the entity manager flushing
>>> state to the database. Yet having it behave differently because of
>>> updating managed entities outside of the tx also sometimes have the
>>> persistence context not behave as one would expect...
>>>
>> I'm afraid I can't quite follow you there. (Use of 'beyond'?) The clear
>> operation is exactly what I want to do - with the result you mention. This
>> is the start of a request (HTTP param conversion and calling setters on
>> model properties), with a new EntityManager, so anything that goes wrong
>> should cause everything to be cleared.
>>
>>
> An example of what I mean is if now you'd query for every Entity with
> someField == oldValue (see above), you'd still get the same instance as
> myEntity, which would have myEntity.getSomeField().equals(oldValue) ==
> false!
Yes true but you're not really making use of the struts interceptor framework
here, which should keep from doing things like that.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Fundamental flaw in Model-Driven?
Posted by Adam Hardy <ah...@cyberspaceroad.com>.
Alexander Snaps on 01/05/08 12:02, wrote:
> On Thu, May 1, 2008 at 12:49 PM, Adam Hardy <
> ahardy.struts@cyberspaceroad.com> wrote:
>
>> Alexander Snaps on 01/05/08 11:30, wrote:
>>
>>>> Using the JPA extended persistence context (not EJB), the clear() will
>>>> eliminate any changes to the model before JPA comes to flush to the
>>>> db. I
>>>> assume it would work in an EJB container too.
>>>>
>>>>
>>> I'm not really sure I get the point your making here, but if you just do
>>> not
>>> want the EntityManager not flush to the database, shouldn't you rather
>>> just
>>> update the model outside of the transaction.
>>>
>> I mean persistent entities in the extended persistence context. Within or
>> outside a transaction shouldn't affect the EntityManager, which will still
>> flush updates to the database when closed.
>
>
>
> Not if no tx was ever started...
>
> Entity myEntity = em.find(Entity.class, someId);
>
>
> oldValue = myEntity.getSomeField(); // this is only for further down in the
> mail ;)
> myEntity.setSomeField(someValue);
>
> if(!hasError) {
> EntityTransaction tx = em.getTransaction();
> tx.begin();
> tx.commit(); // Flushes state to DB
> } else {
> // do whatever _outside_ of any transaction, like re-ask user to re-enter
> data
> }
Sorry to doubt you, but are you 100% sure on that? I could check for myself but
I don't have half an hour at the moment.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Fundamental flaw in Model-Driven?
Posted by Alexander Snaps <al...@gmail.com>.
On Thu, May 1, 2008 at 12:49 PM, Adam Hardy <
ahardy.struts@cyberspaceroad.com> wrote:
> Alexander Snaps on 01/05/08 11:30, wrote:
>
> >
> > >
> > > Using the JPA extended persistence context (not EJB), the clear() will
> > > eliminate any changes to the model before JPA comes to flush to the
> > > db. I
> > > assume it would work in an EJB container too.
> > >
> > >
> > I'm not really sure I get the point your making here, but if you just do
> > not
> > want the EntityManager not flush to the database, shouldn't you rather
> > just
> > update the model outside of the transaction.
> >
>
> I mean persistent entities in the extended persistence context. Within or
> outside a transaction shouldn't affect the EntityManager, which will still
> flush updates to the database when closed.
Not if no tx was ever started...
Entity myEntity = em.find(Entity.class, someId);
oldValue = myEntity.getSomeField(); // this is only for further down in the
mail ;)
myEntity.setSomeField(someValue);
if(!hasError) {
EntityTransaction tx = em.getTransaction();
tx.begin();
tx.commit(); // Flushes state to DB
} else {
// do whatever _outside_ of any transaction, like re-ask user to re-enter
data
}
>
>
> Indeed calling the clear operation on the entity manager also results in
> > all
> > managed entities becoming detached. Beyond many operations on the
> > current
> > persistence context might result in the entity manager flushing state to
> > the
> > database. Yet having it behave differently because of updating managed
> > entities outside of the tx also sometimes have the persistence context
> > not
> > behave as one would expect...
> >
>
> I'm afraid I can't quite follow you there. (Use of 'beyond'?) The clear
> operation is exactly what I want to do - with the result you mention. This
> is the start of a request (HTTP param conversion and calling setters on
> model properties), with a new EntityManager, so anything that goes wrong
> should cause everything to be cleared.
>
>
An example of what I mean is if now you'd query for every Entity with
someField == oldValue (see above), you'd still get the same instance as
myEntity, which would have myEntity.getSomeField().equals(oldValue) ==
false!
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
--
Alexander Snaps <al...@gmail.com>
http://www.jroller.com/page/greenhorn
http://www.linkedin.com/in/alexandersnaps
Re: Fundamental flaw in Model-Driven?
Posted by Adam Hardy <ah...@cyberspaceroad.com>.
Alexander Snaps on 01/05/08 11:30, wrote:
>>
>>
>> Using the JPA extended persistence context (not EJB), the clear() will
>> eliminate any changes to the model before JPA comes to flush to the db. I
>> assume it would work in an EJB container too.
>>
>
> I'm not really sure I get the point your making here, but if you just do not
> want the EntityManager not flush to the database, shouldn't you rather just
> update the model outside of the transaction.
I mean persistent entities in the extended persistence context. Within or
outside a transaction shouldn't affect the EntityManager, which will still flush
updates to the database when closed.
> Indeed calling the clear operation on the entity manager also results in all
> managed entities becoming detached. Beyond many operations on the current
> persistence context might result in the entity manager flushing state to the
> database. Yet having it behave differently because of updating managed
> entities outside of the tx also sometimes have the persistence context not
> behave as one would expect...
I'm afraid I can't quite follow you there. (Use of 'beyond'?) The clear
operation is exactly what I want to do - with the result you mention. This is
the start of a request (HTTP param conversion and calling setters on model
properties), with a new EntityManager, so anything that goes wrong should cause
everything to be cleared.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Fundamental flaw in Model-Driven?
Posted by Adam Hardy <ah...@cyberspaceroad.com>.
Alexander Snaps on 01/05/08 11:30, wrote:
>>
>>
>> Using the JPA extended persistence context (not EJB), the clear() will
>> eliminate any changes to the model before JPA comes to flush to the db. I
>> assume it would work in an EJB container too.
>>
>
> I'm not really sure I get the point your making here, but if you just do not
> want the EntityManager not flush to the database, shouldn't you rather just
> update the model outside of the transaction.
I mean persistent entities in the extended persistence context. Within or
outside a transaction shouldn't affect the EntityManager, which will still flush
updates to the database when closed.
> Indeed calling the clear operation on the entity manager also results in all
> managed entities becoming detached. Beyond many operations on the current
> persistence context might result in the entity manager flushing state to the
> database. Yet having it behave differently because of updating managed
> entities outside of the tx also sometimes have the persistence context not
> behave as one would expect...
I'm afraid I can't quite follow you there. (Use of 'beyond'?) The clear
operation is exactly what I want to do - with the result you mention. This is
the start of a request (HTTP param conversion and calling setters on model
properties), with a new EntityManager, so anything that goes wrong should cause
everything to be cleared.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Fundamental flaw in Model-Driven?
Posted by Alexander Snaps <al...@gmail.com>.
>
>
>
> Using the JPA extended persistence context (not EJB), the clear() will
> eliminate any changes to the model before JPA comes to flush to the db. I
> assume it would work in an EJB container too.
>
I'm not really sure I get the point your making here, but if you just do not
want the EntityManager not flush to the database, shouldn't you rather just
update the model outside of the transaction.
Indeed calling the clear operation on the entity manager also results in all
managed entities becoming detached. Beyond many operations on the current
persistence context might result in the entity manager flushing state to the
database. Yet having it behave differently because of updating managed
entities outside of the tx also sometimes have the persistence context not
behave as one would expect...
Not sure how useful this was now, yet my $.02 ;)
--
Alexander Snaps <al...@gmail.com>
http://www.jroller.com/page/greenhorn
http://www.linkedin.com/in/alexandersnaps