adoroszlai commented on code in PR #9540:
URL: https://github.com/apache/ozone/pull/9540#discussion_r2649157811
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java:
##########
@@ -606,6 +606,7 @@ public Response head(
S3GAction s3GAction = S3GAction.HEAD_KEY;
OzoneKey key;
+ RangeHeader rangeHeader = null;
Review Comment:
Please move to the code block where header is parsed.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectHead.java:
##########
@@ -185,4 +192,97 @@ public void
testHeadWhenKeyIsAFileAndKeyPathEndsWithASlash()
assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatus());
bucket.deleteKey(keyPath);
}
+
+ @Test
+ public void testHeadWithRangeHeader() throws Exception {
+ //GIVEN
+ String value = "0123456789";
+ OzoneOutputStream out = bucket.createKey("key1",
+ value.getBytes(UTF_8).length,
+ ReplicationConfig.fromTypeAndFactor(ReplicationType.RATIS,
+ ReplicationFactor.ONE), new HashMap<>());
+ out.write(value.getBytes(UTF_8));
+ out.close();
+
+ HttpHeaders headers = mock(HttpHeaders.class);
+ when(headers.getHeaderString(RANGE_HEADER)).thenReturn("bytes=0-0");
+ keyEndpoint = EndpointBuilder.newObjectEndpointBuilder()
+ .setClient(client)
+ .setHeaders(headers)
+ .build();
+
+ //WHEN
+ Response response = keyEndpoint.head(bucketName, "key1");
+
+ //THEN
+ assertEquals(206, response.getStatus());
+ assertEquals("1", response.getHeaderString("Content-Length"));
+ assertEquals(String.format("bytes 0-0/%d", value.length()),
+ response.getHeaderString("Content-Range"));
+ assertEquals("bytes", response.getHeaderString("Accept-Ranges"));
+
+ // Test range from start to end
+ when(headers.getHeaderString(RANGE_HEADER)).thenReturn("bytes=0-");
+ response = keyEndpoint.head(bucketName, "key1");
+ assertEquals(206, response.getStatus());
+ assertEquals(String.valueOf(value.length()),
+ response.getHeaderString("Content-Length"));
+ assertEquals(String.format("bytes 0-%d/%d", value.length() - 1,
value.length()),
+ response.getHeaderString("Content-Range"));
+
+ bucket.deleteKey("key1");
+ }
+
+ @Test
+ public void testHeadWithInvalidRangeHeader() throws Exception {
+ //GIVEN
+ String value = "0123456789"; // length = 10
+ OzoneOutputStream out = bucket.createKey("key1",
+ value.getBytes(UTF_8).length,
+ ReplicationConfig.fromTypeAndFactor(ReplicationType.RATIS,
+ ReplicationFactor.ONE), new HashMap<>());
+ out.write(value.getBytes(UTF_8));
+ out.close();
+
+ HttpHeaders headers = mock(HttpHeaders.class);
+ // Invalid range: both start and end are beyond file length
+ // According to RangeHeaderParserUtil, bytes=11-10 with length=10 will
trigger isInValidRange()
+ when(headers.getHeaderString(RANGE_HEADER)).thenReturn("bytes=11-10");
+ keyEndpoint = EndpointBuilder.newObjectEndpointBuilder()
+ .setClient(client)
+ .setHeaders(headers)
+ .build();
+
+ //WHEN/THEN
+ OS3Exception ex = assertThrows(OS3Exception.class,
+ () -> keyEndpoint.head(bucketName, "key1"));
+ assertEquals("InvalidRange", ex.getCode());
+ assertEquals(416, ex.getHttpCode());
Review Comment:
Can simplify using `EndpointTestUtils`:
```java
assertErrorResponse(S3ErrorTable.INVALID_RANGE, () ->
keyEndpoint.head(bucketName, "key1"));
```
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectHead.java:
##########
@@ -185,4 +192,97 @@ public void
testHeadWhenKeyIsAFileAndKeyPathEndsWithASlash()
assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatus());
bucket.deleteKey(keyPath);
}
+
+ @Test
+ public void testHeadWithRangeHeader() throws Exception {
+ //GIVEN
+ String value = "0123456789";
+ OzoneOutputStream out = bucket.createKey("key1",
+ value.getBytes(UTF_8).length,
+ ReplicationConfig.fromTypeAndFactor(ReplicationType.RATIS,
+ ReplicationFactor.ONE), new HashMap<>());
+ out.write(value.getBytes(UTF_8));
+ out.close();
Review Comment:
Please move key creation to `setup()` to avoid duplication. While at that,
please change to:
```java
try (OutputStream out = ...) {
out.write(...)
}
```
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectHead.java:
##########
@@ -185,4 +192,97 @@ public void
testHeadWhenKeyIsAFileAndKeyPathEndsWithASlash()
assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatus());
bucket.deleteKey(keyPath);
}
+
+ @Test
+ public void testHeadWithRangeHeader() throws Exception {
+ //GIVEN
+ String value = "0123456789";
+ OzoneOutputStream out = bucket.createKey("key1",
+ value.getBytes(UTF_8).length,
+ ReplicationConfig.fromTypeAndFactor(ReplicationType.RATIS,
+ ReplicationFactor.ONE), new HashMap<>());
+ out.write(value.getBytes(UTF_8));
+ out.close();
+
+ HttpHeaders headers = mock(HttpHeaders.class);
+ when(headers.getHeaderString(RANGE_HEADER)).thenReturn("bytes=0-0");
+ keyEndpoint = EndpointBuilder.newObjectEndpointBuilder()
+ .setClient(client)
+ .setHeaders(headers)
+ .build();
+
+ //WHEN
+ Response response = keyEndpoint.head(bucketName, "key1");
Review Comment:
Please define constant for `"key1"` and reuse in creation, lookup and
cleanup.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectHead.java:
##########
@@ -185,4 +192,97 @@ public void
testHeadWhenKeyIsAFileAndKeyPathEndsWithASlash()
assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatus());
bucket.deleteKey(keyPath);
}
+
+ @Test
+ public void testHeadWithRangeHeader() throws Exception {
+ //GIVEN
+ String value = "0123456789";
+ OzoneOutputStream out = bucket.createKey("key1",
+ value.getBytes(UTF_8).length,
+ ReplicationConfig.fromTypeAndFactor(ReplicationType.RATIS,
+ ReplicationFactor.ONE), new HashMap<>());
+ out.write(value.getBytes(UTF_8));
+ out.close();
+
+ HttpHeaders headers = mock(HttpHeaders.class);
+ when(headers.getHeaderString(RANGE_HEADER)).thenReturn("bytes=0-0");
+ keyEndpoint = EndpointBuilder.newObjectEndpointBuilder()
+ .setClient(client)
+ .setHeaders(headers)
+ .build();
Review Comment:
Please create and set `headers` on `keyEndpoint` in `setup()` and do not
create new endpoint in test cases. This also makes it unnecessary to create
field for `OzoneClient`.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectHead.java:
##########
@@ -185,4 +192,97 @@ public void
testHeadWhenKeyIsAFileAndKeyPathEndsWithASlash()
assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatus());
bucket.deleteKey(keyPath);
}
+
+ @Test
+ public void testHeadWithRangeHeader() throws Exception {
+ //GIVEN
+ String value = "0123456789";
+ OzoneOutputStream out = bucket.createKey("key1",
+ value.getBytes(UTF_8).length,
+ ReplicationConfig.fromTypeAndFactor(ReplicationType.RATIS,
+ ReplicationFactor.ONE), new HashMap<>());
+ out.write(value.getBytes(UTF_8));
+ out.close();
+
+ HttpHeaders headers = mock(HttpHeaders.class);
+ when(headers.getHeaderString(RANGE_HEADER)).thenReturn("bytes=0-0");
+ keyEndpoint = EndpointBuilder.newObjectEndpointBuilder()
+ .setClient(client)
+ .setHeaders(headers)
+ .build();
+
+ //WHEN
+ Response response = keyEndpoint.head(bucketName, "key1");
+
+ //THEN
+ assertEquals(206, response.getStatus());
+ assertEquals("1", response.getHeaderString("Content-Length"));
+ assertEquals(String.format("bytes 0-0/%d", value.length()),
+ response.getHeaderString("Content-Range"));
+ assertEquals("bytes", response.getHeaderString("Accept-Ranges"));
Review Comment:
Please use constants from `HttpHeaders` for `"Content-Length"`, etc..
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectHead.java:
##########
@@ -185,4 +192,97 @@ public void
testHeadWhenKeyIsAFileAndKeyPathEndsWithASlash()
assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatus());
bucket.deleteKey(keyPath);
}
+
+ @Test
+ public void testHeadWithRangeHeader() throws Exception {
+ //GIVEN
+ String value = "0123456789";
+ OzoneOutputStream out = bucket.createKey("key1",
+ value.getBytes(UTF_8).length,
+ ReplicationConfig.fromTypeAndFactor(ReplicationType.RATIS,
+ ReplicationFactor.ONE), new HashMap<>());
+ out.write(value.getBytes(UTF_8));
+ out.close();
+
+ HttpHeaders headers = mock(HttpHeaders.class);
+ when(headers.getHeaderString(RANGE_HEADER)).thenReturn("bytes=0-0");
+ keyEndpoint = EndpointBuilder.newObjectEndpointBuilder()
+ .setClient(client)
+ .setHeaders(headers)
+ .build();
+
+ //WHEN
+ Response response = keyEndpoint.head(bucketName, "key1");
+
+ //THEN
+ assertEquals(206, response.getStatus());
+ assertEquals("1", response.getHeaderString("Content-Length"));
+ assertEquals(String.format("bytes 0-0/%d", value.length()),
+ response.getHeaderString("Content-Range"));
+ assertEquals("bytes", response.getHeaderString("Accept-Ranges"));
+
+ // Test range from start to end
+ when(headers.getHeaderString(RANGE_HEADER)).thenReturn("bytes=0-");
+ response = keyEndpoint.head(bucketName, "key1");
+ assertEquals(206, response.getStatus());
+ assertEquals(String.valueOf(value.length()),
+ response.getHeaderString("Content-Length"));
+ assertEquals(String.format("bytes 0-%d/%d", value.length() - 1,
value.length()),
+ response.getHeaderString("Content-Range"));
+
+ bucket.deleteKey("key1");
Review Comment:
Please move to `@AfterEach` method to ensure cleanup even in case of test
failure.
--
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]
