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 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 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> 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 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> /**release.html> >>>>>>> >> > 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 >