ArafatKhan2198 commented on code in PR #6969:
URL: https://github.com/apache/ozone/pull/6969#discussion_r1803290005
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightSearchEndpoint.java:
##########
@@ -110,25 +116,17 @@ public
OMDBInsightSearchEndpoint(OzoneStorageContainerManager reconSCM,
@GET
@Path("/open/search")
public Response searchOpenKeys(
- @DefaultValue(DEFAULT_START_PREFIX) @QueryParam("startPrefix")
+ @DefaultValue(RECON_OM_INSIGHTS_DEFAULT_START_PREFIX)
@QueryParam("startPrefix")
String startPrefix,
- @DefaultValue(RECON_OPEN_KEY_DEFAULT_SEARCH_LIMIT) @QueryParam("limit")
+ @DefaultValue(RECON_OM_INSIGHTS_DEFAULT_SEARCH_LIMIT)
@QueryParam("limit")
int limit,
- @DefaultValue(RECON_OPEN_KEY_SEARCH_DEFAULT_PREV_KEY)
@QueryParam("prevKey") String prevKey) throws IOException {
+ @DefaultValue(RECON_OM_INSIGHTS_DEFAULT_SEARCH_PREV_KEY)
@QueryParam("prevKey")
+ String prevKey) throws IOException {
try {
- // Ensure startPrefix is not null or empty and starts with '/'
- if (startPrefix == null || startPrefix.length() == 0) {
- return createBadRequestResponse(
- "Invalid startPrefix: Path must be at the bucket level or
deeper.");
- }
- startPrefix = startPrefix.startsWith("/") ? startPrefix : "/" +
startPrefix;
-
- // Split the path to ensure it's at least at the bucket level
- String[] pathComponents = startPrefix.split("/");
- if (pathComponents.length < 3 || pathComponents[2].isEmpty()) {
- return createBadRequestResponse(
- "Invalid startPrefix: Path must be at the bucket level or
deeper.");
+ // Validate the request parameters
+ if (!validateStartPrefix(startPrefix)) {
Review Comment:
I believe there's no need to check if the startPrefix is blank because the
`validateStartPrefix` method already handles that. For instance, if startPrefix
is an empty string, the validateStartPrefix method first checks whether it
starts with a "/" or not. Since it doesn't in this case, it simply adds the "/"
and then proceeds with the validation. Therefore, checking for blank values
separately shouldn't be necessary.
```
public static boolean validateStartPrefix(String startPrefix) {
// Ensure startPrefix starts with '/' for non-empty values
startPrefix = startPrefix.startsWith("/") ? startPrefix : "/" +
startPrefix;
// Split the path to ensure it's at least at the bucket level
(volume/bucket).
String[] pathComponents = startPrefix.split("/");
if (pathComponents.length < 3 || pathComponents[2].isEmpty()) {
return false; // Invalid if not at bucket level or deeper
}
return true;
}
```
@devmadhuu
--
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]
