You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@roller.apache.org by sn...@apache.org on 2007/07/20 23:59:07 UTC
svn commit: r558166 -
/roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java
Author: snoopdave
Date: Fri Jul 20 14:59:06 2007
New Revision: 558166
URL: http://svn.apache.org/viewvc?view=rev&rev=558166
Log:
Fixes ROL-1483 by moving the invalidate cache call before the delete template call. Apparently, with JPA, you don't want to attempt to use an object after you have deleted it.
Modified:
roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java
Modified: roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java
URL: http://svn.apache.org/viewvc/roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java?view=diff&rev=558166&r1=558165&r2=558166
==============================================================================
--- roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java (original)
+++ roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java Fri Jul 20 14:59:06 2007
@@ -84,12 +84,13 @@
if(getTemplate() != null) try {
if(!getTemplate().isRequired()) {
UserManager umgr = WebloggerFactory.getWeblogger().getUserManager();
- umgr.removePage(getTemplate());
- WebloggerFactory.getWeblogger().flush();
-
+
// notify cache
CacheManager.invalidate(getTemplate());
-
+
+ umgr.removePage(getTemplate());
+ WebloggerFactory.getWeblogger().flush();
+
return SUCCESS;
} else {
// TODO: i18n
Re: svn commit: r558166 -
/roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java
Posted by Craig L Russell <Cr...@Sun.COM>.
On Jul 20, 2007, at 3:27 PM, Dave wrote:
> On 7/20/07, Allen Gilliland <al...@sun.com> wrote:
>> 1. having JPA complain about using an object after it is removed
>> seems a
>> bit ridiculous. There are very valid reasons to do this, such as the
>> example below. If you really can't work around this then we
>> should plan
>> to change the various removeXXX() methods to return a safe copy of
>> the
>> deleted object that can continue to be used.
>
> JPA did not complain, but it did set fields of the object to null and
> the result was an NPE in the invalidate method. I think its debatable
> whether an object should continue to be valid after it is
> removed/deleted/whatever.
The JPA spec says that the non-generated fields/properties of removed
instances are as they were when the remove method was called.
But I'm still a bit confused by the requirement to access fields of a
deleted instance. You have said that the instance doesn't exist any
more, so what are you expecting of the instance?
If you really do need to know what the fields were when the entity
used to exist, I'd agree that you need to copy the fields you are
interested in before you delete the entity. Maybe use the clone()
method?
>
>> 2. moving the invalidation call before the removal is technically
>> incorrect because then if there is an error during the removal you've
>> mistakenly invalidated the template. while this isn't a huge
>> problem in
>> this particular situation it's still not a proper solution and i am
>> guessing that there is somewhere else in the code where doing the
>> same
>> thing could cause more problems.
>
> Yes. I think it's pretty safe to say that the JPA code needs more
> testing.
All code needs more testing...;-)
Craig
>
> - Dave
Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell@sun.com
P.S. A good JDO? O, Gasp!
Re: svn commit: r558166 - /roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java
Posted by Dave <sn...@gmail.com>.
On 7/20/07, Allen Gilliland <al...@sun.com> wrote:
> 1. having JPA complain about using an object after it is removed seems a
> bit ridiculous. There are very valid reasons to do this, such as the
> example below. If you really can't work around this then we should plan
> to change the various removeXXX() methods to return a safe copy of the
> deleted object that can continue to be used.
JPA did not complain, but it did set fields of the object to null and
the result was an NPE in the invalidate method. I think its debatable
whether an object should continue to be valid after it is
removed/deleted/whatever.
> 2. moving the invalidation call before the removal is technically
> incorrect because then if there is an error during the removal you've
> mistakenly invalidated the template. while this isn't a huge problem in
> this particular situation it's still not a proper solution and i am
> guessing that there is somewhere else in the code where doing the same
> thing could cause more problems.
Yes. I think it's pretty safe to say that the JPA code needs more testing.
- Dave
Re: svn commit: r558166 -
/roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java
Posted by Allen Gilliland <al...@sun.com>.
Dave,
a couple things concern me here.
1. having JPA complain about using an object after it is removed seems a
bit ridiculous. There are very valid reasons to do this, such as the
example below. If you really can't work around this then we should plan
to change the various removeXXX() methods to return a safe copy of the
deleted object that can continue to be used.
2. moving the invalidation call before the removal is technically
incorrect because then if there is an error during the removal you've
mistakenly invalidated the template. while this isn't a huge problem in
this particular situation it's still not a proper solution and i am
guessing that there is somewhere else in the code where doing the same
thing could cause more problems.
-- Allen
snoopdave@apache.org wrote:
> Author: snoopdave
> Date: Fri Jul 20 14:59:06 2007
> New Revision: 558166
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=558166
> Log:
> Fixes ROL-1483 by moving the invalidate cache call before the delete template call. Apparently, with JPA, you don't want to attempt to use an object after you have deleted it.
>
> Modified:
> roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java
>
> Modified: roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java
> URL: http://svn.apache.org/viewvc/roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java?view=diff&rev=558166&r1=558165&r2=558166
> ==============================================================================
> --- roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java (original)
> +++ roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/ui/struts2/editor/TemplateRemove.java Fri Jul 20 14:59:06 2007
> @@ -84,12 +84,13 @@
> if(getTemplate() != null) try {
> if(!getTemplate().isRequired()) {
> UserManager umgr = WebloggerFactory.getWeblogger().getUserManager();
> - umgr.removePage(getTemplate());
> - WebloggerFactory.getWeblogger().flush();
> -
> +
> // notify cache
> CacheManager.invalidate(getTemplate());
> -
> +
> + umgr.removePage(getTemplate());
> + WebloggerFactory.getWeblogger().flush();
> +
> return SUCCESS;
> } else {
> // TODO: i18n
>
>