fix pgsql lob behavior #466
Conversation
|
|
||
| private byte[] certificateData; | ||
|
|
||
| public byte[] getCertificateData() { |
There was a problem hiding this comment.
should we name it like getRaw() ? instead of getCertData ?
There was a problem hiding this comment.
I'm all ears, I still don't like either name, but ran out of ideas :D
There was a problem hiding this comment.
Envoyé de mon iPhone
Le 13 janv. 2015 à 19:06, Matthias Wessendorf notifications@github.com a écrit :
In model/api/src/main/java/org/jboss/aerogear/unifiedpush/api/Certificate.java:
@@ -0,0 +1,15 @@
+package org.jboss.aerogear.unifiedpush.api;
+
+
+public class Certificate extends BaseModel {
+
- private byte[] certificateData;
- public byte[] getCertificateData() {
I like raw :-)What about getContent ?
—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
What about getContent ?
Not sure - naming is hard :-) but to me content implies text, instead of binary.
How about getRawCertificate() ?
|
@sebastienblanc do you mind reviewing/testing too ? thanks! |
| package org.jboss.aerogear.unifiedpush.api; | ||
|
|
||
|
|
||
| public class Certificate extends BaseModel { |
There was a problem hiding this comment.
Can you add some simple javadoc and a license header?
|
Tested against both MySQL and PostgreSQL: |
|
I can create an app and its iOS variant but when trying to send a notification that is what I get : https://gist.github.com/sebastienblanc/77651f87529b44c3ab10 |
|
nice find! On Wed, Jan 14, 2015 at 4:48 PM, Sebastien Blanc notifications@github.com
Matthias Wessendorf blog: http://matthiaswessendorf.wordpress.com/ |
| <discriminator-value>ios</discriminator-value> | ||
| <attributes> | ||
| <basic name="certificate"> | ||
| <lob/> | ||
| <many-to-one name="certificate" fetch="LAZY"> |
There was a problem hiding this comment.
Looks like this is actually causing the issue @sebastienblanc identified here: https://gist.github.com/sebastienblanc/77651f87529b44c3ab10
There was a problem hiding this comment.
@qmx any comment on the error ?
qmx
changed the title
|
looks like the only portable way of doing this is by just storing it as a Base64-encoded string - should work for all databases and the overhead is minimal. @matzew @sebastienblanc could you give it a try please? |
|
@matzew done |
|
@qmx the migration, that will be on a different PR, right ? |
|
and, oh - can you submit this PR agains the 1.0.x branch ? To speed up the 1.0.3 release train? |
|
@matzew sure - looks like I've missed the [RFC] at the PR title ;) will look into the migration now |
|
yeah, the JIRA task for migration is here: https://issues.jboss.org/browse/AGPUSH-1147 |
|
@matzew at least can you confirm that it works for you? |
|
Tested on a fresh postgres DB 👍 Can create ios variant and send push notifications. |
|
@sebastienblanc indeed, migration will be another PR, to follow shortly |
|
merged, thx landed 6b20f3e |
PostgreSQL may use more than one row for storing *LOB fields, hence a
transaction is always needed for ensuring consistency.
By extracting the certificate storage to its own entity, access to variants
becomes more lightweight, and cert lookup can be made lazy.
AGPUSH-1144