rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Raminderjeet Singh <raminderjsi...@gmail.com>
Subject Re: [DISCUSS] Apache Rave 0.18 Release Candidate
Date Wed, 05 Dec 2012 14:38:45 GMT
Because of this discussion, i feel like extending the vote. I will keep the vote open till
end of day today. 

Raminder
On Dec 5, 2012, at 8:35 AM, Franklin, Matthew B. wrote:

>> -----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