alexr17 commented on code in PR #13886:
URL: https://github.com/apache/hudi/pull/13886#discussion_r2393711438
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestLockAuditingCommand.java:
##########
@@ -282,4 +421,544 @@ public void testEnableLockAuditMultipleTimes() throws
Exception {
assertTrue(enabledNode.asBoolean(), "Audit should still be enabled");
}
+
+ // ==================== Validation Tests ====================
+
+ /**
+ * Test validation when no audit folder exists.
+ */
+ @Test
+ public void testValidateAuditLocksNoAuditFolder() {
+ Object result = shell.evaluate(() -> "locks audit validate");
+
+ assertAll("Validation handles missing audit folder",
+ () -> assertTrue(ShellEvaluationResultUtil.isSuccess(result)),
+ () -> assertNotNull(result.toString()),
+ () -> assertTrue(result.toString().contains("Validation Result:
PASSED")),
+ () -> assertTrue(result.toString().contains("Transactions Validated:
0")),
+ () -> assertTrue(result.toString().contains("Issues Found: 0")),
+ () -> assertTrue(result.toString().contains("No audit folder found")));
+ }
+
+ /**
+ * Test validation when audit folder exists but no audit files.
+ */
+ @Test
+ public void testValidateAuditLocksNoAuditFiles() throws IOException {
+ // Create audit folder but no files
+ String auditFolderPath =
StorageLockProviderAuditService.getAuditFolderPath(HoodieCLI.basePath);
+ StoragePath auditDir = new StoragePath(auditFolderPath);
+ HoodieCLI.storage.createDirectory(auditDir);
+
+ Object result = shell.evaluate(() -> "locks audit validate");
+
+ assertAll("Validation handles no audit files",
+ () -> assertTrue(ShellEvaluationResultUtil.isSuccess(result)),
+ () -> assertNotNull(result.toString()),
+ () -> assertTrue(result.toString().contains("Validation Result:
PASSED")),
+ () -> assertTrue(result.toString().contains("Transactions Validated:
0")),
+ () -> assertTrue(result.toString().contains("Issues Found: 0")),
+ () -> assertTrue(result.toString().contains("No audit files found")));
+ }
+
+ /**
+ * Test validation - No Issues (PASSED)
+ */
+ @Test
+ public void testValidateAuditLocksNoIssues() throws IOException {
+ long baseTime = System.currentTimeMillis();
+ List<TransactionScenario> scenarios = new ArrayList<>();
+
+ // Transaction 1: Complete transaction
+ List<AuditRecord> records1 = new ArrayList<>();
+ records1.add(new AuditRecord("owner1", baseTime, baseTime + 100, "START",
baseTime + 60000));
+ records1.add(new AuditRecord("owner1", baseTime, baseTime + 200, "RENEW",
baseTime + 60000));
+ records1.add(new AuditRecord("owner1", baseTime, baseTime + 300, "END",
baseTime + 60000));
+ scenarios.add(new TransactionScenario(baseTime + "_owner1.jsonl",
records1));
+
+ // Transaction 2: Complete transaction starting after first one ends
+ List<AuditRecord> records2 = new ArrayList<>();
+ records2.add(new AuditRecord("owner2", baseTime + 500, baseTime + 600,
"START", baseTime + 60000));
+ records2.add(new AuditRecord("owner2", baseTime + 500, baseTime + 700,
"END", baseTime + 60000));
+ scenarios.add(new TransactionScenario((baseTime + 500) + "_owner2.jsonl",
records2));
+
+ createAuditFiles(scenarios);
+
+ Object result = shell.evaluate(() -> "locks audit validate");
+
+ assertAll("Validation passes with no issues",
+ () -> assertTrue(ShellEvaluationResultUtil.isSuccess(result)),
+ () -> assertNotNull(result.toString()),
+ () -> assertTrue(result.toString().contains("Validation Result:
PASSED")),
+ () -> assertTrue(result.toString().contains("Transactions Validated:
2")),
+ () -> assertTrue(result.toString().contains("Issues Found: 0")),
+ () -> assertTrue(result.toString().contains("successfully")));
+ }
+
+ /**
+ * Test validation - Single Unclosed Transaction (WARNING)
+ */
+ @Test
+ public void testValidateAuditLocksSingleUnclosedTransaction() throws
IOException {
+ long baseTime = 1000000L;
+ List<TransactionScenario> scenarios = new ArrayList<>();
+
+ // Single audit file without END record
+ List<AuditRecord> records1 = new ArrayList<>();
+ records1.add(new AuditRecord("owner1", baseTime, baseTime + 100, "START",
baseTime + 200));
+ scenarios.add(new TransactionScenario(baseTime + "_owner1.jsonl",
records1));
+
+ createAuditFiles(scenarios);
+
+ Object result = shell.evaluate(() -> "locks audit validate");
+
+ assertAll("Single unclosed transaction shows warning",
+ () -> assertTrue(ShellEvaluationResultUtil.isSuccess(result)),
+ () -> assertNotNull(result.toString()),
+ () -> assertTrue(result.toString().contains("Validation Result:
WARNING")),
+ () -> assertTrue(result.toString().contains("Transactions Validated:
1")),
+ () -> assertTrue(result.toString().contains("Issues Found: 1")));
+ }
+
+ /**
+ * Test validation - Unclosed Transactions (WARNING)
+ */
+ @Test
+ public void testValidateAuditLocksUnclosedTransactions() throws IOException {
+ long baseTime = 1000000L;
+ List<TransactionScenario> scenarios = new ArrayList<>();
+
+ // Transaction 1: Unclosed (effective end at expiration = baseTime + 200)
+ List<AuditRecord> records1 = new ArrayList<>();
+ records1.add(new AuditRecord("owner1", baseTime, baseTime + 100, "START",
baseTime + 200));
+ scenarios.add(new TransactionScenario(baseTime + "_owner1.jsonl",
records1));
+
+ // Transaction 2: Complete, starts after owner1's expiration
+ List<AuditRecord> records2 = new ArrayList<>();
+ records2.add(new AuditRecord("owner2", baseTime + 300, baseTime + 400,
"START", baseTime + 60000));
+ records2.add(new AuditRecord("owner2", baseTime + 300, baseTime + 500,
"END", baseTime + 60000));
+ scenarios.add(new TransactionScenario((baseTime + 300) + "_owner2.jsonl",
records2));
+
+ createAuditFiles(scenarios);
+
+ Object result = shell.evaluate(() -> "locks audit validate");
+
+ assertAll("Unclosed transactions show warning",
+ () -> assertTrue(ShellEvaluationResultUtil.isSuccess(result)),
+ () -> assertNotNull(result.toString()),
+ () -> assertTrue(result.toString().contains("Validation Result:
WARNING")),
+ () -> assertTrue(result.toString().contains("Transactions Validated:
2")),
+ () -> assertTrue(result.toString().contains("Issues Found: 1")),
Review Comment:
yes, no it would not be noisy
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestLockAuditingCommand.java:
##########
@@ -282,4 +421,544 @@ public void testEnableLockAuditMultipleTimes() throws
Exception {
assertTrue(enabledNode.asBoolean(), "Audit should still be enabled");
}
+
+ // ==================== Validation Tests ====================
+
+ /**
+ * Test validation when no audit folder exists.
+ */
+ @Test
+ public void testValidateAuditLocksNoAuditFolder() {
+ Object result = shell.evaluate(() -> "locks audit validate");
+
+ assertAll("Validation handles missing audit folder",
+ () -> assertTrue(ShellEvaluationResultUtil.isSuccess(result)),
+ () -> assertNotNull(result.toString()),
+ () -> assertTrue(result.toString().contains("Validation Result:
PASSED")),
+ () -> assertTrue(result.toString().contains("Transactions Validated:
0")),
+ () -> assertTrue(result.toString().contains("Issues Found: 0")),
+ () -> assertTrue(result.toString().contains("No audit folder found")));
+ }
+
+ /**
+ * Test validation when audit folder exists but no audit files.
+ */
+ @Test
+ public void testValidateAuditLocksNoAuditFiles() throws IOException {
+ // Create audit folder but no files
+ String auditFolderPath =
StorageLockProviderAuditService.getAuditFolderPath(HoodieCLI.basePath);
+ StoragePath auditDir = new StoragePath(auditFolderPath);
+ HoodieCLI.storage.createDirectory(auditDir);
+
+ Object result = shell.evaluate(() -> "locks audit validate");
+
+ assertAll("Validation handles no audit files",
+ () -> assertTrue(ShellEvaluationResultUtil.isSuccess(result)),
+ () -> assertNotNull(result.toString()),
+ () -> assertTrue(result.toString().contains("Validation Result:
PASSED")),
+ () -> assertTrue(result.toString().contains("Transactions Validated:
0")),
+ () -> assertTrue(result.toString().contains("Issues Found: 0")),
+ () -> assertTrue(result.toString().contains("No audit files found")));
+ }
+
+ /**
+ * Test validation - No Issues (PASSED)
+ */
+ @Test
+ public void testValidateAuditLocksNoIssues() throws IOException {
+ long baseTime = System.currentTimeMillis();
+ List<TransactionScenario> scenarios = new ArrayList<>();
+
+ // Transaction 1: Complete transaction
+ List<AuditRecord> records1 = new ArrayList<>();
+ records1.add(new AuditRecord("owner1", baseTime, baseTime + 100, "START",
baseTime + 60000));
+ records1.add(new AuditRecord("owner1", baseTime, baseTime + 200, "RENEW",
baseTime + 60000));
+ records1.add(new AuditRecord("owner1", baseTime, baseTime + 300, "END",
baseTime + 60000));
+ scenarios.add(new TransactionScenario(baseTime + "_owner1.jsonl",
records1));
+
+ // Transaction 2: Complete transaction starting after first one ends
+ List<AuditRecord> records2 = new ArrayList<>();
+ records2.add(new AuditRecord("owner2", baseTime + 500, baseTime + 600,
"START", baseTime + 60000));
+ records2.add(new AuditRecord("owner2", baseTime + 500, baseTime + 700,
"END", baseTime + 60000));
+ scenarios.add(new TransactionScenario((baseTime + 500) + "_owner2.jsonl",
records2));
+
+ createAuditFiles(scenarios);
+
+ Object result = shell.evaluate(() -> "locks audit validate");
+
+ assertAll("Validation passes with no issues",
+ () -> assertTrue(ShellEvaluationResultUtil.isSuccess(result)),
+ () -> assertNotNull(result.toString()),
+ () -> assertTrue(result.toString().contains("Validation Result:
PASSED")),
+ () -> assertTrue(result.toString().contains("Transactions Validated:
2")),
+ () -> assertTrue(result.toString().contains("Issues Found: 0")),
+ () -> assertTrue(result.toString().contains("successfully")));
+ }
+
+ /**
+ * Test validation - Single Unclosed Transaction (WARNING)
+ */
+ @Test
+ public void testValidateAuditLocksSingleUnclosedTransaction() throws
IOException {
+ long baseTime = 1000000L;
+ List<TransactionScenario> scenarios = new ArrayList<>();
+
+ // Single audit file without END record
+ List<AuditRecord> records1 = new ArrayList<>();
+ records1.add(new AuditRecord("owner1", baseTime, baseTime + 100, "START",
baseTime + 200));
+ scenarios.add(new TransactionScenario(baseTime + "_owner1.jsonl",
records1));
+
+ createAuditFiles(scenarios);
+
+ Object result = shell.evaluate(() -> "locks audit validate");
+
+ assertAll("Single unclosed transaction shows warning",
+ () -> assertTrue(ShellEvaluationResultUtil.isSuccess(result)),
+ () -> assertNotNull(result.toString()),
+ () -> assertTrue(result.toString().contains("Validation Result:
WARNING")),
+ () -> assertTrue(result.toString().contains("Transactions Validated:
1")),
+ () -> assertTrue(result.toString().contains("Issues Found: 1")));
+ }
+
+ /**
+ * Test validation - Unclosed Transactions (WARNING)
+ */
+ @Test
+ public void testValidateAuditLocksUnclosedTransactions() throws IOException {
+ long baseTime = 1000000L;
+ List<TransactionScenario> scenarios = new ArrayList<>();
+
+ // Transaction 1: Unclosed (effective end at expiration = baseTime + 200)
+ List<AuditRecord> records1 = new ArrayList<>();
+ records1.add(new AuditRecord("owner1", baseTime, baseTime + 100, "START",
baseTime + 200));
+ scenarios.add(new TransactionScenario(baseTime + "_owner1.jsonl",
records1));
+
+ // Transaction 2: Complete, starts after owner1's expiration
+ List<AuditRecord> records2 = new ArrayList<>();
+ records2.add(new AuditRecord("owner2", baseTime + 300, baseTime + 400,
"START", baseTime + 60000));
+ records2.add(new AuditRecord("owner2", baseTime + 300, baseTime + 500,
"END", baseTime + 60000));
+ scenarios.add(new TransactionScenario((baseTime + 300) + "_owner2.jsonl",
records2));
+
+ createAuditFiles(scenarios);
+
+ Object result = shell.evaluate(() -> "locks audit validate");
+
+ assertAll("Unclosed transactions show warning",
+ () -> assertTrue(ShellEvaluationResultUtil.isSuccess(result)),
+ () -> assertNotNull(result.toString()),
+ () -> assertTrue(result.toString().contains("Validation Result:
WARNING")),
+ () -> assertTrue(result.toString().contains("Transactions Validated:
2")),
+ () -> assertTrue(result.toString().contains("Issues Found: 1")),
Review Comment:
this is essentially a dangling lock
--
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]
