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

[Matthias Wessendorf]

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.

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> 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> 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");
> >
> 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
> > 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/aerogear-dev/attachments/20150213/decee81d/attachment-0001.html