Skip to content

Commit cfe4346

Browse files
gavinkinggavinking
committed
Graphpocalypse: actually let's just deprecate the makeXxxxSubGraph() operations...
... and delete the new ones I added. The reasoning here is that they just aren't properly typesafe, as you can see by the hoops I had to go through to actually implement them. And since the whole package is marked @Incubating, we're not committed to them. This lets me make the implementation even more typesafe and detect more user errors and bugs.
1 parent ced58cc commit cfe4346

File tree

17 files changed

+392
-289
lines changed

17 files changed

+392
-289
lines changed

‎hibernate-core/src/main/java/org/hibernate/graph/AttributeNode.java

Lines changed: 14 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66

77

88
import jakarta.persistence.Subgraph;
9-
import jakarta.persistence.metamodel.ManagedType;
10-
import org.hibernate.Incubating;
119
import org.hibernate.metamodel.model.domain.PersistentAttribute;
1210

1311
import java.util.Map;
@@ -38,6 +36,8 @@
3836
* @apiNote Historically, this interface declared operations with incorrect generic types,
3937
* leading to unsound code. This was in Hibernate 7, with possible breakage to older code.
4038
*
39+
* @param <J> The type of the {@linkplain #getAttributeDescriptor attribute}
40+
*
4141
* @author Strong Liu
4242
* @author Steve Ebersole
4343
* @author Andrea Boriero
@@ -99,23 +99,27 @@ public interface AttributeNode<J> extends GraphNode<J>, jakarta.persistence.Attr
9999
/**
100100
* Create and return a new value {@link SubGraph} rooted at this node,
101101
* or return an existing such {@link SubGraph} if there is one.
102-
* <p>
102+
*
103+
* @deprecated This operation is not properly type safe.
103104
* Note that {@code graph.addAttributeNode(att).makeSubGraph()} is a
104105
* synonym for {@code graph.addSubgraph(att)}.
105106
*
106107
* @see Graph#addSubgraph(jakarta.persistence.metamodel.Attribute)
107108
*/
109+
@Deprecated(since = "7.0")
108110
SubGraph<?> makeSubGraph();
109111

110112
/**
111113
* Create and return a new key {@link SubGraph} rooted at this node,
112114
* or return an existing such {@link SubGraph} if there is one.
113-
* <p>
115+
*
116+
* @deprecated This operation is not properly type safe.
114117
* Note that {@code graph.addAttributeNode(att).makeKeySubGraph()} is a
115118
* synonym for {@code graph.addMapKeySubgraph(att)}.
116119
*
117120
* @see Graph#addMapKeySubgraph(jakarta.persistence.metamodel.MapAttribute)
118121
*/
122+
@Deprecated(since = "7.0")
119123
SubGraph<?> makeKeySubGraph();
120124

121125
/**
@@ -125,14 +129,16 @@ public interface AttributeNode<J> extends GraphNode<J>, jakarta.persistence.Attr
125129
* <p>
126130
* If the given type is a proper subtype of the value type, the result
127131
* is a treated subgraph.
128-
* <p>
132+
*
133+
* @deprecated This operation is not properly type safe.
129134
* Note that {@code graph.addAttributeNode(att).makeSubGraph(cl)}
130135
* is a synonym for {@code graph.addTreatedSubgraph(att,cl)}.
131136
*
132137
* @param subtype The type or treated type of the value type
133138
*
134139
* @see Graph#addTreatedSubgraph(jakarta.persistence.metamodel.Attribute, Class)
135140
*/
141+
@Deprecated(since = "7.0")
136142
<S> SubGraph<S> makeSubGraph(Class<S> subtype);
137143

138144
/**
@@ -142,43 +148,15 @@ public interface AttributeNode<J> extends GraphNode<J>, jakarta.persistence.Attr
142148
* <p>
143149
* If the given type is a proper subtype of the key type, the result
144150
* is a treated subgraph.
145-
* <p>
151+
*
152+
* @deprecated This operation is not properly type safe.
146153
* Note that {@code graph.addAttributeNode(att).makeKeySubGraph(cl)}
147154
* is a synonym for {@code graph.addTreatedMapKeySubgraph(att,cl)}.
148155
*
149156
* @param subtype The type or treated type of the key type
150157
*
151158
* @see Graph#addTreatedMapKeySubgraph(jakarta.persistence.metamodel.MapAttribute,Class)
152159
*/
160+
@Deprecated(since = "7.0")
153161
<S> SubGraph<S> makeKeySubGraph(Class<S> subtype);
154-
155-
/**
156-
* Create and return a new value {@link SubGraph} rooted at this node,
157-
* with the given type, which may be a subtype of the value type,
158-
* or return an existing such {@link SubGraph} if there is one.
159-
* <p>
160-
* If the given type is a proper subtype of the value type, the result
161-
* is a treated subgraph.
162-
*
163-
* @param subtype The type or treated type of the value type
164-
*
165-
* @since 7.0
166-
*/
167-
@Incubating
168-
<S> SubGraph<S> makeSubGraph(ManagedType<S> subtype);
169-
170-
/**
171-
* Create and return a new value {@link SubGraph} rooted at this node,
172-
* with the given type, which may be a subtype of the key type,
173-
* or return an existing such {@link SubGraph} if there is one.
174-
* <p>
175-
* If the given type is a proper subtype of the key type, the result
176-
* is a treated subgraph.
177-
*
178-
* @param subtype The type or treated type of the key type
179-
*
180-
* @since 7.0
181-
*/
182-
@Incubating
183-
<S> SubGraph<S> makeKeySubGraph(ManagedType<S> subtype);
184162
}

‎hibernate-core/src/main/java/org/hibernate/graph/Graph.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,7 @@ RootGraph<J> makeRootGraph(String name, boolean mutable)
176176
* @param attributeName The name of an attribute of the represented type
177177
*/
178178
@Override
179-
default <X> SubGraph<X> addSubgraph(String attributeName) {
180-
return addSubGraph( attributeName );
181-
}
179+
<X> SubGraph<X> addSubgraph(String attributeName);
182180

183181
/**
184182
* Create and return a new (mutable) {@link SubGraph} associated with

‎hibernate-core/src/main/java/org/hibernate/graph/internal/AttributeNodeImpl.java

Lines changed: 81 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,25 @@
44
*/
55
package org.hibernate.graph.internal;
66

7-
import jakarta.persistence.metamodel.ManagedType;
7+
import jakarta.persistence.metamodel.Attribute;
88
import org.hibernate.graph.CannotContainSubGraphException;
99
import org.hibernate.graph.spi.AttributeNodeImplementor;
1010
import org.hibernate.graph.spi.SubGraphImplementor;
1111
import org.hibernate.metamodel.model.domain.DomainType;
1212
import org.hibernate.metamodel.model.domain.ManagedDomainType;
13+
import org.hibernate.metamodel.model.domain.MapPersistentAttribute;
1314
import org.hibernate.metamodel.model.domain.PersistentAttribute;
15+
import org.hibernate.metamodel.model.domain.PluralPersistentAttribute;
1416
import org.hibernate.metamodel.model.domain.SimpleDomainType;
1517

1618
import java.util.HashMap;
1719
import java.util.Map;
1820

21+
import static jakarta.persistence.metamodel.Attribute.PersistentAttributeType.EMBEDDED;
22+
import static jakarta.persistence.metamodel.Attribute.PersistentAttributeType.MANY_TO_MANY;
23+
import static jakarta.persistence.metamodel.Attribute.PersistentAttributeType.MANY_TO_ONE;
24+
import static jakarta.persistence.metamodel.Attribute.PersistentAttributeType.ONE_TO_MANY;
25+
import static jakarta.persistence.metamodel.Attribute.PersistentAttributeType.ONE_TO_ONE;
1926
import static java.util.Collections.emptyMap;
2027

2128

@@ -25,30 +32,38 @@
2532
* @author Steve Ebersole
2633
* @author Gavin King
2734
*/
28-
public class AttributeNodeImpl<J,V,K>
35+
public class AttributeNodeImpl<J, E, K>
2936
extends AbstractGraphNode<J>
30-
implements AttributeNodeImplementor<J> {
37+
implements AttributeNodeImplementor<J, E, K> {
3138
private final PersistentAttribute<?, J> attribute;
32-
private final DomainType<V> valueGraphType;
39+
private final DomainType<E> valueGraphType;
3340
private final SimpleDomainType<K> keyGraphType;
3441

35-
private SubGraphImplementor<V> valueSubgraph;
42+
private SubGraphImplementor<E> valueSubgraph;
3643
private SubGraphImplementor<K> keySubgraph;
3744

3845
static <X,J> AttributeNodeImpl<J,?,?> create(PersistentAttribute<X, J> attribute, boolean mutable) {
3946
return new AttributeNodeImpl<>( attribute, mutable, attribute.getValueGraphType(), attribute.getKeyGraphType() );
4047
}
4148

49+
static <X,J,E> AttributeNodeImpl<J,E,?> create(PluralPersistentAttribute<X, J, E> attribute, boolean mutable) {
50+
return new AttributeNodeImpl<>( attribute, mutable, attribute.getValueGraphType(), attribute.getKeyGraphType() );
51+
}
52+
53+
static <X,K,V> AttributeNodeImpl<Map<K,V>,V,K> create(MapPersistentAttribute<X, K, V> attribute, boolean mutable) {
54+
return new AttributeNodeImpl<>( attribute, mutable, attribute.getValueGraphType(), attribute.getKeyGraphType() );
55+
}
56+
4257
private <X> AttributeNodeImpl(
4358
PersistentAttribute<X, J> attribute, boolean mutable,
44-
DomainType<V> valueGraphType, SimpleDomainType<K> keyGraphType) {
59+
DomainType<E> valueGraphType, SimpleDomainType<K> keyGraphType) {
4560
super( mutable );
4661
this.attribute = attribute;
4762
this.valueGraphType = valueGraphType;
4863
this.keyGraphType = keyGraphType;
4964
}
5065

51-
private AttributeNodeImpl(AttributeNodeImpl<J,V,K> that, boolean mutable) {
66+
private AttributeNodeImpl(AttributeNodeImpl<J, E,K> that, boolean mutable) {
5267
super( mutable );
5368
attribute = that.attribute;
5469
valueGraphType = that.valueGraphType;
@@ -68,52 +83,81 @@ public PersistentAttribute<?, J> getAttributeDescriptor() {
6883
}
6984

7085
@Override
71-
public SubGraphImplementor<V> getSubGraph() {
86+
public SubGraphImplementor<E> addValueSubgraph() {
87+
// this one is intentionally lenient and disfavored
88+
if ( valueSubgraph == null ) {
89+
valueSubgraph = new SubGraphImpl<>( asManagedType( valueGraphType ), true );
90+
}
7291
return valueSubgraph;
7392
}
7493

7594
@Override
76-
public SubGraphImplementor<K> getKeySubGraph() {
77-
return keySubgraph;
95+
public SubGraphImplementor<J> addSingularSubgraph() {
96+
checkToOne();
97+
if ( valueSubgraph == null ) {
98+
valueSubgraph = new SubGraphImpl<>( asManagedType( valueGraphType ), true );
99+
}
100+
// Safe cast, in this case E = J
101+
// TODO: would be more elegant to separate singularSubgraph vs elementSubgraph fields
102+
//noinspection unchecked
103+
return (SubGraphImplementor<J>) valueSubgraph;
78104
}
79105

80106
@Override
81-
public SubGraphImplementor<V> makeSubGraph() {
82-
verifyMutability();
107+
public SubGraphImplementor<E> addElementSubgraph() {
108+
checkToMany();
83109
if ( valueSubgraph == null ) {
84110
valueSubgraph = new SubGraphImpl<>( asManagedType( valueGraphType ), true );
85111
}
86112
return valueSubgraph;
87113
}
88114

89115
@Override
90-
public <S> SubGraphImplementor<S> makeSubGraph(Class<S> subtype) {
91-
final ManagedDomainType<V> managedType = asManagedType( valueGraphType );
92-
if ( !managedType.getBindableJavaType().isAssignableFrom( subtype ) ) {
93-
throw new IllegalArgumentException( "Not a subtype: " + subtype.getName() );
116+
public SubGraphImplementor<K> addKeySubgraph() {
117+
checkMap();
118+
if ( keySubgraph == null ) {
119+
keySubgraph = new SubGraphImpl<>( asManagedType( keyGraphType ), true );
94120
}
95-
@SuppressWarnings("unchecked")
96-
final Class<? extends V> castSuptype = (Class<? extends V>) subtype;
97-
final SubGraphImplementor<? extends V> result = makeSubGraph().addTreatedSubgraph( castSuptype );
98-
//noinspection unchecked
99-
return (SubGraphImplementor<S>) result;
121+
return keySubgraph;
100122
}
101123

102-
@Override
103-
public <S> SubGraphImplementor<S> makeSubGraph(ManagedType<S> subtype) {
104-
final ManagedDomainType<V> managedType = asManagedType( valueGraphType );
105-
final Class<S> javaType = subtype.getJavaType();
106-
if ( !managedType.getBindableJavaType().isAssignableFrom( javaType ) ) {
107-
throw new IllegalArgumentException( "Not a subtype: " + javaType.getName() );
124+
private void checkToOne() {
125+
final Attribute.PersistentAttributeType attributeType = attribute.getPersistentAttributeType();
126+
if ( attributeType != MANY_TO_ONE && attributeType != ONE_TO_ONE && attributeType != EMBEDDED ) {
127+
throw new CannotContainSubGraphException( "Attribute '" + attribute.getName() + "' is not a to-one association" );
128+
}
129+
}
130+
131+
private void checkToMany() {
132+
final Attribute.PersistentAttributeType attributeType = attribute.getPersistentAttributeType();
133+
if ( attributeType != MANY_TO_MANY && attributeType != ONE_TO_MANY ) {
134+
throw new CannotContainSubGraphException( "Attribute '" + attribute.getName() + "' is not a to-many association" );
135+
}
136+
}
137+
138+
@Override @Deprecated
139+
public SubGraphImplementor<E> makeSubGraph() {
140+
verifyMutability();
141+
if ( valueSubgraph == null ) {
142+
valueSubgraph = new SubGraphImpl<>( asManagedType( valueGraphType ), true );
143+
}
144+
return valueSubgraph;
145+
}
146+
147+
@Override @Deprecated
148+
public <S> SubGraphImplementor<S> makeSubGraph(Class<S> subtype) {
149+
final ManagedDomainType<E> managedType = asManagedType( valueGraphType );
150+
if ( !managedType.getBindableJavaType().isAssignableFrom( subtype ) ) {
151+
throw new IllegalArgumentException( "Not a subtype: " + subtype.getName() );
108152
}
109153
@SuppressWarnings("unchecked")
110-
final ManagedDomainType<? extends V> castType = (ManagedDomainType<? extends V>) subtype;
111-
final SubGraphImplementor<? extends V> result = makeSubGraph().addTreatedSubgraph( castType );
154+
final Class<? extends E> castSuptype = (Class<? extends E>) subtype;
155+
final SubGraphImplementor<? extends E> result = makeSubGraph().addTreatedSubgraph( castSuptype );
112156
//noinspection unchecked
113157
return (SubGraphImplementor<S>) result;
114158
}
115159

116-
@Override
160+
@Override @Deprecated
117161
public SubGraphImplementor<K> makeKeySubGraph() {
118162
verifyMutability();
119163
checkMap();
@@ -123,7 +167,7 @@ public SubGraphImplementor<K> makeKeySubGraph() {
123167
return keySubgraph;
124168
}
125169

126-
@Override
170+
@Override @Deprecated
127171
public <S> SubGraphImplementor<S> makeKeySubGraph(Class<S> subtype) {
128172
checkMap();
129173
final ManagedDomainType<K> type = asManagedType( keyGraphType );
@@ -137,21 +181,6 @@ public <S> SubGraphImplementor<S> makeKeySubGraph(Class<S> subtype) {
137181
return (SubGraphImplementor<S>) result;
138182
}
139183

140-
@Override
141-
public <S> SubGraphImplementor<S> makeKeySubGraph(ManagedType<S> subtype) {
142-
checkMap();
143-
final ManagedDomainType<K> type = asManagedType( keyGraphType );
144-
final Class<S> javaType = subtype.getJavaType();
145-
if ( !type.getBindableJavaType().isAssignableFrom( javaType ) ) {
146-
throw new IllegalArgumentException( "Not a key subtype: " + javaType.getName() );
147-
}
148-
@SuppressWarnings("unchecked")
149-
final ManagedDomainType<? extends K> castType = (ManagedDomainType<? extends K>) subtype;
150-
final SubGraphImplementor<? extends K> result = makeKeySubGraph().addTreatedSubgraph( castType );
151-
//noinspection unchecked
152-
return (SubGraphImplementor<S>) result;
153-
}
154-
155184
private void checkMap() {
156185
if ( keyGraphType == null ) {
157186
throw new CannotContainSubGraphException( "Attribute '" + description() + "' is not a Map" );
@@ -179,16 +208,16 @@ public String toString() {
179208
}
180209

181210
@Override
182-
public AttributeNodeImplementor<J> makeCopy(boolean mutable) {
211+
public AttributeNodeImplementor<J, E, K> makeCopy(boolean mutable) {
183212
return !mutable && !isMutable() ? this : new AttributeNodeImpl<>( this, mutable );
184213
}
185214

186215
@Override
187-
public void merge(AttributeNodeImplementor<J> other) {
216+
public void merge(AttributeNodeImplementor<J, E, K> other) {
188217
assert other.isMutable() == isMutable();
189218
assert other.getAttributeDescriptor() == attribute;
190-
final AttributeNodeImpl<J, V, K> that = (AttributeNodeImpl<J, V, K>) other;
191-
final SubGraphImplementor<V> otherValueSubgraph = that.valueSubgraph;
219+
final AttributeNodeImpl<J, E, K> that = (AttributeNodeImpl<J, E, K>) other;
220+
final SubGraphImplementor<E> otherValueSubgraph = that.valueSubgraph;
192221
if ( otherValueSubgraph != null ) {
193222
if ( valueSubgraph == null ) {
194223
valueSubgraph = otherValueSubgraph.makeCopy( isMutable() );
@@ -216,7 +245,7 @@ public Map<Class<?>, SubGraphImplementor<?>> getSubGraphs() {
216245
return emptyMap();
217246
}
218247
else {
219-
final HashMap<Class<?>, SubGraphImplementor<?>> map = new HashMap<>( valueSubgraph.getSubGraphs() );
248+
final HashMap<Class<?>, SubGraphImplementor<?>> map = new HashMap<>( valueSubgraph.getTreatedSubgraphs() );
220249
map.put( attribute.getValueGraphType().getBindableJavaType(), valueSubgraph );
221250
return map;
222251
}
@@ -228,7 +257,7 @@ public Map<Class<?>, SubGraphImplementor<?>> getKeySubGraphs() {
228257
return emptyMap();
229258
}
230259
else {
231-
final HashMap<Class<?>, SubGraphImplementor<?>> map = new HashMap<>( keySubgraph.getSubGraphs() );
260+
final HashMap<Class<?>, SubGraphImplementor<?>> map = new HashMap<>( keySubgraph.getTreatedSubgraphs() );
232261
map.put( attribute.getKeyGraphType().getJavaType(), keySubgraph );
233262
return map;
234263
}

0 commit comments

Comments
 (0)