jojochuang commented on code in PR #9399:
URL: https://github.com/apache/ozone/pull/9399#discussion_r2819364786
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java:
##########
@@ -1436,7 +1436,7 @@ public void testSnapDiff() throws Exception {
IOException ioException = assertThrows(IOException.class,
() -> store.snapshotDiff(volume, bucket, snap6,
- snap7, "3", 0, forceFullSnapshotDiff, disableNativeDiff));
+ snap7, "43", 0, forceFullSnapshotDiff, disableNativeDiff));
Review Comment:
yeah this needs fix.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -982,24 +982,18 @@ public int getInFlightSnapshotCount() {
return inFlightSnapshotCount.get();
}
- private int getIndexFromToken(final String token) throws IOException {
+ private String validateToken(final String token) throws IOException {
if (isBlank(token)) {
- return 0;
+ return "";
}
// Validate that token passed in the request is valid integer as of now.
// Later we can change it if we migrate to encrypted or cursor token.
Review Comment:
yes
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -524,15 +565,21 @@ SnapshotDiffReportOzone createPageResponse(
final String bucketName,
final String fromSnapshotName,
final String toSnapshotName,
- final int index,
+ final String index,
final int pageSize
) throws IOException {
- if (index < 0 || index > snapDiffJob.getTotalDiffEntries()
- || pageSize <= 0) {
+ if (!isBlank(index)) {
+
DIFF_TYPE_STRING_MAP.values().stream().filter(index::startsWith).findFirst()
+ .orElseThrow(() -> new IOException("Token " + index + " has invalid
prefix. Valid prefixes: "
+ +
DIFF_TYPE_STRING_MAP.values().stream().map(String::valueOf).collect(Collectors.joining(","))));
+ }
+
+ int idx = isBlank(index) ? 0 : Integer.parseInt(index.substring(1));
Review Comment:
yes; nice to have.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -521,15 +563,21 @@ SnapshotDiffReportOzone createPageResponse(
final String bucketName,
final String fromSnapshotName,
final String toSnapshotName,
- final int index,
+ final String index,
final int pageSize
) throws IOException {
- if (index < 0 || index > snapDiffJob.getTotalDiffEntries()
- || pageSize <= 0) {
+ if (!isBlank(index)) {
+
DIFF_TYPE_STRING_MAP.values().stream().filter(index::startsWith).findFirst()
+ .orElseThrow(() -> new IOException("Token " + index + " has invalid
prefix. Valid prefixes: "
+ +
DIFF_TYPE_STRING_MAP.values().stream().map(String::valueOf).collect(Collectors.joining(","))));
+ }
+
+ int idx = isBlank(index) ? 0 : Integer.parseInt(index.substring(1));
Review Comment:
extract this line and make it more meaningful what this means. (The first
character represents the type of snapshot diff entry and the rest is the index)
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -982,24 +982,18 @@ public int getInFlightSnapshotCount() {
return inFlightSnapshotCount.get();
}
- private int getIndexFromToken(final String token) throws IOException {
+ private String validateToken(final String token) throws IOException {
if (isBlank(token)) {
- return 0;
+ return "";
}
// Validate that token passed in the request is valid integer as of now.
// Later we can change it if we migrate to encrypted or cursor token.
- try {
- int index = Integer.parseInt(token);
- if (index < 0) {
- throw new IOException("Passed token is invalid. Resend the request " +
- "with valid token returned in previous request.");
- }
- return index;
- } catch (NumberFormatException exception) {
+ if (!token.matches("[0-9]+")) {
Review Comment:
yes need this.
--
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]
