nastra commented on code in PR #13400:
URL: https://github.com/apache/iceberg/pull/13400#discussion_r2538933772
##########
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);
+
+ // Test that metadata tables work with remote planning
+ // Files metadata table
+ Table filesTable =
+ MetadataTableUtils.createMetadataTableInstance(table,
MetadataTableType.FILES);
+ assertThat(filesTable).isNotNull();
+
+ TableScan filesScan = filesTable.newScan();
+ assertThat(filesScan).isNotNull();
+
+ // Verify metadata table scan works (should not use REST scan planning)
+ CloseableIterable<FileScanTask> filesIterable = filesScan.planFiles();
+ List<FileScanTask> filesTasks = Lists.newArrayList(filesIterable);
+ assertThat(filesTasks).isNotEmpty();
+
+ // Snapshots metadata table
+ Table snapshotsTable =
+ MetadataTableUtils.createMetadataTableInstance(table,
MetadataTableType.SNAPSHOTS);
+ assertThat(snapshotsTable).isNotNull();
+
+ TableScan snapshotsScan = snapshotsTable.newScan();
+ CloseableIterable<FileScanTask> snapshotsIterable =
snapshotsScan.planFiles();
+ List<FileScanTask> snapshotsTasks = Lists.newArrayList(snapshotsIterable);
+ assertThat(snapshotsTasks).isNotEmpty();
+
+ // Manifests metadata table
+ Table manifestsTable =
+ MetadataTableUtils.createMetadataTableInstance(table,
MetadataTableType.MANIFESTS);
+ assertThat(manifestsTable).isNotNull();
+
+ TableScan manifestsScan = manifestsTable.newScan();
+ CloseableIterable<FileScanTask> manifestsIterable =
manifestsScan.planFiles();
+ List<FileScanTask> manifestsTasks = Lists.newArrayList(manifestsIterable);
+ assertThat(manifestsTasks).isNotEmpty();
+ }
+
+ @Test
+ public void testIterableCloseTriggersCancel() throws IOException {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithPagination);
+ Table table = createTableWithScanPlanning("iterable_close_test");
+
+ // 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);
+ RESTTableScan restTableScan = (RESTTableScan) scan;
+
+ // Get the iterable
+ CloseableIterable<FileScanTask> iterable = restTableScan.planFiles();
+
+ // Verify we can close the iterable without exceptions
+ // This tests that cancellation callbacks are properly wired through
+ iterable.close();
+
+ // Verify the scan is still functional
+ boolean cancelled = restTableScan.cancelPlan();
+ assertThat(cancelled).isFalse(); // No active plan to cancel
+ }
+
+ @Test
+ public void testRESTScanPlanningWithPositionDeletes() throws IOException {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithAllTasks);
+ Table table = createTableWithScanPlanning("position_deletes_test");
+
+ // Add position deletes that correspond to FILE_A (which was added in
table creation)
+ table.newRowDelta().addDeletes(FILE_A_DELETES).commit();
+
+ // Ensure we have a RESTTable with server-side planning enabled
+ assertThat(table).isInstanceOf(RESTTable.class);
Review Comment:
should this be using `restTableFor`?
--
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]
