Skip to content

HHH-14725 Fix reset handling on BlobProxy #4496

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

HHH-14725 Fix reset handling on BlobProxy #4496

wants to merge 2 commits into from

Conversation

@Naros
Copy link
Member

@Naros Naros commented Dec 19, 2021

@Naros Naros added the 6.0 label Dec 19, 2021
@Naros Naros marked this pull request as draft December 19, 2021 08:53
@Naros Naros marked this pull request as ready for review December 19, 2021 09:57
beikov
beikov approved these changes Dec 20, 2021
View reviewed changes
Copy link
Member

@beikov beikov left a comment

Choose a reason for hiding this comment

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

LGTM

@beikov
Copy link
Member

beikov commented Dec 20, 2021

How about wrapping an InputStream to make sure closing only happens at the end of a TX when we do resource cleanup in Hibernate? If we see that the InputStream doesn't support marking, we could read the contents first into a byte[] and then serve data from there, or alternatively, write into a temporary file and serve data from that.

@oppahansi
Copy link

oppahansi commented Feb 20, 2024 •
edited
Loading

Is there maybe an ETA when this will be merged?
We have encountered an issue with exactly this problem recently.

Our workaround for now is:
We refresh the Entity with the Blob using the EntityManager before operating on the Blob's InputStream/Bytes.

Another workaround is to use a Session and get the Entity with the Blob from the Session and then operate on the InputStream / bytes[].

@Naros
Copy link
Member Author

Naros commented Mar 10, 2024

How about wrapping an InputStream to make sure closing only happens at the end of a TX when we do resource cleanup in Hibernate? If we see that the InputStream doesn't support marking, we could read the contents first into a byte[] and then serve data from there, or alternatively, write into a temporary file and serve data from that.

Hi @beikov do we have another proxy type that does this so I can use that as a reference guide?

@beikov
Copy link
Member

beikov commented Mar 27, 2024

It's fine, let's merge this as soon as tests succeed.

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 27, 2024
View reviewed changes
...rnate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java
final Asset asset = new Asset();
asset.setFileName( "blob.txt" );

final InputStream stream = new BufferedInputStream( Files.newInputStream( path ) );

Check warning

Code scanning / CodeQL

Potential input resource leak

This BufferedInputStream is not always closed on method exit.
...rnate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java
@Test
@Priority(10)
public void initData() {
final Path path = Path.of( getClass().getResource( "./blob.txt" ).getPath() );

Check warning

Code scanning / CodeQL

Unsafe use of getResource

The idiom getClass().getResource() is unsafe for classes that may be extended.
@beikov
Copy link
Member

beikov commented Mar 27, 2024

The test seems to fail @Naros

@gavinking
Copy link
Member

gavinking commented Nov 24, 2024

@Naros What is the status of this one?

@dreab8
Copy link
Member

dreab8 commented Dec 11, 2024

Closing this one in favour of #9402 where I rebased the PR and fixed a problem with the test.

@dreab8 dreab8 closed this Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@beikov beikov beikov approved these changes

Assignees

No one assigned

Labels

6.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Naros @beikov @oppahansi @gavinking @dreab8