From 367d3f0936a44b0aff82325649a69fe34634b153 Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Sat, 18 Dec 2021 23:35:49 -0500 Subject: [PATCH 1/4] HHH-14725 Fix reset handling on BlobProxy --- .../engine/jdbc/proxy/BlobProxy.java | 21 ++++ .../integration/blob/BasicBlobTest.java | 96 +++++++++++++++++++ .../orm/test/envers/integration/blob/blob.txt | 31 ++++++ 3 files changed, 148 insertions(+) create mode 100644 hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java create mode 100644 hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/blob.txt diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java index 953cf49481e3..7eb106991256 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java @@ -40,6 +40,8 @@ public final class BlobProxy implements Blob, BlobImplementer { // no longer necessary. The class name could be updated to reflect this but that would break APIs. private final BinaryStream binaryStream; + private final int markBytes; + private boolean resetAllowed; private boolean needsReset; /** @@ -50,6 +52,8 @@ public final class BlobProxy implements Blob, BlobImplementer { */ private BlobProxy(byte[] bytes) { binaryStream = new ArrayBackedBinaryStream( bytes ); + markBytes = bytes.length + 1; + setStreamMark(); } /** @@ -61,6 +65,18 @@ private BlobProxy(byte[] bytes) { */ private BlobProxy(InputStream stream, long length) { this.binaryStream = new StreamBackedBinaryStream( stream, length ); + this.markBytes = (int) length + 1; + setStreamMark(); + } + + private void setStreamMark() { + if ( binaryStream.getInputStream().markSupported() ) { + binaryStream.getInputStream().mark( markBytes ); + resetAllowed = true; + } + else { + resetAllowed = false; + } } private InputStream getStream() throws SQLException { @@ -76,7 +92,12 @@ public BinaryStream getUnderlyingStream() throws SQLException { private void resetIfNeeded() throws SQLException { try { if ( needsReset ) { + if ( !resetAllowed ) { + throw new SQLException( "Underlying stream does not allow reset" ); + } + binaryStream.getInputStream().reset(); + setStreamMark(); } } catch ( IOException ioe) { diff --git a/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java b/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java new file mode 100644 index 000000000000..1b69fe4f8404 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java @@ -0,0 +1,96 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-or-later + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.envers.integration.blob; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hibernate.testing.transaction.TransactionUtil.doInJPA; +import static org.junit.Assert.fail; + +import java.io.BufferedInputStream; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.Blob; + +import org.hamcrest.Matchers; +import org.hibernate.engine.jdbc.proxy.BlobProxy; +import org.hibernate.envers.Audited; +import org.hibernate.orm.test.envers.BaseEnversJPAFunctionalTestCase; +import org.hibernate.orm.test.envers.Priority; +import org.junit.Test; + +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; + +/** + * @author Chris Cranford + */ +public class BasicBlobTest extends BaseEnversJPAFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { Asset.class }; + } + + @Test + @Priority(10) + public void initData() { + final Path path = Path.of( getClass().getResource( "./blob.txt" ).getPath() ); + doInJPA( this::entityManagerFactory, entityManager -> { + try { + final Asset asset = new Asset(); + asset.setFileName( "blob.txt" ); + + final InputStream stream = new BufferedInputStream( Files.newInputStream( path ) ); + assertThat( stream.markSupported(), Matchers.is( true ) ); + + Blob blob = BlobProxy.generateProxy( stream, Files.size( path ) ); + + asset.setData( blob ); + entityManager.persist( asset ); + } + catch (Exception e) { + e.printStackTrace(); + fail( "Failed to persist the entity" ); + } + } ); + } + + @Audited + @Entity(name = "Asset") + public static class Asset { + @Id + @GeneratedValue + private Integer id; + private String fileName; + private Blob data; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getFileName() { + return fileName; + } + + public void setFileName(String fileName) { + this.fileName = fileName; + } + + public Blob getData() { + return data; + } + + public void setData(Blob data) { + this.data = data; + } + } + +} diff --git a/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/blob.txt b/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/blob.txt new file mode 100644 index 000000000000..7a1354040f95 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/blob.txt @@ -0,0 +1,31 @@ + + + + + + + create-drop + + false + false + + org.hibernate.dialect.H2Dialect + jdbc:h2:mem:envers + org.h2.Driver + sa + + + + + + + + + + + From a886aa1324efce1cc9cc01bdd4cb842c2a2892ea Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Sun, 19 Dec 2021 04:57:47 -0500 Subject: [PATCH 2/4] HHH-14725 Adjust test to use byte array, update Javadoc --- .../hibernate/engine/jdbc/proxy/BlobProxy.java | 5 +++++ .../envers/integration/blob/BasicBlobTest.java | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java index 7eb106991256..68d91a37c33d 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java @@ -120,6 +120,11 @@ public static Blob generateProxy(byte[] bytes) { /** * Generates a BlobImpl proxy using a given number of bytes from an InputStream. * + * Be aware that certain database drivers will automatically close the provided InputStream after the + * contents have been written to the database. This may cause unintended side effects if the entity + * is also audited by Envers. In this case, it's recommended to use {@link #generateProxy(byte[])} + * instead as it isn't affected by this non-standard behavior. + * * @param stream The input stream of bytes to be created as a Blob. * @param length The number of bytes from stream to be written to the Blob. * diff --git a/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java b/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java index 1b69fe4f8404..88bf2273a494 100644 --- a/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java +++ b/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java @@ -47,7 +47,20 @@ public void initData() { final InputStream stream = new BufferedInputStream( Files.newInputStream( path ) ); assertThat( stream.markSupported(), Matchers.is( true ) ); - Blob blob = BlobProxy.generateProxy( stream, Files.size( path ) ); + // We use the method readAllBytes instead of passing the raw stream to the proxy + // since this is the only guaranteed way that will work across all dialects in a + // deterministic way. Postgres and Sybase will automatically close the stream + // after the blob has been written by the driver, which prevents Envers from + // then writing the contents of the stream to the audit table. + // + // If the driver and dialect are known not to close the input stream after the + // contents have been written by the driver, then it's safe to pass the stream + // here instead and the stream will be automatically marked and reset so that + // Envers can serialize the data after Hibernate has done so. Dialects like + // H2, MySQL, Oracle, SQL Server work this way. + // + // + Blob blob = BlobProxy.generateProxy( stream.readAllBytes() ); asset.setData( blob ); entityManager.persist( asset ); From 8ff3e8f3a484552dea1f638efc7f5e18906188d4 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Wed, 11 Dec 2024 12:04:12 +0100 Subject: [PATCH 3/4] HHH-14725 Fixed test --- .../integration/blob/BasicBlobTest.java | 85 ++++++++++++++----- .../orm/test/envers/integration/blob/blob.txt | 0 2 files changed, 65 insertions(+), 20 deletions(-) rename hibernate-envers/src/test/{java => resources}/org/hibernate/orm/test/envers/integration/blob/blob.txt (100%) diff --git a/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java b/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java index 88bf2273a494..4ad979158cce 100644 --- a/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java +++ b/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/BasicBlobTest.java @@ -4,26 +4,28 @@ */ package org.hibernate.orm.test.envers.integration.blob; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hibernate.testing.transaction.TransactionUtil.doInJPA; -import static org.junit.Assert.fail; - -import java.io.BufferedInputStream; -import java.io.InputStream; -import java.nio.file.Files; -import java.nio.file.Path; -import java.sql.Blob; - +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; import org.hamcrest.Matchers; +import org.hibernate.dialect.PostgreSQLDialect; +import org.hibernate.dialect.SQLServerDialect; import org.hibernate.engine.jdbc.proxy.BlobProxy; import org.hibernate.envers.Audited; import org.hibernate.orm.test.envers.BaseEnversJPAFunctionalTestCase; import org.hibernate.orm.test.envers.Priority; +import org.hibernate.testing.SkipForDialect; import org.junit.Test; -import jakarta.persistence.Entity; -import jakarta.persistence.GeneratedValue; -import jakarta.persistence.Id; +import java.io.BufferedInputStream; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.Blob; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hibernate.testing.transaction.TransactionUtil.doInJPA; +import static org.junit.Assert.fail; /** * @author Chris Cranford @@ -32,19 +34,19 @@ public class BasicBlobTest extends BaseEnversJPAFunctionalTestCase { @Override protected Class[] getAnnotatedClasses() { - return new Class[] { Asset.class }; + return new Class[] {Asset.class}; } @Test @Priority(10) - public void initData() { - final Path path = Path.of( getClass().getResource( "./blob.txt" ).getPath() ); + public void testGenerateProxyNoStream() { + final Path path = Path.of( Thread.currentThread().getContextClassLoader() + .getResource( "org/hibernate/orm/test/envers/integration/blob/blob.txt" ).getPath() ); doInJPA( this::entityManagerFactory, entityManager -> { - try { - final Asset asset = new Asset(); - asset.setFileName( "blob.txt" ); + final Asset asset = new Asset(); + asset.setFileName( "blob.txt" ); - final InputStream stream = new BufferedInputStream( Files.newInputStream( path ) ); + try (final InputStream stream = new BufferedInputStream( Files.newInputStream( path ) )) { assertThat( stream.markSupported(), Matchers.is( true ) ); // We use the method readAllBytes instead of passing the raw stream to the proxy @@ -70,6 +72,49 @@ public void initData() { fail( "Failed to persist the entity" ); } } ); + + } + + @Test + @Priority(10) + @SkipForDialect(value = PostgreSQLDialect.class, + comment = "The driver closes the stream, so it cannot be reused by envers") + @SkipForDialect(value = SQLServerDialect.class, + comment = "The driver closes the stream, so it cannot be reused by envers") + public void testGenerateProxyStream() { + final Path path = Path.of( Thread.currentThread().getContextClassLoader() + .getResource( "org/hibernate/orm/test/envers/integration/blob/blob.txt" ).getPath() ); + + try (final InputStream stream = new BufferedInputStream( Files.newInputStream( path ) )) { + doInJPA( this::entityManagerFactory, entityManager -> { + final Asset asset = new Asset(); + asset.setFileName( "blob.txt" ); + + assertThat( stream.markSupported(), Matchers.is( true ) ); + + // We use the method readAllBytes instead of passing the raw stream to the proxy + // since this is the only guaranteed way that will work across all dialects in a + // deterministic way. Postgres and Sybase will automatically close the stream + // after the blob has been written by the driver, which prevents Envers from + // then writing the contents of the stream to the audit table. + // + // If the driver and dialect are known not to close the input stream after the + // contents have been written by the driver, then it's safe to pass the stream + // here instead and the stream will be automatically marked and reset so that + // Envers can serialize the data after Hibernate has done so. Dialects like + // H2, MySQL, Oracle, SQL Server work this way. + // + // + Blob blob = BlobProxy.generateProxy( stream, 9192L ); + + asset.setData( blob ); + entityManager.persist( asset ); + } ); + } + catch (Exception e) { + e.printStackTrace(); + fail( "Failed to persist the entity" ); + } } @Audited diff --git a/hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/blob.txt b/hibernate-envers/src/test/resources/org/hibernate/orm/test/envers/integration/blob/blob.txt similarity index 100% rename from hibernate-envers/src/test/java/org/hibernate/orm/test/envers/integration/blob/blob.txt rename to hibernate-envers/src/test/resources/org/hibernate/orm/test/envers/integration/blob/blob.txt From f93d7c8546e137f78d2b7e2cc44a24d701f0f688 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Wed, 11 Dec 2024 12:33:16 +0100 Subject: [PATCH 4/4] HHH-14725 Fix reset handling on BlobProxy --- .../hibernate/engine/jdbc/proxy/BlobProxy.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java index 68d91a37c33d..12e12dcae0f3 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/proxy/BlobProxy.java @@ -70,8 +70,9 @@ private BlobProxy(InputStream stream, long length) { } private void setStreamMark() { - if ( binaryStream.getInputStream().markSupported() ) { - binaryStream.getInputStream().mark( markBytes ); + final InputStream inputStream = binaryStream.getInputStream(); + if ( inputStream != null && inputStream.markSupported() ) { + inputStream.mark( markBytes ); resetAllowed = true; } else { @@ -92,12 +93,14 @@ public BinaryStream getUnderlyingStream() throws SQLException { private void resetIfNeeded() throws SQLException { try { if ( needsReset ) { - if ( !resetAllowed ) { + final InputStream inputStream = binaryStream.getInputStream(); + if ( !resetAllowed && inputStream != null) { throw new SQLException( "Underlying stream does not allow reset" ); } - - binaryStream.getInputStream().reset(); - setStreamMark(); + if ( inputStream != null ) { + inputStream.reset(); + setStreamMark(); + } } } catch ( IOException ioe) {