You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by Eric D Nielsen <ni...@MIT.EDU> on 2008/04/24 17:43:07 UTC
Fundamental flaw in Model-Driven?
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
First I outline the problem, then a possible framework provided solution and a
request for feedback before I start trying to generate a patch.
In brief:
Configuration:
ModelDriven with validation is configured
Hibernate/JPA is being used (OpenEntityManagerInView/OpenSessionInView is
something of a red-herring)
Sequence of Events:
Model is loaded from DB
Params (second params) inject parameters into the model
Validation fails
Input reached
session.flush occurs (either from closing the session in OSIV pattern, OR from
an implicit flush before an explicit query or a lazy-load query)
dirty and invalid object written to the DB
In the thread referenced, his solution was to preload all collections backing
form elements -- thus avoiding the lazy-load queries after dirtying the object.
(This is why I say a lot of the discussion in that thread about the flush in
OEMIV is a red-herring. It could cause the problem, but wasn't the cause of
the problem)
A lot of the other workarounds suggested in the thread seem very unclean as its
something that should be taken care of by the framework.
To my eye, there is a fundamental flaw in Struts 2 validation (and type
conversion). Under the model-driven paradigm, validation conditions need to be
viewed as invariants, and either all fields should be set or none -- it should
be atomic. There is also a violation of the SRP going on by making the model
objects responsible for holding and redisplaying invalid values to the web
form.
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.
Thank you for reading,
Eric Nielsen
---------------------------------------------------------------------
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 26/04/08 20:29, wrote:
> Adam Hardy wrote:
>> Chase on 26/04/08 11:16, wrote:
>>>> 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.
>>>>
>>> If it was a blank (just id) entity then you'd lose the ability to
>>> validate "num1 > num2" where num1 was set by the params interceptor
>>> but num2 came from the database.
>>
>> Sorry Chase, can you elaborate a little? You're talking about one of
>> the S2 validations? I didn't see that in the list.
>>
> The number of params might be less than the number of properties in the
> entity. A benefit of using a managed object to perform validation is
> that the validation interceptor can compare the params AND the db
> populated properties. Imagine an objeiect with two properties, minPrice
> and maxPrice. If a form is submitted that modifies the minPrice don't
> you want to be able to validate it to make sure that it doesn't exceed
> the value of maxPrice? But then you have the problem of the original
> poster which basically comes down to, if you give the params &
> validation interceptors a live managed entity then it might end up
> causing unwanted database updates. I'm just saying that I don't think
> the problem should be fixed in such a way as to lose the ability to
> validate params against the db values of other properties.
I really like that second solution to the problem where you would use a dynamic
entity wrapper to cache the incoming HTTP version of the data (we had something
a little like that on our last project).
My reaction would be to refuse to accept the problem:
- in the example with minPrice vs maxPrice, on grounds of KISS, I would simply
put the original values in hidden fields rather than try validations relying on
retrieval from the database. For security, to stop the client being able to
cheat by manipulating the hidden fields with some Firefox plug-in, you would
also verify such business rules in the entity manager as well.
---------------------------------------------------------------------
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:
> Chase on 26/04/08 11:16, wrote:
>>
>>>
>>> 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.
>>>
>> If it was a blank (just id) entity then you'd lose the ability to
>> validate "num1 > num2" where num1 was set by the params interceptor
>> but num2 came from the database.
>
> Sorry Chase, can you elaborate a little? You're talking about one of
> the S2 validations? I didn't see that in the list.
>
The number of params might be less than the number of properties in the
entity. A benefit of using a managed object to perform validation is
that the validation interceptor can compare the params AND the db
populated properties. Imagine an objeiect with two properties, minPrice
and maxPrice. If a form is submitted that modifies the minPrice don't
you want to be able to validate it to make sure that it doesn't exceed
the value of maxPrice? But then you have the problem of the original
poster which basically comes down to, if you give the params &
validation interceptors a live managed entity then it might end up
causing unwanted database updates. I'm just saying that I don't think
the problem should be fixed in such a way as to lose the ability to
validate params against the db values of other properties.
I think there are 3 possible ways to fix the problem.
1) Basically something along the lines of what the original poster
stated, make a copy of the managed object to use for validation. The
thread he linked to mentioned BeanUtils.copyProperties(tempBean,
yourBean) but I think Jeromy was right when he warned about possibly
pulling in all related entities. I think serialization would be a better
technique to copy the object (ObjectOutputStream -> Pipe ->
ObjectInputStream).
2) Enhance the model driven and validation interceptors. Instead of
pushing the result of getModel() directly on the stack the model driven
interceptor could first wrap the entity in some type of dynamically
generated EntityWrapper class and push the wrapper on the stack. The
EntityWrapper would leverage the real entity object to read the existing
db values but cache all property changes until some type of flush
operation occurred. The validation interceptor, upon finishing
validation with no action errors, would call the flush method on the
wrapper which would cause all modified properties to be transfered to
the managed entity.
3) This basically comes down to wanting everything to succeed or nothing
to succeed (no partial completion). That's what transactions are for.
Make versions of the interceptors that integrate with the transactions
used by the persistence engines.
-Chase
---------------------------------------------------------------------
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 26/04/08 11:16, wrote:
>
>>
>> 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.
>>
> If it was a blank (just id) entity then you'd lose the ability to
> validate "num1 > num2" where num1 was set by the params interceptor but
> num2 came from the database.
Sorry Chase, can you elaborate a little? You're talking about one of the S2
validations? I didn't see that in the list.
---------------------------------------------------------------------
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>.
>
> 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.
>
If it was a blank (just id) entity then you'd lose the ability to
validate "num1 > num2" where num1 was set by the params interceptor but
num2 came from the database.
-Chase
---------------------------------------------------------------------
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
Re: Fundamental flaw in Model-Driven?
Posted by Adam Hardy <ah...@cyberspaceroad.com>.
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>.
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.
---------------------------------------------------------------------
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 Bob Tiernay <bt...@hotmail.com>.
I'm wondering if something like OVal (a great validation framework btw) can
help out here:
See probe mode:
http://oval.sourceforge.net/#d0e739
--------------------------------------------------
From: "Adam Hardy" <ah...@cyberspaceroad.com>
Sent: Saturday, April 26, 2008 6:34 AM
To: "Struts Developers List" <de...@struts.apache.org>
Subject: Re: Fundamental flaw in Model-Driven?
> Jeromy Evans on 26/04/08 04:51, wrote:
>> 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.
>
> Pondering over this issue during breakfast, I thought I'd check out how
> some other frameworks do it and I found some weak descriptions criticising
> struts for 'architectural issues' which I've been hearing for years but
> now I've forgotten what they actually are. I think in most cases they
> referred to Struts 1, but does anyone know of a good architectural
> critique that goes over such issues?
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
---------------------------------------------------------------------
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>.
Jeromy Evans on 26/04/08 04:51, wrote:
> 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.
Pondering over this issue during breakfast, I thought I'd check out how some
other frameworks do it and I found some weak descriptions criticising struts for
'architectural issues' which I've been hearing for years but now I've forgotten
what they actually are. I think in most cases they referred to Struts 1, but
does anyone know of a good architectural critique that goes over such issues?
---------------------------------------------------------------------
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 Jeromy Evans <je...@blueskyminds.com.au>.
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.
regards,
Jeromy Evans
.
---------------------------------------------------------------------
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 Eric D Nielsen <ni...@MIT.EDU>.
Another possible solution I want to try exploring (but unfortunately its
Hibernate specific) is making a custom version of the validator interceptor:
Inject the session into the interceptor
Call session.readOnly(entity,true) on failed validation
If I'm reading things correctly this should
a) keep the session alive (versus issuing a rollback), keeping the object
attached and allow lazy loading
b) stop any of the dirty data in the object from getting pushed back on
implicit
or explicit flushes
In fact I might flip set readOnly(entity,true) on entrance to the validator
interceptor and then set it to false on success. This should allow the
validators of submitted version stored quantities to proceed --
for instance the classic example of Order value can't exceed X, when adding a
new LineItem -- the validation might trigger a lazy load of the LineItems, so
the entity must be "persistence read-only" before any validation starts to
avoid writing dirty values back.
This still feels like a hackish solution ( and one that probably can't be
distributed/incorporated by/into Struts 2's official jars ), but it looks
promising...
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 Chase <ch...@osdev.org>.
Could you do what you propose with a custom interceptor stack? Make your
first call to getModel() return a serialized copy of the model object
and the second call return the managed instance. Internally the action
would maintain the reference to the managed instance from the time of
the first getModel() or prepare() call.
<interceptor-stack name="JPAparamsPrepareParamsStack">
<interceptor-ref name="exception"/>
<interceptor-ref name="alias"/>
<interceptor-ref name="servletConfig"/>
<interceptor-ref name="params"/>
<interceptor-ref name="prepare"/>
<interceptor-ref name="modelDriven"/>
<interceptor-ref name="params"/>
<interceptor-ref name="fileUpload"/>
<interceptor-ref name="checkbox"/>
<interceptor-ref name="staticParams"/>
<interceptor-ref name="conversionError"/>
<interceptor-ref name="validation">
<interceptor-ref name="i18n"/>
<interceptor-ref name="workflow">
<param name="excludeMethods">input,back,cancel</param>
</interceptor-ref>
<interceptor-ref name="modelDriven"/>
<interceptor-ref name="params"/>
<interceptor-ref name="checkbox"/>
<interceptor-ref name="staticParams"/>
</interceptor-stack>
-Chase
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
>
> First I outline the problem, then a possible framework provided solution and a
> request for feedback before I start trying to generate a patch.
>
> In brief:
> Configuration:
> ModelDriven with validation is configured
> Hibernate/JPA is being used (OpenEntityManagerInView/OpenSessionInView is
> something of a red-herring)
>
> Sequence of Events:
> Model is loaded from DB
> Params (second params) inject parameters into the model
> Validation fails
> Input reached
> session.flush occurs (either from closing the session in OSIV pattern, OR from
> an implicit flush before an explicit query or a lazy-load query)
> dirty and invalid object written to the DB
>
> In the thread referenced, his solution was to preload all collections backing
> form elements -- thus avoiding the lazy-load queries after dirtying the object.
> (This is why I say a lot of the discussion in that thread about the flush in
> OEMIV is a red-herring. It could cause the problem, but wasn't the cause of
> the problem)
>
> A lot of the other workarounds suggested in the thread seem very unclean as its
> something that should be taken care of by the framework.
>
> To my eye, there is a fundamental flaw in Struts 2 validation (and type
> conversion). Under the model-driven paradigm, validation conditions need to be
> viewed as invariants, and either all fields should be set or none -- it should
> be atomic. There is also a violation of the SRP going on by making the model
> objects responsible for holding and redisplaying invalid values to the web
> form.
>
> 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.
>
> Thank you for reading,
> Eric Nielsen
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org