Merge pull request #27188 from vespa-engine/vekterli/handle-open-visit-timestamp-ranges

Properly handle open visitor timestamp ranges in request parameters

2 files changed, 39 insertions, 4 deletions

diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
index eb13bf634ee..e6d5ea48e8f 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
@@ -1219,10 +1219,12 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
         parameters.setPriority(DocumentProtocol.Priority.NORMAL_4);
 
         getProperty(request, FROM_TIMESTAMP, unsignedLongParser).ifPresent(parameters::setFromTimestamp);
-        getProperty(request, TO_TIMESTAMP, unsignedLongParser).ifPresent(parameters::setToTimestamp);
-        if (Long.compareUnsigned(parameters.getFromTimestamp(), parameters.getToTimestamp()) > 0) {
-            throw new IllegalArgumentException("toTimestamp must be greater than, or equal to, fromTimestamp");
-        }
+        getProperty(request, TO_TIMESTAMP, unsignedLongParser).ifPresent(ts -> {
+            parameters.setToTimestamp(ts);
+            if (Long.compareUnsigned(parameters.getFromTimestamp(), parameters.getToTimestamp()) > 0) {
+                throw new IllegalArgumentException("toTimestamp must be greater than, or equal to, fromTimestamp");
+            }
+        });
 
         StorageCluster storageCluster = resolveCluster(cluster, clusters);
         parameters.setRoute(storageCluster.name());
diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
index c6cbd20651a..e8f42fbecfa 100644
--- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
+++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
@@ -956,6 +956,39 @@ public class DocumentV1ApiTest {
         driver.close();
     }
 
+    private void doTestVisitRequestWithParams(String httpReqParams, Consumer<VisitorParameters> paramChecker) {
+        try (var driver = new RequestHandlerTestDriver(handler)) {
+            access.expect(parameters -> {
+                paramChecker.accept(parameters);
+                parameters.getControlHandler().onDone(VisitorControlHandler.CompletionCode.SUCCESS, "great success");
+            });
+            var response = driver.sendRequest("http://localhost/document/v1/?cluster=content&%s".formatted(httpReqParams));
+            assertSameJson("""
+                            {
+                              "pathId": "/document/v1/",
+                              "documents": [ ],
+                              "documentCount": 0
+                            }""",
+                    response.readAll());
+            assertEquals(200, response.getStatus());
+        }
+    }
+
+    @Test
+    public void visit_timestamp_ranges_can_be_open_in_both_ends() {
+        // Only specifying fromTimestamp; visit up to current time
+        doTestVisitRequestWithParams("fromTimestamp=1234", (params) -> {
+            assertEquals(params.getFromTimestamp(), 1234);
+            assertEquals(params.getToTimestamp(), 0); // Means "current wall clock time" when it hits storage
+        });
+
+        // Only specifying toTimestamp; visit all docs up to this time point
+        doTestVisitRequestWithParams("toTimestamp=2345", (params) -> {
+            assertEquals(params.getFromTimestamp(), 0); // The dawn of time(tm)
+            assertEquals(params.getToTimestamp(), 2345);
+        });
+    }
+
     @Test
     public void testThroughput() throws InterruptedException {
         DocumentOperationExecutorConfig executorConfig = new DocumentOperationExecutorConfig.Builder().build();