Skip to content

Commit 0ed81d1

Browse files
gavinkinggavinking
committed
more work on uniform reporting of illegal arguments
1 parent 7d9c170 commit 0ed81d1

File tree

5 files changed

+125
-31
lines changed

5 files changed

+125
-31
lines changed

‎hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1707,27 +1707,26 @@ private void clearNullProperties() {
17071707
@Override
17081708
public boolean isReadOnly(Object entityOrProxy) {
17091709
if ( entityOrProxy == null ) {
1710-
throw new IllegalArgumentException( "Null entity instance" );
1710+
throw new IllegalArgumentException( "Entity may not be null" );
17111711
}
1712-
boolean isReadOnly;
1712+
17131713
final LazyInitializer lazyInitializer = extractLazyInitializer( entityOrProxy );
17141714
if ( lazyInitializer != null ) {
1715-
isReadOnly = lazyInitializer.isReadOnly();
1715+
return lazyInitializer.isReadOnly();
17161716
}
17171717
else {
1718-
final EntityEntry ee = getEntry( entityOrProxy );
1718+
final EntityEntry ee = getEntry( entityOrProxy );
17191719
if ( ee == null ) {
1720-
throw new IllegalArgumentException( "Instance is not associated with this persistence context" );
1720+
throw new IllegalArgumentException( "Given entity is not associated with the persistence context" );
17211721
}
1722-
isReadOnly = ee.isReadOnly();
1722+
return ee.isReadOnly();
17231723
}
1724-
return isReadOnly;
17251724
}
17261725

17271726
@Override
17281727
public void setReadOnly(Object object, boolean readOnly) {
17291728
if ( object == null ) {
1730-
throw new IllegalArgumentException( "Null entity instance" );
1729+
throw new IllegalArgumentException( "Entity may not be null" );
17311730
}
17321731
if ( isReadOnly( object ) == readOnly ) {
17331732
return;
@@ -1765,7 +1764,7 @@ private void setProxyReadOnly(LazyInitializer hibernateLazyInitializer, boolean
17651764
private void setEntityReadOnly(Object entity, boolean readOnly) {
17661765
final EntityEntry entry = getEntry( entity );
17671766
if ( entry == null ) {
1768-
throw new IllegalArgumentException( "Instance was not associated with this persistence context" );
1767+
throw new IllegalArgumentException( "Given entity is not associated with the persistence context" );
17691768
}
17701769
entry.setReadOnly( readOnly, entity );
17711770
hasNonReadOnlyEntities = hasNonReadOnlyEntities || ! readOnly;

‎hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -549,16 +549,13 @@ public LockMode getCurrentLockMode(Object object) {
549549
}
550550
}
551551

552-
final EntityEntry e = persistenceContext.getEntry( object );
553-
if ( e == null ) {
554-
throw new IllegalArgumentException( "Given entity is not associated with the persistence context" );
555-
}
556-
else if ( e.getStatus().isDeletedOrGone() ) {
557-
throw new ObjectDeletedException( "Given entity was removed", e.getId(),
558-
e.getPersister().getEntityName() );
552+
final EntityEntry entry = getEntityEntry( object );
553+
if ( entry.getStatus().isDeletedOrGone() ) {
554+
throw new ObjectDeletedException( "Given entity was removed", entry.getId(),
555+
entry.getPersister().getEntityName() );
559556
}
560557
else {
561-
return e.getLockMode();
558+
return entry.getLockMode();
562559
}
563560
}
564561

@@ -1555,17 +1552,11 @@ public Object getIdentifier(Object object) {
15551552
}
15561553
final LazyInitializer lazyInitializer = extractLazyInitializer( object );
15571554
if ( lazyInitializer != null ) {
1558-
if ( lazyInitializer.getSession() != this ) {
1559-
throw new IllegalArgumentException( "Given proxy is not associated with the persistence context" );
1560-
}
1555+
checkOwnsProxy( lazyInitializer );
15611556
return lazyInitializer.getInternalIdentifier();
15621557
}
15631558
else {
1564-
final EntityEntry entry = persistenceContext.getEntry( object );
1565-
if ( entry == null ) {
1566-
throw new IllegalArgumentException( "Given entity is not associated with the persistence context" );
1567-
}
1568-
return entry.getId();
1559+
return getEntityEntry( object ).getId();
15691560
}
15701561
}
15711562

@@ -1819,17 +1810,25 @@ public String getEntityName(Object object) {
18191810

18201811
final LazyInitializer lazyInitializer = extractLazyInitializer( object );
18211812
if ( lazyInitializer != null ) {
1822-
if ( !persistenceContext.containsProxy( object ) ) {
1823-
throw new IllegalArgumentException( "Given proxy is not associated with the persistence context" );
1824-
}
1813+
checkOwnsProxy( lazyInitializer );
18251814
object = lazyInitializer.getImplementation();
18261815
}
18271816

1817+
return getEntityEntry( object ).getPersister().getEntityName();
1818+
}
1819+
1820+
private void checkOwnsProxy(LazyInitializer lazyInitializer) {
1821+
if ( lazyInitializer.getSession() != this ) {
1822+
throw new IllegalArgumentException( "Given proxy is not associated with the persistence context" );
1823+
}
1824+
}
1825+
1826+
private EntityEntry getEntityEntry(Object object) {
18281827
final EntityEntry entry = persistenceContext.getEntry( object );
18291828
if ( entry == null ) {
18301829
throw new IllegalArgumentException( "Given entity is not associated with the persistence context" );
18311830
}
1832-
return entry.getPersister().getEntityName();
1831+
return entry;
18331832
}
18341833

18351834
@Override @SuppressWarnings("unchecked")
@@ -2788,7 +2787,7 @@ public LockModeType getLockMode(Object entity) {
27882787
if ( !contains( entity ) ) {
27892788
// convert() calls markForRollbackOnly()
27902789
throw getExceptionConverter()
2791-
.convert( new IllegalArgumentException( "Entity not associated with the persistence context" ) );
2790+
.convert( new IllegalArgumentException( "Given entity is not associated with the persistence context" ) );
27922791
}
27932792

27942793
return LockModeTypeHelper.getLockModeType( getCurrentLockMode( entity ) );

‎hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/DetachedEntityManagerArgumentsTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.nullargs;
6+
7+
import jakarta.persistence.Entity;
8+
import jakarta.persistence.Id;
9+
import jakarta.persistence.LockModeType;
10+
import org.hibernate.testing.orm.junit.EntityManagerFactoryScope;
11+
import org.hibernate.testing.orm.junit.Jpa;
12+
import org.junit.jupiter.api.Test;
13+
14+
import java.util.List;
15+
import java.util.function.Consumer;
16+
17+
import static org.junit.jupiter.api.Assertions.assertTrue;
18+
19+
@Jpa(annotatedClasses = DetachedEntityManagerArgumentsTest.Thing.class)
20+
class DetachedEntityManagerArgumentsTest {
21+
@Test void test(EntityManagerFactoryScope scope) {
22+
scope.inTransaction( em -> {
23+
var operations = List.<Consumer<Object>>of(
24+
em::refresh,
25+
e -> em.lock( e, LockModeType.NONE ),
26+
em::getLockMode
27+
);
28+
Thing thing = new Thing();
29+
thing.id = 5L;
30+
operations.forEach( c -> {
31+
try {
32+
c.accept( thing );
33+
}
34+
catch ( IllegalArgumentException e ) {
35+
assertTrue( e.getMessage().startsWith( "Given entity is not associated with the persistence context" ) );
36+
}
37+
} );
38+
} );
39+
}
40+
@Entity static class Thing {
41+
@Id
42+
Long id;
43+
}
44+
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/DetachedSessionArgumentsTest.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.nullargs;
6+
7+
import jakarta.persistence.Entity;
8+
import jakarta.persistence.Id;
9+
import jakarta.persistence.LockModeType;
10+
import org.hibernate.testing.orm.junit.DomainModel;
11+
import org.hibernate.testing.orm.junit.SessionFactory;
12+
import org.hibernate.testing.orm.junit.SessionFactoryScope;
13+
import org.junit.jupiter.api.Test;
14+
15+
import java.util.List;
16+
import java.util.function.Consumer;
17+
18+
import static org.junit.jupiter.api.Assertions.assertTrue;
19+
20+
@SessionFactory
21+
@DomainModel(annotatedClasses = DetachedSessionArgumentsTest.Thing.class)
22+
class DetachedSessionArgumentsTest {
23+
@Test void test(SessionFactoryScope scope) {
24+
scope.inTransaction( session -> {
25+
var operations = List.<Consumer<Object>>of(
26+
session::refresh,
27+
e -> session.lock( e, LockModeType.NONE ),
28+
session::getLockMode,
29+
session::isReadOnly,
30+
e -> session.setReadOnly( e, true ),
31+
session::getIdentifier
32+
33+
);
34+
Thing thing = new Thing();
35+
thing.id = 5L;
36+
operations.forEach( c -> {
37+
try {
38+
c.accept( thing );
39+
}
40+
catch ( IllegalArgumentException e ) {
41+
assertTrue( e.getMessage().startsWith( "Given entity is not associated with the persistence context" ) );
42+
}
43+
} );
44+
} );
45+
}
46+
@Entity static class Thing {
47+
@Id
48+
Long id;
49+
}
50+
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/NullSessionArgumentsTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ class NullSessionArgumentsTest {
3333
session::getLockMode,
3434
session::getEntityName,
3535
session::getIdentifier,
36-
session::contains
36+
session::contains,
37+
session::isReadOnly,
38+
e -> session.setReadOnly( e, true )
3739
);
3840
operations.forEach( c -> {
3941
try {

0 commit comments

Comments
 (0)