Skip to content

Bugfix - Error when trying to download an empty (zero bytes size) file#507

Merged
Or-Geva merged 3 commits intojfrog:masterfrom
Or-Geva:fixemptyfiledownloads
May 30, 2021
Merged

Bugfix - Error when trying to download an empty (zero bytes size) file#507
Or-Geva merged 3 commits intojfrog:masterfrom
Or-Geva:fixemptyfiledownloads

Conversation

@Or-Geva
Copy link
Copy Markdown
Contributor

@Or-Geva Or-Geva commented May 28, 2021

  • All tests passed. If this feature is not already covered by the tests, I added new tests.

All JFrog services ignore response with zero content length, this isn't fit to upload service, as it stops the download flow before reaching file creation. Therefore it was removed.
Fixes jfrog/jenkins-artifactory-plugin#490

@Or-Geva Or-Geva requested review from eyalbe4 and yahavi May 28, 2021 13:39
Copy link
Copy Markdown
Member

@yahavi yahavi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we there is no point in creating a new input stream if the response type is EMPTY.
Moreover, shouldn't we call handleEmptyEntity() in that case?
If you agree with the above, please consider changing processResponse to the following:

private void processResponse(HttpEntity entity) throws IOException {
    if (entity == null || responseType == JFrogServiceResponseType.EMPTY) {
        handleEmptyEntity();
        return;
    }
    try (InputStream stream = entity.getContent()) {
        setResponse(stream);
    }
}

@Or-Geva
Copy link
Copy Markdown
Contributor Author

Or-Geva commented May 28, 2021

I think we there is no point in creating a new input stream if the response type is `EMPTY`.

You're right.

Moreover, shouldn't we call handleEmptyEntity() in that case?

No, JFrogServiceResponseType.EMPTY means that the service expects nothing to be returned (void) and supposed return successfully, whereas handleEmptyEntity handles a service that expects to find one but got no entity.
@yahavi, I propose the following:

private void processResponse(HttpEntity entity) throws IOException {
    if (responseType == JFrogServiceResponseType.EMPTY) {
        return;
    }
    if (entity == null ) {
        handleEmptyEntity();
    }
    try (InputStream stream = entity.getContent()) {
        setResponse(stream);
    }
}

@Or-Geva Or-Geva requested a review from yahavi May 28, 2021 14:50
@yahavi
Copy link
Copy Markdown
Member

yahavi commented May 30, 2021

@Or-Geva OK, but please add Javadoc above handleEmptyEntity explaining when it should and shouldn't be called.

@Or-Geva Or-Geva merged commit 557e3ce into jfrog:master May 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error when trying to download an empty (zero bytes size) file

2 participants