rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Carlucci, Tony" <acarlu...@mitre.org>
Subject RE: [discuss] Apache Rave 0.5-incubating Release Candidate
Date Tue, 01 Nov 2011 15:09:43 GMT
-----Original Message-----
From: Jasha Joachimsthal [mailto:j.joachimsthal@onehippo.com] 
Sent: Tuesday, November 01, 2011 11:00 AM
To: rave-dev@incubator.apache.org
Subject: Re: [discuss] Apache Rave 0.5-incubating Release Candidate

On 1 November 2011 15:36, Carlucci, Tony <acarlucci@mitre.org> wrote:

>
> -----Original Message-----
> From: Jasha Joachimsthal [mailto:j.joachimsthal@onehippo.com]
> Sent: Tuesday, November 01, 2011 10:10 AM
> To: rave-dev@incubator.apache.org
> Subject: Re: [discuss] Apache Rave 0.5-incubating Release Candidate
>
> On 1 November 2011 15:05, Ciancetta, Jesse E. <jcian@mitre.org> wrote:
>
> > >-----Original Message-----
> > >From: Jasha Joachimsthal [mailto:j.joachimsthal@onehippo.com]
> > >Sent: Tuesday, November 01, 2011 10:00 AM
> > >To: rave-dev@incubator.apache.org
> > >Subject: Re: [discuss] Apache Rave 0.5-incubating Release Candidate
> > >
> > >On 1 November 2011 14:50, Marlon Pierce <mpierce@cs.indiana.edu> 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 

---
Anthony Carlucci | SW App Dev Eng, Sr. | R501 / KW App Development & Maint
e: acarlucci@mitre.org | v: 781.271.2432 | f: 781.271.3299
The MITRE Corporation | 202 Burlington Rd | Bedford, MA 01730-1420

Mime
View raw message