You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pirk.apache.org by wraydulany <gi...@git.apache.org> on 2016/10/31 03:22:35 UTC
[GitHub] incubator-pirk pull request #115: PIRK-79
GitHub user wraydulany opened a pull request:
https://github.com/apache/incubator-pirk/pull/115
PIRK-79
This PR is to address the notional problem that the QueryDeserializer I wrote would throw an exception if it received a request to deserialize a query which didn't have a full schema included, only a name. I realized that Pirk should be expected to handle such a circumstance, and that my code wouldn't.
Ironically, the best way of fixing this was to eliminate my custom deserializer. I realized this when I couldn't get changes to the QueryDeserializer code to affect anything. I looked closely at the JsonSerializer and realized my IDE had auto-imported the wrong "Query" (thanks, Intellij!) and so the custom class wasn't actually performing the deserialization. This...I'll be honest, it felt embarassing, and so I share it with you, to simultaneously embiggen and diffuse the embarassment.
Along the way, I did some small bug fixing and, more importantly, fleshed out SerializationTest. We now actually test serializing and deserializing Response and Querier objects (and, along the way, Query, QueryInfo, QuerySchema, and Paillier objects), for both serializers, java and json.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/wraydulany/incubator-pirk PIRK-79
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-pirk/pull/115.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #115
----
commit 708bf0cf8c705d2d09a5c8fac1859371e21d6c9c
Author: Walter Ray-Dulany <ra...@gmail.com>
Date: 2016-10-28T15:26:56Z
Working on it.
commit 7d72474423ef48388e93605200e206ea8b313a24
Author: Walter Ray-Dulany <ra...@gmail.com>
Date: 2016-10-29T16:05:48Z
Begin excision of QueryDeserializer; gson does a good enough job on its own.
commit a3c5b6c080202af9ca50d8281bff89b0a0466d41
Author: Walter Ray-Dulany <ra...@gmail.com>
Date: 2016-10-30T00:06:48Z
Passes all local tests. Time to merge in master
commit 67064dbf628e692922bb9b9ec820e44145f126a0
Author: Walter Ray-Dulany <ra...@gmail.com>
Date: 2016-10-30T00:07:43Z
Merge remote-tracking branch 'upstream/master' into PIRK-79
commit cd03f18bfe823f790c70d94d96bab18e61a04b47
Author: Walter Ray-Dulany <ra...@gmail.com>
Date: 2016-10-30T17:05:13Z
How did that Expose annotation die?
commit 9943966a5a653c2b1d67203e3b845c50f4d405c8
Author: Walter Ray-Dulany <ra...@gmail.com>
Date: 2016-10-30T23:15:17Z
Updated SerializationTest to exercise both java and json serialization on querier and response.
commit 56c8b7e6e281eeab393c3aa54427fb41ce7ca7c1
Author: Walter Ray-Dulany <ra...@gmail.com>
Date: 2016-10-31T00:01:05Z
turn back on default java serialization.
commit a1dcce98492b3702b5b7291a8af924122cf78593
Author: Walter Ray-Dulany <ra...@gmail.com>
Date: 2016-10-31T00:06:15Z
Turns out google is often better at json deserialization than I.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by smarthi <gi...@git.apache.org>.
Github user smarthi commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85682663
--- Diff: src/main/java/org/apache/pirk/response/wideskies/Response.java ---
@@ -71,4 +71,29 @@ public void addElement(int position, BigInteger element)
{
responseElements.put(position, element);
}
+
+ @Override public boolean equals(Object o)
+ {
+ if (this == o)
+ return true;
+ if (o == null || getClass() != o.getClass())
+ return false;
+
+ Response response = (Response) o;
+
+ if (responseVersion != response.responseVersion)
+ return false;
+ if (!queryInfo.equals(response.queryInfo))
+ return false;
+ return responseElements.equals(response.responseElements);
+
--- End diff --
Can we use Java 8 - Objects.equal() functionality for all equals() methods ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by smarthi <gi...@git.apache.org>.
Github user smarthi commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85682511
--- Diff: src/main/java/org/apache/pirk/encryption/Paillier.java ---
@@ -340,4 +342,41 @@ private String parametersToString()
return "p = " + p.intValue() + " q = " + q.intValue() + " N = " + N.intValue() + " NSquared = " + NSquared.intValue() + " lambdaN = " + lambdaN.intValue()
+ " bitLength = " + bitLength;
}
+
+ @Override public boolean equals(Object o)
+ {
+ if (this == o)
+ return true;
+ if (o == null || getClass() != o.getClass())
+ return false;
+
+ Paillier paillier = (Paillier) o;
+
+ if (bitLength != paillier.bitLength)
+ return false;
+ if (!p.equals(paillier.p))
+ return false;
+ if (!q.equals(paillier.q))
+ return false;
+ if (!N.equals(paillier.N))
+ return false;
+ if (!NSquared.equals(paillier.NSquared))
+ return false;
+ if (!lambdaN.equals(paillier.lambdaN))
+ return false;
--- End diff --
Can all of the above if stmts be OR'ed into one single stmt that returns false ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by wraydulany <gi...@git.apache.org>.
Github user wraydulany commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85686457
--- Diff: src/main/java/org/apache/pirk/encryption/Paillier.java ---
@@ -340,4 +342,41 @@ private String parametersToString()
return "p = " + p.intValue() + " q = " + q.intValue() + " N = " + N.intValue() + " NSquared = " + NSquared.intValue() + " lambdaN = " + lambdaN.intValue()
+ " bitLength = " + bitLength;
}
+
+ @Override public boolean equals(Object o)
+ {
+ if (this == o)
+ return true;
+ if (o == null || getClass() != o.getClass())
+ return false;
+
+ Paillier paillier = (Paillier) o;
+
+ if (bitLength != paillier.bitLength)
+ return false;
+ if (!p.equals(paillier.p))
+ return false;
+ if (!q.equals(paillier.q))
+ return false;
+ if (!N.equals(paillier.N))
+ return false;
+ if (!NSquared.equals(paillier.NSquared))
+ return false;
+ if (!lambdaN.equals(paillier.lambdaN))
+ return false;
--- End diff --
I can, but it just leaves a blob of ORd-together stuff. I admit to knowing little about the java compiler; wouldn't it just optimize these into such a beast anyway? If so, then I feel like it's more aesthetically pleasing to just leave them separated. If not, well, screw that, and I'll change it; or if the community feels my subjective aesthetics aren't aesthetically pleasing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by wraydulany <gi...@git.apache.org>.
Github user wraydulany commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85686604
--- Diff: src/main/java/org/apache/pirk/test/utils/BaseTests.java ---
@@ -51,9 +51,9 @@
public static final int dataPartitionBitSize = 8;
// Selectors for domain and IP queries, queryIdentifier is the first entry for file generation
- private static ArrayList<String> selectorsDomain = new ArrayList<>(
+ public static ArrayList<String> selectorsDomain = new ArrayList<>(
Arrays.asList("s.t.u.net", "d.e.com", "r.r.r.r", "a.b.c.com", "something.else", "x.y.net"));
- private static ArrayList<String> selectorsIP = new ArrayList<>(Arrays.asList("55.55.55.55", "5.6.7.8", "10.20.30.40", "13.14.15.16", "21.22.23.24"));
+ public static ArrayList<String> selectorsIP = new ArrayList<>(Arrays.asList("55.55.55.55", "5.6.7.8", "10.20.30.40", "13.14.15.16", "21.22.23.24"));
--- End diff --
Ditto.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by smarthi <gi...@git.apache.org>.
Github user smarthi commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85682703
--- Diff: src/main/java/org/apache/pirk/test/utils/BaseTests.java ---
@@ -51,9 +51,9 @@
public static final int dataPartitionBitSize = 8;
// Selectors for domain and IP queries, queryIdentifier is the first entry for file generation
- private static ArrayList<String> selectorsDomain = new ArrayList<>(
+ public static ArrayList<String> selectorsDomain = new ArrayList<>(
Arrays.asList("s.t.u.net", "d.e.com", "r.r.r.r", "a.b.c.com", "something.else", "x.y.net"));
- private static ArrayList<String> selectorsIP = new ArrayList<>(Arrays.asList("55.55.55.55", "5.6.7.8", "10.20.30.40", "13.14.15.16", "21.22.23.24"));
+ public static ArrayList<String> selectorsIP = new ArrayList<>(Arrays.asList("55.55.55.55", "5.6.7.8", "10.20.30.40", "13.14.15.16", "21.22.23.24"));
--- End diff --
Generic interface List<> on LHS
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk issue #115: PIRK-79
Posted by ellisonanne <gi...@git.apache.org>.
Github user ellisonanne commented on the issue:
https://github.com/apache/incubator-pirk/pull/115
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by jacobwil <gi...@git.apache.org>.
Github user jacobwil commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85682254
--- Diff: src/test/java/org/apache/pirk/serialization/SerializationTest.java ---
@@ -75,6 +94,106 @@ public void testJavaSerDe() throws Exception
Assert.assertTrue(deserializedDummyObject.equals(dummyRecord));
}
+ @Test
+ public void testQuerierResponseSerializationDeserialization()
+ {
+ testQuerierResponseSerializationDeserialization(jsonSerializer);
+ testQuerierResponseSerializationDeserialization(javaSerializer);
--- End diff --
Is this line supposed to be here twice? If so, a comment as to why would be nice.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by jacobwil <gi...@git.apache.org>.
Github user jacobwil commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85745699
--- Diff: src/main/java/org/apache/pirk/encryption/Paillier.java ---
@@ -340,4 +342,41 @@ private String parametersToString()
return "p = " + p.intValue() + " q = " + q.intValue() + " N = " + N.intValue() + " NSquared = " + NSquared.intValue() + " lambdaN = " + lambdaN.intValue()
+ " bitLength = " + bitLength;
}
+
+ @Override public boolean equals(Object o)
+ {
+ if (this == o)
+ return true;
+ if (o == null || getClass() != o.getClass())
+ return false;
+
+ Paillier paillier = (Paillier) o;
+
+ if (bitLength != paillier.bitLength)
+ return false;
+ if (!p.equals(paillier.p))
+ return false;
+ if (!q.equals(paillier.q))
+ return false;
+ if (!N.equals(paillier.N))
+ return false;
+ if (!NSquared.equals(paillier.NSquared))
+ return false;
+ if (!lambdaN.equals(paillier.lambdaN))
+ return false;
--- End diff --
Personally I prefer the separate if statements when there are this many entries because it's easier to skim through. Putting all of those statements on one line makes it easy to visually glide past one of them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by jacobwil <gi...@git.apache.org>.
Github user jacobwil commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85727264
--- Diff: src/test/java/org/apache/pirk/serialization/SerializationTest.java ---
@@ -75,6 +94,106 @@ public void testJavaSerDe() throws Exception
Assert.assertTrue(deserializedDummyObject.equals(dummyRecord));
}
+ @Test
+ public void testQuerierResponseSerializationDeserialization()
+ {
+ testQuerierResponseSerializationDeserialization(jsonSerializer);
+ testQuerierResponseSerializationDeserialization(javaSerializer);
--- End diff --
My tired brain took too many shortcuts\u2026
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-pirk/pull/115
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by smarthi <gi...@git.apache.org>.
Github user smarthi commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85682557
--- Diff: src/main/java/org/apache/pirk/encryption/Paillier.java ---
@@ -340,4 +342,41 @@ private String parametersToString()
return "p = " + p.intValue() + " q = " + q.intValue() + " N = " + N.intValue() + " NSquared = " + NSquared.intValue() + " lambdaN = " + lambdaN.intValue()
+ " bitLength = " + bitLength;
}
+
+ @Override public boolean equals(Object o)
+ {
+ if (this == o)
+ return true;
+ if (o == null || getClass() != o.getClass())
+ return false;
+
+ Paillier paillier = (Paillier) o;
+
+ if (bitLength != paillier.bitLength)
+ return false;
+ if (!p.equals(paillier.p))
+ return false;
+ if (!q.equals(paillier.q))
+ return false;
+ if (!N.equals(paillier.N))
+ return false;
+ if (!NSquared.equals(paillier.NSquared))
+ return false;
+ if (!lambdaN.equals(paillier.lambdaN))
+ return false;
+ return w.equals(paillier.w);
+
+ }
+
+ @Override public int hashCode()
+ {
+ int result = p.hashCode();
+ result = 31 * result + q.hashCode();
+ result = 31 * result + N.hashCode();
+ result = 31 * result + NSquared.hashCode();
+ result = 31 * result + lambdaN.hashCode();
+ result = 31 * result + w.hashCode();
+ result = 31 * result + bitLength;
+ return result;
+ }
--- End diff --
Java 8 provides a Objects.hash(). Could we use that for all hashCode() methods ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by smarthi <gi...@git.apache.org>.
Github user smarthi commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85682681
--- Diff: src/main/java/org/apache/pirk/test/utils/BaseTests.java ---
@@ -51,9 +51,9 @@
public static final int dataPartitionBitSize = 8;
// Selectors for domain and IP queries, queryIdentifier is the first entry for file generation
- private static ArrayList<String> selectorsDomain = new ArrayList<>(
+ public static ArrayList<String> selectorsDomain = new ArrayList<>(
--- End diff --
Include generic interface List<> on the LHS.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-pirk pull request #115: PIRK-79
Posted by wraydulany <gi...@git.apache.org>.
Github user wraydulany commented on a diff in the pull request:
https://github.com/apache/incubator-pirk/pull/115#discussion_r85683430
--- Diff: src/test/java/org/apache/pirk/serialization/SerializationTest.java ---
@@ -75,6 +94,106 @@ public void testJavaSerDe() throws Exception
Assert.assertTrue(deserializedDummyObject.equals(dummyRecord));
}
+ @Test
+ public void testQuerierResponseSerializationDeserialization()
+ {
+ testQuerierResponseSerializationDeserialization(jsonSerializer);
+ testQuerierResponseSerializationDeserialization(javaSerializer);
--- End diff --
That line isn't in there twice. It's in there once with the arg jsonSerializer, and once with the arg javaSerializer. ;) Thus are both extant serialization services tested.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---