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

HHH-17948 add Session.findAll(), StatelessSession.getAll() #8166

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ default MultiIdentifierLoadAccess<T> with(RootGraph<T> graph) {

/**
* Should {@link #multiLoad} return entity instances that have been
* {@link Session#remove(Object) marked for removal} in the current
* session, but not yet {@code delete}d in the database?
* {@linkplain Session#remove(Object) marked for removal} in the
* current session, but not yet {@code delete}d in the database?
* <p>
* By default, instances marked for removal are replaced by null in
* the returned list of entities when {@link #enableOrderedReturn}
Expand Down
24 changes: 22 additions & 2 deletions hibernate-core/src/main/java/org/hibernate/Session.java
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,26 @@ public interface Session extends SharedSessionContract, EntityManager {
*/
void clear();

/**
* Return the persistent instances of the given entity class with the given identifiers
* as a list. The position of an instance in the list matches the position of its identifier
* in the given array, and the list contains a null value if there is no persistent instance
* matching a given identifier. If an instance is already associated with the session, that
* instance is returned. This method never returns an uninitialized instance.
* <p>
* Every object returned by {@code findAll()} is either an unproxied instance of the given
* entity class, or a fully-fetched proxy object.
*
* @param entityType the entity type
* @param ids the identifiers
*
* @return an ordered list of persistent instances, with null elements representing missing
* entities
*
* @since 6.5
*/
<E> List<E> findAll(Class<E> entityType, Object... ids);
gavinking marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

So just a convenience for byMultipleIds(Class<?>).multiLoad(Object...)? Maybe it's just me, but I don't think we should start "duplicating" API for convenience.

Copy link
Member

Choose a reason for hiding this comment

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

So just a convenience for byMultipleIds(Class<?>).multiLoad(Object...)? Maybe it's just me, but I don't think we should start "duplicating" API for convenience.

I generally very much agree with that. But I think a single convenience method for the common case is perfectly fine. Now if/when this starts getting overloaded, then I have a big problem with this

Copy link
Member Author

Choose a reason for hiding this comment

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

I also usually agree, but the thing is that JPA 3.2 has gone in a quite different direction with FindOptions.

I initially proposed something more like our XxxxLoadAccess interfaces, but the XxxOptions objects were the direction Lukas wanted to take, and he talked me into it. Ultimately I think this was the right choice.

Therefore the concept here, as described in the issue, is that in H7 we would allow findAll() to accept FindOptions. Yes, it's still "duplication" in some sense, but justifiable duplication, I would say.


/**
* Return the persistent instance of the given entity class with the given identifier,
* or null if there is no such persistent instance. If the instance is already associated
Expand All @@ -978,8 +998,8 @@ public interface Session extends SharedSessionContract, EntityManager {
* <p>
* This operation is very similar to {@link #find(Class, Object)}.
* <p>
* The object returned by {@code get()} or {@code find() } is either an unproxied instance
* of the given entity class, of a fully-fetched proxy object.
* The object returned by {@code get()} or {@code find()} is either an unproxied instance
* of the given entity class, or a fully-fetched proxy object.
* <p>
* This operation requests {@link LockMode#NONE}, that is, no lock, allowing the object
* to be retrieved from the cache without the cost of database access. However, if it is
Expand Down
19 changes: 19 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/StatelessSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import jakarta.persistence.EntityGraph;
import org.hibernate.graph.GraphSemantic;

import java.util.List;

/**
* A command-oriented API often used for performing bulk operations against
* the database. A stateless session has no persistence context, and always
Expand Down Expand Up @@ -198,6 +200,23 @@ public interface StatelessSession extends SharedSessionContract {
*/
<T> T get(EntityGraph<T> graph, GraphSemantic graphSemantic, Object id, LockMode lockMode);

/**
* Retrieve multiple rows, returning entity instances in a
* list where the position of an instance in the list matches
* the position of its identifier in the given array, and the
* list contains a null value if there is no persistent
* instance matching a given identifier.
*
* @param entityClass The class of the entity to retrieve
* @param ids The ids of the entities to retrieve
*
* @return an ordered list of detached entity instances, with
* null elements representing missing entities
*
* @since 6.5
*/
<T> List<T> getAll(Class<T> entityClass, Object... ids);

/**
* Refresh the entity instance state from the database.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.hibernate.engine.jdbc.connections.spi.JdbcConnectionAccess;
import org.hibernate.engine.jdbc.spi.JdbcCoordinator;
import org.hibernate.engine.jdbc.spi.JdbcServices;
import org.hibernate.event.spi.AutoFlushEvent;
import org.hibernate.event.spi.EventManager;
import org.hibernate.event.spi.DeleteContext;
import org.hibernate.event.spi.EventSource;
Expand Down Expand Up @@ -1013,6 +1012,11 @@ public void detach(Object entity) {
delegate.detach( entity );
}

@Override
public <E> List<E> findAll(Class<E> entityType, Object... ids) {
return delegate.findAll( entityType, ids );
}

@Override
public <T> T get(Class<T> theClass, Object id) {
return delegate.get( theClass, id );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,11 @@ public void clear() {
this.lazySession.get().clear();
}

@Override
public <E> List<E> findAll(Class<E> entityType, Object... ids) {
return this.lazySession.get().findAll( entityType, ids );
}

@Override
public <T> T get(Class<T> entityType, Object id) {
return this.lazySession.get().get( entityType, id );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.sql.NClob;
import java.sql.SQLException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -1019,6 +1020,11 @@ public Object load(String entityName, Object id) throws HibernateException {
return this.byId( entityName ).getReference( id );
}

@Override
public <E> List<E> findAll(Class<E> entityType, Object... ids) {
return this.byMultipleIds( entityType ).multiLoad( ids );
}

@Override
public <T> T get(Class<T> entityClass, Object id) throws HibernateException {
return this.byId( entityClass ).load( id );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
*/
package org.hibernate.internal;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import org.hibernate.CacheMode;
import org.hibernate.FlushMode;
Expand Down Expand Up @@ -39,11 +43,14 @@
import org.hibernate.persister.collection.CollectionPersister;
import org.hibernate.persister.entity.EntityPersister;
import org.hibernate.proxy.LazyInitializer;
import org.hibernate.query.criteria.JpaCriteriaQuery;
import org.hibernate.query.criteria.JpaRoot;
import org.hibernate.tuple.entity.EntityMetamodel;

import jakarta.persistence.EntityGraph;
import jakarta.transaction.SystemException;

import static java.util.Arrays.asList;
import static org.hibernate.engine.internal.ManagedTypeHelper.asPersistentAttributeInterceptable;
import static org.hibernate.engine.internal.ManagedTypeHelper.isPersistentAttributeInterceptable;
import static org.hibernate.engine.internal.Versioning.incrementVersion;
Expand Down Expand Up @@ -276,6 +283,30 @@ public <T> T get(
}
}

@Override
public <T> List<T> getAll(Class<T> entityClass, Object... ids) {
for (Object id : ids) {
if ( id == null ) {
throw new IllegalArgumentException("Null id");
}
}
final EntityPersister persister = getEntityPersister( entityClass.getName() );
final JpaCriteriaQuery<T> query = getCriteriaBuilder().createQuery(entityClass);
final JpaRoot<T> from = query.from(entityClass);
query.where( from.get(persister.getIdentifierPropertyName()).in(ids) );
final List<T> resultList = createSelectionQuery(query).getResultList();
final List<Object> idList = new ArrayList<>(resultList.size());
for (T entity : resultList) {
idList.add( persister.getIdentifier(entity, this) );
}
final List<T> list = new ArrayList<>(ids.length);
for (Object id : ids) {
final int pos = idList.indexOf(id);
list.add( pos < 0 ? null : resultList.get(pos) );
}
return list;
}

private EntityPersister getEntityPersister(String entityName) {
return getFactory().getMappingMetamodel().getEntityDescriptor( entityName );
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package org.hibernate.orm.test.loading.multiLoad;

import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.Test;

import java.util.List;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

@SessionFactory
@DomainModel(annotatedClasses = FindAllTest.Record.class)
public class FindAllTest {
@Test void test(SessionFactoryScope scope) {
scope.inTransaction(s-> {
s.persist(new Record(123L,"hello earth"));
s.persist(new Record(456L,"hello mars"));
});
scope.inTransaction(s-> {
List<Record> all = s.findAll(Record.class, 456L, 123L, 2L);
assertEquals("hello mars",all.get(0).message);
assertEquals("hello earth",all.get(1).message);
assertNull(all.get(2));
});
}
@Entity
static class Record {
@Id Long id;
String message;

Record(Long id, String message) {
this.id = id;
this.message = message;
}

Record() {
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package org.hibernate.orm.test.stateless;

import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.Test;

import java.util.List;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

@SessionFactory
@DomainModel(annotatedClasses = GetAllTest.Record.class)
public class GetAllTest {
@Test void test(SessionFactoryScope scope) {
scope.inStatelessTransaction(s-> {
s.insert(new Record(123L,"hello earth"));
s.insert(new Record(456L,"hello mars"));
});
scope.inStatelessTransaction(s-> {
List<Record> all = s.getAll(Record.class, 456L, 123L, 2L);
assertEquals("hello mars",all.get(0).message);
assertEquals("hello earth",all.get(1).message);
assertNull(all.get(2));
});
}
@Entity
static class Record {
@Id Long id;
String message;

Record(Long id, String message) {
this.id = id;
this.message = message;
}

Record() {
}
}
}