rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Sharples <p.sharp...@bolton.ac.uk>
Subject Re: [DISCUSS] Apache Rave 0.18 Release Candidate
Date Wed, 05 Dec 2012 10:41:48 GMT
On 05/12/2012 10:17, Jasha Joachimsthal wrote:
> On 5 December 2012 10:41, Ate Douma <ate@douma.nu> wrote:
>
>> On 12/05/2012 03:39 AM, Franklin, Matthew B. wrote:
>>
>>> I haven't reviewed the release, but I agree that we should push forward
>>> with the known issue. I would also like to see if this is an issue in
>>> earlier releases.
>>>
>>>   Well, I checked two earlier versions (0.15 & 0.16). I cannot reproduce
>> the error with those, but because I cannot delete a user at all, regardless
>> having a shared page or not.
>> When I try to delete any user I get an exception (see below).
>> Maybe that is related to H2 database only, but worrisome anyway.
>> At least with 0.18 we now *can* delete users :)
>>
> I'm sure we were able to delete a new user in earlier versions because
> that's one of the steps in the integration tests. This new user doesn't
> have widgets on his page and doesn't have a shared page, which may be the
> cause of the issue.
>

Looks like a logic error.  In JpaPageRepository the method "deletePages" 
operates on a resultset obtained from the named query 
"JpaPageUser.GET_BY_USER_ID_AND_PAGE_TYPE".

That query is used in a few other places - to get all the pages for a 
user in the page controller for example (to show owned & shared pages 
alongside each other).  However, when deleting pages it should probably 
check to see if the page is owned by this user or not before actually 
deleting it.

Updating the "deletePages" method...

     public int deletePages(String userId, PageType pageType) {
         List<Page> pages = getAllPages(userId, pageType);
         int pageCount = pages.size();
         for(Page page : pages) {
             if(page.getOwnerId().equals(userId)){
                 delete(page);
             }
         }
         return pageCount;
     }

...is one workaround as we are cycling around the records anyway. I'll 
do some more testing.

Paul

>> Anyway, I agree not making RAVE-859 a release blocker, but we need spend
>> more time and focus on these basic model management issues.
>>
>> I'll vote +1 on the release now, but we should mentioned RAVE-859 as known
>> issue.
>>
>> Ate
>>
>> FYI the exception stacktrace when trying to delete a user with 0.15/0.16:
>>
>> INFO : org.apache.rave.portal.**service.impl.**DefaultUserService - about
>> to delete userId: 2
>> INFO : org.apache.rave.portal.**service.impl.**DefaultUserService -
>> Deleted user [2,john.doe] - numPages: 2, numPersonPages:0,
>> numWidgetComments: 0, numWidgetRatings: 0, numWidgetsOwned:
>> 0,numCategoriesTouched:0
>> Dec 5, 2012 9:55:27 AM org.apache.catalina.core.**StandardWrapperValve
>> invoke
>> SEVERE: Servlet.service() for servlet dispatcher threw exception
>> org.apache.rave.persistence.**impl.TranslatedH2Exception: Unknown
>> Database Error
>>          at org.apache.rave.persistence.**jpa.impl.H2OpenJpaDialect.**
>> translateExceptionIfPossible(**H2OpenJpaDialect.java:60)
>>          at org.springframework.orm.jpa.**JpaTransactionManager.**doCommit(
>> **JpaTransactionManager.java:**516)
>>          at org.springframework.**transaction.support.**
>> AbstractPlatformTransactionMan**ager.processCommit(**
>> AbstractPlatformTransactionMan**ager.java:754)
>>          at org.springframework.**transaction.support.**
>> AbstractPlatformTransactionMan**ager.commit(**
>> AbstractPlatformTransactionMan**ager.java:723)
>>          at org.springframework.**transaction.interceptor.**
>> TransactionAspectSupport.**commitTransactionAfterReturnin**
>> g(TransactionAspectSupport.**java:394)
>>          at org.springframework.**transaction.interceptor.**
>> TransactionInterceptor.invoke(**TransactionInterceptor.java:**120)
>>          at org.springframework.aop.**framework.**
>> ReflectiveMethodInvocation.**proceed(**ReflectiveMethodInvocation.**
>> java:172)
>>          at org.springframework.aop.**framework.JdkDynamicAopProxy.**
>> invoke(JdkDynamicAopProxy.**java:202)
>>          at $Proxy132.deleteUser(Unknown Source)
>>          at org.apache.rave.portal.web.**controller.admin.**UserController.
>> **deleteUserDetail(**UserController.java:161)
>>
>>
>>
>>
>>>
>>>
>>> -----Original Message-----
>>> From: Chris Geer [chris@cxtsoftware.com<mailto:**chris@cxtsoftware.com<chris@cxtsoftware.com>
>>>> ]
>>> Sent: Tuesday, December 04, 2012 08:52 PM Eastern Standard Time
>>> To: dev@rave.apache.org
>>> Subject: Re: [DISCUSS] Apache Rave 0.18 Release Candidate
>>>
>>>
>>> On Tue, Dec 4, 2012 at 5:35 PM, Ate Douma <ate@douma.nu> wrote:
>>>
>>>   I tested the 0.18 release and all-in-all it works pretty fine.
>>>> The performance on H2 still is an issue of course but not blocking
>>>> (RAVE-838).
>>>> Also, RAVE-845 seems to be fixed, deleting a user with friend
>>>> associations
>>>> now works.
>>>>
>>>> However I discovered a new, and IMO worse error RAVE-859: when I delete a
>>>> user who has pages shared with, that action also deletes those shared
>>>> pages
>>>> which aren't 'owned' by this user. Rather destructive...
>>>>
>>>> I'm not sure we should qualify this as a release blocker, as we already
>>>> canceled the previous release candidate, but *functionally* it certainly
>>>> qualifies. I don't know if anyone (yet) is using this feature in an
>>>> (almost) production environment, but if so then they should *not* upgrade
>>>> to this 0.18 release candidate until this bug is fixed.
>>>> Or, well, maybe previous releases also had this bug already (I haven't
>>>> had
>>>> time to check) in which case it doesn't really matter.
>>>>
>>>> WDYT: should we accept this as a known/recognized bug (and then highlight
>>>> this in the release announcement) or qualify this as a release blocker?
>>>>
>>>>
>>> I vote that we proceed with the release and put a note not to upgrade if
>>> you use this feature. That way people who don't use the feature get an
>>> upgrade and the people who do use it are not any worse off as long as they
>>> don't upgrade.
>>>
>>> Two questions on RAVE-859
>>>    - Do you know if it's a logic error (we are purposely deleting the page)
>>> or is it an unintended JPA delete based on referential integrity?
>>>    - How will your work on the HMVC impact pages and page sharing? Will it
>>> fix this issue by replacing it with a different approach?
>>>
>>> If we can get concurrence on RAVE-859 then here is my +1
>>>
>>> Chris
>>>
>>>
>>>> I'm holding off voting +1/-1 for now.
>>>>
>>>> Ate
>>>>
>>>>
>>>>
>>>> On 12/02/2012 05:34 AM, Raminderjeet Singh wrote:
>>>>
>>>>   Discussion thread for vote on 0.18 release candidate.
>>>>> For more information on the release process, checkout -
>>>>> http://www.apache.org/dev/****release.html<http://www.apache.org/dev/**release.html>
>>>>> <http://www.**apache.org/dev/release.html<http://www.apache.org/dev/release.html>
>>>>> Some of the things to check before voting are:
>>>>> - can you run the demo binaries
>>>>> - can you build the contents of source-release.zip and svn tag
>>>>> - do all of the staged jars/zips contain the required LICENSE and NOTICE
>>>>> files
>>>>> - are all of the staged artifacts signed and the signature verifiable
>>>>> - is the signing key in the project's KEYS file and on a public server
>>>>>
>>>>>
>>>>>
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2012.0.2221 / Virus Database: 2634/5433 - Release Date: 12/02/12


Mime
View raw message