From b84032d5c533e67b54b86e3f9a91402e9299ce37 Mon Sep 17 00:00:00 2001 From: Davide D'Alto Date: Wed, 22 May 2024 11:25:21 +0200 Subject: [PATCH 1/2] [#1915] Fix NPE when batching inserts The NullPointerException because the method that converts the generated id from the database to a ResultSet was not implemented. We forgot about it because there was no test for identity generation with batching. --- .../adaptor/impl/ResultSetAdaptor.java | 68 ++++++++++++++++++- .../reactive/pool/BatchingConnection.java | 8 +-- 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/adaptor/impl/ResultSetAdaptor.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/adaptor/impl/ResultSetAdaptor.java index b481fdf01..c9a021a4a 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/adaptor/impl/ResultSetAdaptor.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/adaptor/impl/ResultSetAdaptor.java @@ -44,6 +44,7 @@ import io.vertx.sqlclient.PropertyKind; import io.vertx.sqlclient.Row; import io.vertx.sqlclient.RowSet; +import io.vertx.sqlclient.Tuple; import io.vertx.sqlclient.desc.ColumnDescriptor; import io.vertx.sqlclient.impl.RowBase; @@ -56,13 +57,71 @@ */ public class ResultSetAdaptor implements ResultSet { - private final Iterator iterator; + private final Iterator iterator; private final List columnDescriptors; private final List columnNames; private Row row; private boolean wasNull; + public ResultSetAdaptor(Object id, Class idClass, String columnName) { + requireNonNull( id ); + this.iterator = List.of( new RowAdaptor( id, idClass, columnName ) ).iterator(); + this.columnNames = columnName == null ? emptyList() : List.of( columnName ); + this.columnDescriptors = List.of( toColumnDescriptor( idClass, columnName ) ); + } + + private static class RowAdaptor implements Row { + private final Object id; + private final Class idClass; + private final String columnName; + + public RowAdaptor(Object id, Class idClass, String columnName) { + this.id = id; + this.idClass = idClass; + this.columnName = columnName; + } + + @Override + public Object getValue(String column) { + return id; + } + + @Override + public String getColumnName(int pos) { + return columnName; + } + + @Override + public int getColumnIndex(String column) { + return 0; + } + + @Override + public Object getValue(int pos) { + return id; + } + + @Override + public Tuple addValue(Object value) { + return null; + } + + @Override + public int size() { + return 1; + } + + @Override + public void clear() { + } + + @Override + public List> types() { + return List.of( idClass ); + } + } + public ResultSetAdaptor(RowSet rows) { requireNonNull( rows ); this.iterator = rows.iterator(); @@ -83,7 +142,11 @@ private ResultSetAdaptor(RowSet rows, Row row, String idColumnName, Class idClass, String idColumnName) { + return new ColumnDescriptor() { @Override public String name() { return idColumnName; @@ -104,7 +167,6 @@ public JDBCType jdbcType() { return null; } }; - this.columnDescriptors = List.of( columnDescriptor ); } private static class RowFromId extends RowBase { diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/BatchingConnection.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/BatchingConnection.java index 0d5234129..88062bc0b 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/BatchingConnection.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/BatchingConnection.java @@ -12,6 +12,8 @@ import java.util.concurrent.CompletionStage; +import org.hibernate.reactive.adaptor.impl.ResultSetAdaptor; + import io.vertx.sqlclient.spi.DatabaseMetadata; import static org.hibernate.reactive.util.impl.CompletionStages.voidFuture; @@ -169,11 +171,7 @@ public CompletionStage insertAndSelectIdentifierAsResultSet( Class idClass, String idColumnName) { return insertAndSelectIdentifier( sql, paramValues, idClass, idColumnName ) - .thenApply( this::convertToResultSet ); - } - - private ResultSet convertToResultSet(Object o) { - return null; + .thenApply( id -> new ResultSetAdaptor( id, idClass, idColumnName ) ); } public CompletionStage select(String sql) { From 3cb9eebee11c37c7946000c5192d7e1a4a19041e Mon Sep 17 00:00:00 2001 From: Davide D'Alto Date: Wed, 22 May 2024 10:44:06 +0200 Subject: [PATCH 2/2] [#1915] Test identity generation with bacthing --- .../IdentityGenerationWithBatchingTest.java | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 hibernate-reactive-core/src/test/java/org/hibernate/reactive/IdentityGenerationWithBatchingTest.java diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/IdentityGenerationWithBatchingTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/IdentityGenerationWithBatchingTest.java new file mode 100644 index 000000000..acad8feba --- /dev/null +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/IdentityGenerationWithBatchingTest.java @@ -0,0 +1,107 @@ +/* Hibernate, Relational Persistence for Idiomatic Java + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright: Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.reactive; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.cfg.Configuration; + +import org.junit.jupiter.api.Test; + +import io.vertx.junit5.VertxTestContext; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.Table; + +import static org.assertj.core.api.Assertions.assertThat; + + +public class IdentityGenerationWithBatchingTest extends BaseReactiveTest { + + @Override + protected Collection> annotatedEntities() { + return List.of( Book.class ); + } + + @Override + protected Configuration constructConfiguration() { + Configuration configuration = super.constructConfiguration(); + configuration.setProperty( AvailableSettings.STATEMENT_BATCH_SIZE, "5" ); + return configuration; + } + + @Test + public void test(VertxTestContext context) { + String[] titles = { + "Around the World in Eighty Days", + "The Book of M: A Novel", + "The Poppy War", + "The poetry home repair manual", + "Relish" + }; + test( context, getMutinySessionFactory() + .withTransaction( s -> s.persistAll( asBooks( titles ) ) ) + .chain( () -> getMutinySessionFactory().withSession( s -> s.createSelectionQuery( "from Book order by id ASC", Book.class ).getResultList() ) ) + .invoke( results -> { + String[] resultTitles = results.stream().map( Book::getTitle ).toArray( String[]::new ); + assertThat( resultTitles ).containsExactly( titles ); + Long savedId = 0L; + for ( Book book : results ) { + assertThat( book.getId() ).isNotNull().isGreaterThan( savedId ); + savedId = book.getId(); + } + } ) + ); + } + + private Book[] asBooks(String[] titles) { + return Arrays.asList( titles ).stream().map( Book::new ).toArray( Book[]::new ); + } + + @Entity(name = "Book") + @Table(name = "books") + static class Book { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + + private String title; + + public Book() { + } + + public Book(String title) { + this.title = title; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getTitle() { + return title; + } + + public void setTitle(String title) { + this.title = title; + } + + @Override + public String toString() { + return id + ":" + title; + } + } +}