nastra commented on code in PR #13400:
URL: https://github.com/apache/iceberg/pull/13400#discussion_r2538919670


##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -3163,4 +3382,459 @@ private static List<HTTPRequest> 
allRequests(RESTCatalogAdapter adapter) {
     verify(adapter, atLeastOnce()).execute(captor.capture(), any(), any(), 
any());
     return captor.getAllValues();
   }
+
+  @Test
+  public void testCancelPlanWithNoActivePlan() {
+    
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithPagination);
+    RESTTableScan restTableScan = restTableScanFor("cancel_test_table");
+
+    // Calling cancel with no active plan should return false
+    assertThat(restTableScan.cancelPlan()).isFalse();
+  }
+
+  @Test
+  public void testCancelPlanEndpointSupport() {
+    
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithPagination);
+    RESTTableScan restTableScan = restTableScanFor("cancel_support_table");
+
+    // Test that cancelPlan method is available and returns false when no plan 
is active
+    assertThat(restTableScan.cancelPlan()).isFalse();
+  }
+
+  @Test
+  public void testCancelPlanMethodAvailability() {
+    
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithPagination);
+    RESTTableScan restTableScan = restTableScanFor("cancel_method_table");
+
+    // Test that cancelPlan method is available and callable
+    // When no plan is active, it should return false
+    assertThat(restTableScan.cancelPlan()).isFalse();
+
+    // Verify the method exists and doesn't throw exceptions when called 
multiple times
+    assertThat(restTableScan.cancelPlan()).isFalse();
+  }
+
+  @Test
+  public void testCancelPlanEndpointPath() {
+    TableIdentifier tableId = TableIdentifier.of("test_namespace", 
"test_table");
+    String planId = "plan-abc-123";
+    ResourcePaths paths = new ResourcePaths("test-prefix");
+
+    // Test that the cancel plan path is generated correctly
+    String cancelPath = paths.plan(tableId, planId);
+
+    assertThat(cancelPath)
+        
.isEqualTo("v1/test-prefix/namespaces/test_namespace/tables/test_table/plan/plan-abc-123");
+
+    // Test with different identifiers
+    TableIdentifier complexId = TableIdentifier.of(Namespace.of("db", 
"schema"), "my_table");
+    String complexPath = paths.plan(complexId, "plan-xyz-789");
+
+    assertThat(complexPath).contains("/plan/plan-xyz-789");
+    assertThat(complexPath).contains("db%1Fschema"); // URL encoded namespace 
separator
+  }
+
+  @Test
+  public void testIteratorCloseTriggersCancel() throws IOException {
+    
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithPagination);
+    Table table = createTableWithScanPlanning("iterator_close_table");
+
+    // Ensure we have a RESTTable with server-side planning enabled
+    assertThat(table).isInstanceOf(RESTTable.class);
+    RESTTable restTable = (RESTTable) table;
+
+    TableScan scan = restTable.newScan();
+    assertThat(scan).isInstanceOf(RESTTableScan.class);
+    boolean cancelled = isCancelled((RESTTableScan) scan);
+    assertThat(cancelled).isFalse(); // No active plan to cancel
+  }
+
+  private static boolean isCancelled(RESTTableScan scan) throws IOException {
+
+    // Get the iterable and iterator
+    CloseableIterable<FileScanTask> iterable = scan.planFiles();
+    CloseableIterator<FileScanTask> iterator = iterable.iterator();
+
+    // Verify we can close the iterator without exceptions
+    // The cancellation callback will be called (though no active plan exists)
+    iterator.close();
+
+    // Verify we can still call cancelPlan on the scan
+    return scan.cancelPlan();
+  }
+
+  @Test
+  public void testMetadataTablesWithRemotePlanning() throws IOException {
+    
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithAllTasks);
+    Table table = createTableWithScanPlanning("metadata_tables_test");
+
+    // Ensure we have a RESTTable with server-side planning enabled
+    assertThat(table).isInstanceOf(RESTTable.class);

Review Comment:
   should this use `restTableFor` where that check is already performed?



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to