On 1 November 2011 16:09, Carlucci, Tony <[email protected]> wrote:
> -----Original Message----- > From: Jasha Joachimsthal [mailto:[email protected]] > Sent: Tuesday, November 01, 2011 11:00 AM > To: [email protected] > Subject: Re: [discuss] Apache Rave 0.5-incubating Release Candidate > > On 1 November 2011 15:36, Carlucci, Tony <[email protected]> wrote: > > > > > -----Original Message----- > > From: Jasha Joachimsthal [mailto:[email protected]] > > Sent: Tuesday, November 01, 2011 10:10 AM > > To: [email protected] > > Subject: Re: [discuss] Apache Rave 0.5-incubating Release Candidate > > > > On 1 November 2011 15:05, Ciancetta, Jesse E. <[email protected]> wrote: > > > > > >-----Original Message----- > > > >From: Jasha Joachimsthal [mailto:[email protected]] > > > >Sent: Tuesday, November 01, 2011 10:00 AM > > > >To: [email protected] > > > >Subject: Re: [discuss] Apache Rave 0.5-incubating Release Candidate > > > > > > > >On 1 November 2011 14:50, Marlon Pierce <[email protected]> > wrote: > > > > > > > >> -----BEGIN PGP SIGNED MESSAGE----- > > > >> Hash: SHA1 > > > >> > > > >> I was able to download and run the svn tagged version, source > release, > > > and > > > >> the .tar.gz and .zip demo artifacts correctly. However, there seems > > to > > > be > > > >> a bug if I try to add a new gadget with the same URL twice. That > > is, I > > > >> tried adding > http://www.google.com/ig/modules/builtin_weather.xmlwith > > > >> two different names. The first time worked correctly (or at least is > > > >> waiting on admin approval). The second time produced an error ("Rave > > has > > > >> suffered a brief meltdown"). I won't put the stack trace here. > > > >> > > > > > > > >The widget URL is unique to prevent duplicates. Until recently the > user > > > saw > > > >a message in the add widget form that the widget already exists in the > > > >database. We need to find out why the error page is shown now. > > > > > > > > > > I don't personally view this as a blocker though since it only happens > in > > > a very specific case -- does anyone disagree? > > > > > > > > It's not nice and should be fixed, but I don't see it as a blocker > > either. > > > > > Jasha > > > > +1 on it needing to be fixed but not being a blocker. > > > > The error is occurring because: 1) DefaultWidgetService.registerNewWidget > > returns a null Widget if the url already exists, and 2) the > > RavePermissionEvaluator.hasPermission functions need to handle null > objects > > better. I'll create a bug ticket for this issue. > > > > >I wrote the 1) logic but now I see it back I see room for improvement. The > >Validator can reject the new widget if its url is already present. Then if > >for some reason DefaultWidgetService#registerNewWidget is called for a URL > >that already exists, it can throw a (DuplicateItem)Exception instead of > >returning null. WDYT? > > >Jasha > > +1 on the DuplicateItemException enhancement - it's a good pattern to use > across the board and easy for calling functions to trap and handle with > "nice" client side messages. The hasPermission code needs to be fixed as > well for better NPE prevention no matter what (RAVE-331). > > Tony > > I created RAVE-332 for the change in logic. Jasha
