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