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

[Matthias Wessendorf]

do we have a JIRA for this ?

On Fri, Feb 13, 2015 at 1:17 PM, Matthias Wessendorf <matzew at apache.org>
wrote:

> 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>
>> 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
>>
>> > _______________________________________________
>> > aerogear-dev mailing list
>> > aerogear-dev at lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>
>>
>> --
>> qmx
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>
>
>
> --
> Sent from Gmail Mobile
>



-- 
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/20150224/bfe44942/attachment.html