[GitHub] [accumulo] keith-turner commented on a diff in pull request #3575: Ensure serialzed verion of SelectedFiles is comparable

["keith-turner (via GitHub)" <gi...@apache.org>]

keith-turner commented on code in PR #3575:
URL: https://github.com/apache/accumulo/pull/3575#discussion_r1257379049


##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/SelectedFiles.java:
##########
@@ -63,23 +72,63 @@ public SelectedFiles(Set<StoredTabletFile> files, boolean initiallySelectedAll,
     jData.selAll = initiallySelectedAll;
     // ELASITICITY_TODO need the produced json to always be the same when the input data is the same
     // as its used for comparison. Need unit test to ensure this behavior.
-    metadataValue = GSON.get().toJson(jData);
+    metadataValue = GSON.toJson(jData);
   }
 
-  private SelectedFiles(Set<StoredTabletFile> files, boolean initiallySelectedAll, long fateTxId,
-      String metaVal) {
-    Preconditions.checkArgument(files != null && !files.isEmpty());
-    this.files = files;
-    this.initiallySelectedAll = initiallySelectedAll;
-    this.fateTxId = fateTxId;
-    this.metadataValue = metaVal;
+  private static class SelectedFilesTypeAdapter extends TypeAdapter<SelectedFiles> {
+
+    @Override
+    public void write(JsonWriter out, SelectedFiles selectedFiles) throws IOException {
+      out.beginObject();
+      out.name("txid").value(FateTxId.formatTid(selectedFiles.getFateTxId()));
+      out.name("selAll").value(selectedFiles.initiallySelectedAll());
+      List<String> sortedFiles = selectedFiles.getFiles().stream()
+          .map(StoredTabletFile::getMetaUpdateDelete).sorted().collect(Collectors.toList());
+      out.name("files").beginArray();
+      for (String file : sortedFiles) {
+        out.value(file);
+      }
+      out.endArray();
+      out.endObject();
+    }
+
+    @Override
+    public SelectedFiles read(JsonReader in) throws IOException {
+      long fateTxId = 0L;
+      boolean selAll = false;
+      List<String> files = new ArrayList<>();
+
+      in.beginObject();
+      while (in.hasNext()) {
+        String name = in.nextName();
+        switch (name) {
+          case "txid":
+            fateTxId = FateTxId.fromString(in.nextString());
+            break;
+          case "selAll":
+            selAll = in.nextBoolean();
+            break;
+          case "files":
+            in.beginArray();
+            while (in.hasNext()) {
+              files.add(in.nextString());
+            }
+            in.endArray();
+            break;

Review Comment:
   Could this be added?
   
   ```suggestion
               break;
           default:
              throw new IllegalArgumentException("Uknown field name : "+name);
   ```



##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/SelectedFiles.java:
##########
@@ -63,23 +72,63 @@ public SelectedFiles(Set<StoredTabletFile> files, boolean initiallySelectedAll,
     jData.selAll = initiallySelectedAll;
     // ELASITICITY_TODO need the produced json to always be the same when the input data is the same
     // as its used for comparison. Need unit test to ensure this behavior.
-    metadataValue = GSON.get().toJson(jData);
+    metadataValue = GSON.toJson(jData);
   }
 
-  private SelectedFiles(Set<StoredTabletFile> files, boolean initiallySelectedAll, long fateTxId,
-      String metaVal) {
-    Preconditions.checkArgument(files != null && !files.isEmpty());
-    this.files = files;
-    this.initiallySelectedAll = initiallySelectedAll;
-    this.fateTxId = fateTxId;
-    this.metadataValue = metaVal;
+  private static class SelectedFilesTypeAdapter extends TypeAdapter<SelectedFiles> {
+
+    @Override
+    public void write(JsonWriter out, SelectedFiles selectedFiles) throws IOException {
+      out.beginObject();
+      out.name("txid").value(FateTxId.formatTid(selectedFiles.getFateTxId()));
+      out.name("selAll").value(selectedFiles.initiallySelectedAll());
+      List<String> sortedFiles = selectedFiles.getFiles().stream()
+          .map(StoredTabletFile::getMetaUpdateDelete).sorted().collect(Collectors.toList());
+      out.name("files").beginArray();
+      for (String file : sortedFiles) {
+        out.value(file);
+      }
+      out.endArray();

Review Comment:
   Could avoid creating the list w/ something like the following.
   
   ```suggestion
         out.name("files").beginArray();
         // sort the data to make serialized json comparable
        selectedFiles.getFiles().stream()
             .map(StoredTabletFile::getMetaUpdateDelete).sorted().forEach(out::value);
         out.endArray();
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org