Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JCRVLT-448 always close output stream passed to #126

Merged
merged 1 commit into from Feb 25, 2021

Conversation

kwin
Copy link
Member

@kwin kwin commented Feb 24, 2021

JcrPackageManager.rewrap(...) and assemble(...)

JcrPackageManager.rewrap(...) and assemble(...)
@kwin kwin requested a review from tripodsan February 24, 2021 14:09
@sonarcloud
Copy link

sonarcloud bot commented Feb 24, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

60.0% 60.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@tripodsan tripodsan left a comment

Choose a reason for hiding this comment

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

I'm not comfortable with this change. it might have side effects for clients, expecting the output stream to be still open. eg when streaming a http response.

if you really need this, I'd prefer to add something like:

JarExporter withCloseStream(boolean value) {
  this.closeInputStream = value;
  return this
}

so you can use:

JarExporter exp = new JarExporter(out).withCloseStream(true)

@kwin
Copy link
Member Author

kwin commented Feb 25, 2021

it might have side effects for clients, expecting the output stream to be still open. eg when streaming a http response.

The JarExporter already closes the underlying output stream in 90% of the cases. I just made it consistent so that this even works, when open() has not been called.

Copy link
Contributor

@tripodsan tripodsan left a comment

Choose a reason for hiding this comment

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

ok. thanks.

@kwin kwin merged commit e2395b9 into master Feb 25, 2021
@kwin kwin deleted the bugfix/JCRVLT-448-always-close-outputstream branch February 25, 2021 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants