HDDS-2501. Sonar: Fix issues found in the ObjectEndpoint class. #198
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -233,16 +233,19 @@ public Response get( | |
| String rangeHeaderVal = headers.getHeaderString(RANGE_HEADER); | ||
| RangeHeader rangeHeader = null; | ||
|
|
||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("range Header provided value is {}", rangeHeaderVal); | ||
| } | ||
|
|
||
| if (rangeHeaderVal != null) { | ||
| rangeHeader = RangeHeaderParserUtil.parseRangeHeader(rangeHeaderVal, | ||
| length); | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("range Header provided value is {}", rangeHeader); | ||
| } | ||
| if (rangeHeader.isInValidRange()) { | ||
| throw S3ErrorTable.newError( | ||
| S3ErrorTable.INVALID_RANGE, rangeHeaderVal); | ||
| } | ||
| } | ||
| ResponseBuilder responseBuilder; | ||
|
|
@@ -351,8 +354,7 @@ public Response head( | |
| .header("Content-Length", key.getDataSize()) | ||
| .header("Content-Type", "binary/octet-stream"); | ||
| addLastModifiedDate(response, key); | ||
| return response.build(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -467,8 +469,8 @@ public Response initializeMultipartUpload( | |
| return Response.status(Status.OK).entity( | ||
| multipartUploadInitiateResponse).build(); | ||
| } catch (IOException ex) { | ||
| LOG.error("Error in Initiate Multipart Upload Request for bucket: {}, " + | ||
| "key: {}", bucket, key, ex); | ||
| throw ex; | ||
| } | ||
| } | ||
|
|
@@ -494,8 +496,9 @@ public Response completeMultipartUpload(@PathParam("bucket") String bucket, | |
| for (CompleteMultipartUploadRequest.Part part : partList) { | ||
| partsMap.put(part.getPartNumber(), part.geteTag()); | ||
| } | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("Parts map {}", partsMap); | ||
| } | ||
|
|
||
| omMultipartUploadCompleteInfo = ozoneBucket.completeMultipartUpload( | ||
| key, uploadID, partsMap); | ||
|
|
@@ -510,27 +513,18 @@ public Response completeMultipartUpload(@PathParam("bucket") String bucket, | |
| return Response.status(Status.OK).entity(completeMultipartUploadResponse) | ||
| .build(); | ||
| } catch (OMException ex) { | ||
| LOG.error("Error in Complete Multipart Upload Request for bucket: {}, " + | ||
| ", key: {}", bucket, key, ex); | ||
| if (ex.getResult() == ResultCodes.INVALID_PART) { | ||
| throw S3ErrorTable.newError(S3ErrorTable.INVALID_PART, key); | ||
| } else if (ex.getResult() == ResultCodes.INVALID_PART_ORDER) { | ||
| throw S3ErrorTable.newError(S3ErrorTable.INVALID_PART_ORDER, key); | ||
| } else if (ex.getResult() == ResultCodes.NO_SUCH_MULTIPART_UPLOAD_ERROR) { | ||
| throw S3ErrorTable.newError(NO_SUCH_UPLOAD, uploadID); | ||
| } else if (ex.getResult() == ResultCodes.ENTITY_TOO_SMALL) { | ||
| throw S3ErrorTable.newError(ENTITY_TOO_SMALL, key); | ||
| } else if(ex.getResult() == ResultCodes.INVALID_REQUEST) { | ||
| OS3Exception os3Exception = S3ErrorTable.newError(INVALID_REQUEST, key); | ||
| os3Exception.setErrorMessage("An error occurred (InvalidRequest) " + | ||
| "when calling the CompleteMultipartUpload operation: You must " + | ||
| "specify at least one part"); | ||
|
|
@@ -546,39 +540,39 @@ private Response createMultipartKey(String bucket, String key, long length, | |
| throws IOException, OS3Exception { | ||
| try { | ||
| OzoneBucket ozoneBucket = getBucket(bucket); | ||
| String copyHeader; | ||
| OmMultipartCommitUploadPartInfo omMultipartCommitUploadPartInfo; | ||
| try (OzoneOutputStream ozoneOutputStream = ozoneBucket.createMultipartKey( | ||
| key, length, partNumber, uploadID)) { | ||
| copyHeader = headers.getHeaderString(COPY_SOURCE_HEADER); | ||
| if (copyHeader != null) { | ||
| Pair<String, String> result = parseSourceHeader(copyHeader); | ||
|
|
||
| String sourceBucket = result.getLeft(); | ||
| String sourceKey = result.getRight(); | ||
|
|
||
| try (OzoneInputStream sourceObject = | ||
| getBucket(sourceBucket).readKey(sourceKey)) { | ||
|
|
||
| String range = | ||
| headers.getHeaderString(COPY_SOURCE_HEADER_RANGE); | ||
| if (range != null) { | ||
| RangeHeader rangeHeader = | ||
| RangeHeaderParserUtil.parseRangeHeader(range, 0); | ||
| IOUtils.copyLarge(sourceObject, ozoneOutputStream, | ||
| rangeHeader.getStartOffset(), | ||
| rangeHeader.getEndOffset() - rangeHeader.getStartOffset()); | ||
|
|
||
| } else { | ||
| IOUtils.copy(sourceObject, ozoneOutputStream); | ||
| } | ||
| } | ||
| } else { | ||
| IOUtils.copy(body, ozoneOutputStream); | ||
| } | ||
| omMultipartCommitUploadPartInfo = ozoneOutputStream | ||
| .getCommitUploadPartInfo(); | ||
|
Contributor
There was a problem hiding this comment. It turns out this should be called only after |
||
| } | ||
| String eTag = omMultipartCommitUploadPartInfo.getPartName(); | ||
|
|
||
| if (copyHeader != null) { | ||
|
|
@@ -761,7 +755,7 @@ public static Pair<String, String> parseSourceHeader(String copyHeader) | |
| if (header.startsWith("/")) { | ||
| header = copyHeader.substring(1); | ||
| } | ||
| int pos = header.indexOf('/'); | ||
| if (pos == -1) { | ||
| OS3Exception ex = S3ErrorTable.newError(S3ErrorTable | ||
| .INVALID_ARGUMENT, header); | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrapping every
LOG.debugin anifincreases method complexity and may trigger a new Sonar warning above a threshold (it does here). I think log statements are safe to call unconditionally (both for performance and for Sonar) as long as their arguments are not eagerly evaluated. Using placeholders and passing existing objects as arguments should be OK.