DomGarguilo commented on code in PR #3642:
URL: https://github.com/apache/accumulo/pull/3642#discussion_r1270953241


##########
test/src/main/java/org/apache/accumulo/test/functional/AmpleConditionalWriterIT.java:
##########
@@ -382,6 +397,125 @@ public void testSelectedFiles() throws Exception {
     assertNull(context.getAmple().readTablet(e1).getSelectedFiles());
   }
 
+  /**
+   * Verifies that if selected files have been manually changed in the 
metadata, the files will be
+   * re-ordered before being read
+   */
+  @Test
+  public void testSelectedFilesReordering() throws Exception {
+    try (AccumuloClient client = 
Accumulo.newClient().from(getClientProps()).build()) {
+      var context = cluster.getServerContext();
+
+      String pathPrefix = 
"hdfs://localhost:8020/accumulo/tables/2a/default_tablet/";
+      StoredTabletFile stf1 = new StoredTabletFile(pathPrefix + "F0000070.rf");
+      StoredTabletFile stf2 = new StoredTabletFile(pathPrefix + "F0000071.rf");
+      StoredTabletFile stf3 = new StoredTabletFile(pathPrefix + "F0000072.rf");
+
+      final Set<StoredTabletFile> storedTabletFiles = Set.of(stf1, stf2, stf3);
+      final boolean initiallySelectedAll = true;
+      final long fateTxId = 2L;
+      final SelectedFiles selectedFiles =
+          new SelectedFiles(storedTabletFiles, initiallySelectedAll, fateTxId);
+
+      ConditionalTabletsMutatorImpl ctmi = new 
ConditionalTabletsMutatorImpl(context);
+
+      // write the SelectedFiles to the keyextent
+      
ctmi.mutateTablet(e1).requireAbsentOperation().putSelectedFiles(selectedFiles)
+          .submit(tm -> false);
+
+      // verify we can read the selected files
+      Status mutationStatus = ctmi.process().get(e1).getStatus();
+      assertEquals(Status.ACCEPTED, mutationStatus, "Failed to put selected 
files to tablet");
+      assertEquals(selectedFiles, 
context.getAmple().readTablet(e1).getSelectedFiles(),
+          "Selected files should match those that were written");
+
+      final Text row = e1.toMetaRow();
+      final Text selectedColumnFamily = SELECTED_COLUMN.getColumnFamily();
+      final Text selectedColumnQualifier = 
SELECTED_COLUMN.getColumnQualifier();
+
+      Supplier<String> selectedMetadataValue = () -> {
+        try (Scanner scanner = client.createScanner(MetadataTable.NAME)) {
+          scanner.fetchColumn(selectedColumnFamily, selectedColumnQualifier);
+          scanner.setRange(new Range(row));
+
+          return scanner.stream().map(Map.Entry::getValue).map(Value::get)
+              .map(entry -> new String(entry, UTF_8)).collect(onlyElement());
+        } catch (Exception e) {
+          throw new RuntimeException(e);
+        }
+      };
+
+      String actualMetadataValue = selectedMetadataValue.get();
+      final String expectedMetadata = selectedFiles.getMetadataValue();
+      assertEquals(expectedMetadata, actualMetadataValue,
+          "Value should be equal to metadata of SelectedFiles object that was 
written");
+
+      // get the string value of the paths
+      List<String> filesPathList = 
storedTabletFiles.stream().map(StoredTabletFile::toString)
+          .sorted().collect(Collectors.toList());
+
+      // verify we have the format of the json correct
+      String newJson = createSelectedFilesJson(fateTxId, initiallySelectedAll, 
filesPathList);
+      assertEquals(actualMetadataValue, newJson,
+          "Test json should be identical to actual metadata at this point");
+
+      // reverse the order of the files and create a new json
+      Collections.reverse(filesPathList);
+      newJson = createSelectedFilesJson(fateTxId, initiallySelectedAll, 
filesPathList);
+      assertNotEquals(actualMetadataValue, newJson,
+          "Test json should have reverse file order of actual metadata");
+
+      // write the json with reverse file order
+      try (BatchWriter bw = client.createBatchWriter(MetadataTable.NAME)) {
+        Mutation mutation = new Mutation(row);
+        mutation.put(selectedColumnFamily, selectedColumnQualifier,
+            new Value(newJson.getBytes(UTF_8)));
+        bw.addMutation(mutation);
+      }
+
+      // verify the metadata has been changed
+      actualMetadataValue = selectedMetadataValue.get();
+      assertEquals(newJson, actualMetadataValue,

Review Comment:
   Yea there may be some un-needed portions of the test. I made the test very 
thorough while developing incrementally to make sure everything was working as 
I expected at each step. Not sure if we want to remove stuff to simplify the 
code or leave it as is.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to