[aerogear-dev] Push Admin UI - Updating

Karel Piwko kpiwko at redhat.com
Thu Jul 18 05:18:08 EDT 2013


On Tue, 16 Jul 2013 15:23:30 +0200
Matthias Wessendorf <matzew at apache.org> wrote:

> On Tue, Jul 16, 2013 at 3:15 PM, Kris Borchers <kris at redhat.com> wrote:
> 
> >
> > On Jul 16, 2013, at 7:42 AM, Matthias Wessendorf <matzew at apache.org>
> > wrote:
> >
> >
> >
> >
> > On Tue, Jul 16, 2013 at 2:39 PM, Lucas Holmquist <lholmqui at redhat.com>wrote:
> >
> >>
> >> On Jul 16, 2013, at 8:35 AM, Kris Borchers <kris at redhat.com> wrote:
> >>
> >>
> >> On Jul 16, 2013, at 7:30 AM, Matthias Wessendorf <matzew at apache.org>
> >> wrote:
> >>
> >>
> >>
> >>
> >> On Tue, Jul 16, 2013 at 2:23 PM, Kris Borchers <kris at redhat.com> wrote:
> >>
> >>>
> >>> On Jul 16, 2013, at 7:20 AM, Matthias Wessendorf <matzew at apache.org>
> >>> wrote:
> >>>
> >>>
> >>>
> >>>
> >>> On Tue, Jul 16, 2013 at 2:14 PM, Kris Borchers <kris at redhat.com> wrote:
> >>>
> >>>>
> >>>> On Jul 16, 2013, at 7:10 AM, Matthias Wessendorf <matzew at apache.org>
> >>>> wrote:
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> On Tue, Jul 16, 2013 at 2:05 PM, Kris Borchers <kris at redhat.com> wrote:
> >>>>
> >>>>>
> >>>>> On Jul 16, 2013, at 3:05 AM, Matthias Wessendorf <matzew at apache.org>
> >>>>> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> On Tue, Jul 16, 2013 at 9:57 AM, Sebastien Blanc
> >>>>> <scm.blanc at gmail.com>wrote:
> >>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Tue, Jul 16, 2013 at 8:36 AM, Matthias Wessendorf <
> >>>>>> matzew at apache.org> wrote:
> >>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On Mon, Jul 15, 2013 at 8:34 PM, Lucas Holmquist <
> >>>>>>> lholmqui at redhat.com> wrote:
> >>>>>>>
> >>>>>>>> here's a scenario.
> >>>>>>>>
> >>>>>>>> A user creates an "Application" then adds a new "iOS Variant" to it.
> >>>>>>>>
> >>>>>>>> they decide they need to rename this Variant since they didn't name
> >>>>>>>> it very well.
> >>>>>>>>
> >>>>>>>> There is currently no way of renaming/ updating the description of
> >>>>>>>> this variant without re-sending the Certificate/pass phrase.  this
> >>>>>>>> would be the same for Android and Simple Push,  just substitute
> >>>>>>>> google key and channels
> >>>>>>>>
> >>>>>>>
> >>>>>>> yes - but should be easy to change :-)
> >>>>>>>
> >>>>>>> See here:
> >>>>>>>
> >>>>>>> https://github.com/aerogear/aerogear-unified-push-server/blob/master/src/main/java/org/jboss/aerogear/connectivity/rest/registry/applications/AndroidVariantEndpoint.java#L134-L138
> >>>>>>>
> >>>>>>> in the "update", we just need to onbly update/replace those things
> >>>>>>> that are NOT null. Of course, perhaps we need to rethink the "update
> >>>>>>> validation" too:
> >>>>>>>
> >>>>>>> https://github.com/aerogear/aerogear-unified-push-server/blob/master/src/main/java/org/jboss/aerogear/connectivity/rest/registry/applications/AndroidVariantEndpoint.java#L130-L132
> >>>>>>>
> >>>>>>> Yes that could work even if I must admit I'm not a big fan of
> >>>>>> updating based on a null check ... Imagine someone wants to delete the
> >>>>>> description by setting it to null , that won't be possible ;)
> >>>>>>
> >>>>>
> >>>>> well, yeah - that (the desc) is optional - I was more interested in
> >>>>> the required fields:
> >>>>> e.g. for iOS Variant:
> >>>>> * cert/passphrase (or googlekey/networkURL on other variants)
> >>>>> PushApp:
> >>>>> * name
> >>>>>
> >>>>>
> >>>>> I'm not sure how I feel about this partial update. I'm not a big fan
> >>>>> of saying "It's a required field but leave it null if you don't want it
> >>>>> to change." Feels wrong.
> >>>>>
> >>>>
> >>>> not sure I am following, but the required fields should never be null
> >>>>
> >>>>
> >>>> Now I'm not following.
> >>>>
> >>>
> >>> :-)
> >>>
> >>>
> >>>> Let's go through a scenario and see how each would work with your
> >>>> method. Imagine there are currently values in every field. What fields
> >>>> need values and what can be null when:
> >>>>
> >>>> I update a single required field?
> >>>> I update a single optional field?
> >>>>
> >>>
> >>> Well,when I update the FORM and hit ENTER (let's assume this is a PUT,
> >>> just for now..), aren't the entire form values sent?
> >>>
> >>> So, any field that changed will be udpated, I think
> >>>
> >>> Now if you NULL out a required field, I'd imagine that is not allowed.
> >>>
> >>>
> >>> So I guess I'm confused as to why there was any discussion of NULL
> >>> fields being ignored then. If the form is filled in for the user, if they
> >>> empty a field it should either fail (required field) or empty the value
> >>> (optional field).
> >>>
> >>
> >> +1
> >>
> >>
> >>> NULL should never just be ignored, right?
> >>>
> >>
> >> I think, the "confusion" comes in..... what happens if (iOS Variant),
> >> JUST the name is changed (note: the certificate FILE is required).
> >>
> >>
> >> I guess that would be just fine... if (on the JUST iOSVariant.name is
> >> changed), we do not bother with the certificate.
> >>
> >> We update the certificate ONLY if it is actually part of the "UPDATE
> >> request".
> >>
> >>
> >> I see. Yeah that's pretty standard. What I have done in my app building
> >> past when dealing with files (certificate) is to show the current file name
> >> in the UI and if the file field is left blank, ignore that in the update.
> >>
> >>
> >> that seems to be a good way of doing it.  we would still need to change
> >> the logic on the endpoint method to ignore the certificate if its not
> >> there, or use PATCH?
> >>
> >
> > I'd say PUT (but I am not Roy Fielding ;-))
> >
> >
> > I would vote PATCH to be correct but would not cry if it's PUT I guess.
> >
> 
> yeah - good call. I guess PATCH does make sense [1].
> 

QE's able to test HTTP PATCH, RESTAssured supports it ootb.

> 
> 
> 
> 
> [1] http://www.mnot.net/blog/2012/09/05/patch
> 
> 
> 
> 
> >
> >
> >
> >>
> >>
> >> the thing i'd be worried about when resending the cert bits,
> >>
> >
> > do not do it :-)   Only send IF a user really did select a file with the
> > [input type:file] field, no ?
> >
> >
> > +1
> >
> >
> >
> >> would be that some bit might change in the transfer and F up everything
> >> else
> >>
> >>
> >>
> >> -M
> >>
> >>
> >>
> >>
> >>
> >>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>
> >>>>> If we are to do partial updates like this though, I would say it
> >>>>> should probably use http PATCH instead of PUT, right?
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>>  (Well not sure it is a valid use case but you see my point) and as
> >>>>>> you said we must then rethink user validation.
> >>>>>> Other option is to split the update into more smaller specific
> >>>>>> updates methods ... but not sure about this also.
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Also,  once we have a variant created,  we have a variant ID,  we
> >>>>>>>> should be able to use that ID( plus type? ) to do updates instead of
> >>>>>>>> having to pass the applicaitonID and variantID( same would go for a
> >>>>>>>> delete i guess ).
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Not really sure the best way to handle this.
> >>>>>>>>
> >>>>>>>
> >>>>>>> the "variantID" is not a PK, but it's unique - so a new finder is
> >>>>>>> what you need.
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Should a user be able to update a certificate/key/channel once it
> >>>>>>>> has been created.
> >>>>>>>
> >>>>>>>
> >>>>>>> yes, they should be able to replace an outdated (or revoked)
> >>>>>>> certificate, API etc.
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>>  How would this affect any installations it might have?
> >>>>>>>>
> >>>>>>>
> >>>>>>> Not at all.  so if (speaking Androind) ... a new "project" is used -
> >>>>>>> there is a new SenderID - that's more impact, since the actual mobile
> >>>>>>> app needs to be updated, but again that's not really a concern for
> >>>>>>> the push server
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>> thoughts?
> >>>>>>>>
> >>>>>>>> -Luke
> >>>>>>>> _______________________________________________
> >>>>>>>> aerogear-dev mailing list
> >>>>>>>> aerogear-dev at lists.jboss.org
> >>>>>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> Matthias Wessendorf
> >>>>>>>
> >>>>>>> blog: http://matthiaswessendorf.wordpress.com/
> >>>>>>> sessions: http://www.slideshare.net/mwessendorf
> >>>>>>> twitter: http://twitter.com/mwessendorf
> >>>>>>>
> >>>>>>> _______________________________________________
> >>>>>>> aerogear-dev mailing list
> >>>>>>> aerogear-dev at lists.jboss.org
> >>>>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> aerogear-dev mailing list
> >>>>>> aerogear-dev at lists.jboss.org
> >>>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> Matthias Wessendorf
> >>>>>
> >>>>> blog: http://matthiaswessendorf.wordpress.com/
> >>>>> sessions: http://www.slideshare.net/mwessendorf
> >>>>> twitter: http://twitter.com/mwessendorf
> >>>>> _______________________________________________
> >>>>> aerogear-dev mailing list
> >>>>> aerogear-dev at lists.jboss.org
> >>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>>>>
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> aerogear-dev mailing list
> >>>>> aerogear-dev at lists.jboss.org
> >>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>> --
> >>>> Matthias Wessendorf
> >>>>
> >>>> blog: http://matthiaswessendorf.wordpress.com/
> >>>> sessions: http://www.slideshare.net/mwessendorf
> >>>> twitter: http://twitter.com/mwessendorf
> >>>> _______________________________________________
> >>>> aerogear-dev mailing list
> >>>> aerogear-dev at lists.jboss.org
> >>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> aerogear-dev mailing list
> >>>> aerogear-dev at lists.jboss.org
> >>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>>>
> >>>
> >>>
> >>>
> >>> --
> >>> Matthias Wessendorf
> >>>
> >>> blog: http://matthiaswessendorf.wordpress.com/
> >>> sessions: http://www.slideshare.net/mwessendorf
> >>> twitter: http://twitter.com/mwessendorf
> >>> _______________________________________________
> >>> aerogear-dev mailing list
> >>> aerogear-dev at lists.jboss.org
> >>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> aerogear-dev mailing list
> >>> aerogear-dev at lists.jboss.org
> >>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>>
> >>
> >>
> >>
> >> --
> >> Matthias Wessendorf
> >>
> >> blog: http://matthiaswessendorf.wordpress.com/
> >> sessions: http://www.slideshare.net/mwessendorf
> >> twitter: http://twitter.com/mwessendorf
> >> _______________________________________________
> >> aerogear-dev mailing list
> >> aerogear-dev at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>
> >>
> >> _______________________________________________
> >> aerogear-dev mailing list
> >> aerogear-dev at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>
> >>
> >>
> >> _______________________________________________
> >> aerogear-dev mailing list
> >> aerogear-dev at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >>
> >
> >
> >
> > --
> > Matthias Wessendorf
> >
> > blog: http://matthiaswessendorf.wordpress.com/
> > sessions: http://www.slideshare.net/mwessendorf
> > twitter: http://twitter.com/mwessendorf
> > _______________________________________________
> > aerogear-dev mailing list
> > aerogear-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >
> >
> >
> > _______________________________________________
> > aerogear-dev mailing list
> > aerogear-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/aerogear-dev
> >
> 
> 
> 



More information about the aerogear-dev mailing list