[aerogear-dev] UPS bug - am I crazy?

[Matthias Wessendorf]

yes, that's a bug in the test

On Friday, February 13, 2015, Douglas Campos <qmx at qmx.me> wrote:

> On Fri, Feb 13, 2015 at 12:01:07PM +0100, Matthias Wessendorf wrote:
> > Hi,
> >
> > I agree w/ Erik - however, I think the test has a bug, if the
> installations
> > are not directly associated with a Variant. Should be easy to fix.
> That's my point - this test should never pass - hence the "bug"
> suspicion.
>
> >
> > BTW. qmx, you mentioned the "setDeviceType()", but that is really just
> > plain text metadata, like here:
> >
> https://github.com/aerogear/aerogear-push-helloworld/blob/master/ios/HelloWorld/AGAppDelegate.m#L106
> >
> >
> > The validator is triggered by the actual variant type (installation
> > .getVariant().getType())
> >
> > -Matthias
> >
> >
> >
> > On Fri, Feb 13, 2015 at 8:17 AM, Erik Jan de Wit <edewit at redhat.com
> <javascript:;>> wrote:
> >
> > > Hi,
> > >
> > > Don’t know if it’s really a bug, but in order to validate that a device
> > > token is valid we need to know what variant type it belongs to. It
> makes no
> > > sense for a installation to be persisted without a Variant, because
> then
> > > you cannot send any messages to that device. So if you try to persist a
> > > installation without a variant the device token is not valid seems to
> be to
> > > be a valid statement.
> > >
> > > This was introduced when we added cordova, there it’s easy to make a
> > > mistake and have the wrong variantID / secret in your settings.
> Resulting
> > > in a iOS device registering under an android variant. With the
> deviceToken
> > > validation this can no longer happen.
> > >
> > > Does this make sense?
> > >
> > > Cheers,
> > >         Erik Jan
> > >
> > > On 13 Feb,2015, at 2:02 , Douglas Campos <qmx at qmx.me <javascript:;>>
> wrote:
> > >
> > > > Howdy!
> > > >
> > > > I was doing the migration work and found out something funny:
> > > >
> > > > For Installations, we have `installation.setDeviceType()`, but during
> > > > Installation `persist()`, the DeviceTokenValidator is called, which
> in
> > > > turn looks for a Variant.
> > > >
> > > > So after fiddling with some orm settings suddenly a bunch of tests
> > > > started to break, which led me to this snippet[1]:
> > > >
> > > > // disabled
> > > > Installation android3 = new Installation();
> > > > android3.setAlias("foo at bar.org <javascript:;>");
> > > >
> > >
> android3.setDeviceToken("543234234890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890");
> > > > android3.setDeviceType("Android Tablet");
> > > > android3.setEnabled(false);
> > > >
> > > > installationDao.create(android3);
> > > >
> > > > See, our tests are creating an installation without associating it
> to a
> > > > variant first, then it blows up here[2]:
> > > >
> > > > // DeviceTokenValidator#isValid()
> > > > if (installation.getVariant() == null ||
> > > installation.getVariant().getType() == null || deviceToken == null) {
> > > >    return false;
> > > > }
> > > >
> > > > This is smelling like a bug to me - kinda like the validator wasn't
> > > > actually running during tests. Am I crazy?
> > > >
> > > > [1]:
> > >
> https://github.com/aerogear/aerogear-unifiedpush-server/blob/master/model/jpa/src/test/java/org/jboss/aerogear/unifiedpush/jpa/InstallationDaoTest.java#L133-L140
> > > > [2]:
> > >
> https://github.com/aerogear/aerogear-unifiedpush-server/blob/master/model/api/src/main/java/org/jboss/aerogear/unifiedpush/api/validation/DeviceTokenValidator.java#L60
> > > >
> > > > --
> > > > qmx
> > > > _______________________________________________
> > > > aerogear-dev mailing list
> > > > aerogear-dev at lists.jboss.org <javascript:;>
> > > > https://lists.jboss.org/mailman/listinfo/aerogear-dev
> > >
> > >
> > > _______________________________________________
> > > aerogear-dev mailing list
> > > aerogear-dev at lists.jboss.org <javascript:;>
> > > 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 <javascript:;>
> > https://lists.jboss.org/mailman/listinfo/aerogear-dev
>
>
> --
> qmx
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev at lists.jboss.org <javascript:;>
> https://lists.jboss.org/mailman/listinfo/aerogear-dev



-- 
Sent from Gmail Mobile
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/aerogear-dev/attachments/20150213/7d4b0282/attachment.html