rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Franklin, Matthew B." <mfrank...@mitre.org>
Subject RE: [DISCUSS] Apache Rave 0.18 Release Candidate
Date Wed, 05 Dec 2012 13:35:53 GMT
>-----Original Message-----
>From: Paul Sharples [mailto:p.sharples@bolton.ac.uk]
>Sent: Wednesday, December 05, 2012 5:42 AM
>To: dev@rave.apache.org
>Subject: Re: [DISCUSS] Apache Rave 0.18 Release Candidate
>
>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;
>     }

+1.  We also need a corresponding unit test


>
>...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@cxtsoftwar
>e.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