Skip to content

Commit 2cf69b3

Browse files
gavinkinggavinking
committed
make the format of entity ids in log and error messages more transparent
since I just met a user who did not understand what [Entity#10] meant
1 parent a2e683f commit 2cf69b3

File tree

3 files changed

+102
-108
lines changed

3 files changed

+102
-108
lines changed

‎hibernate-core/src/main/java/org/hibernate/pretty/MessageHelper.java‎

Lines changed: 99 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -40,25 +40,23 @@ private MessageHelper() {
4040
* @return An info string, in the form [FooBar#1].
4141
*/
4242
public static String infoString(@Nullable String entityName, @Nullable Object id) {
43-
StringBuilder s = new StringBuilder();
44-
s.append( '[' );
43+
final StringBuilder info = new StringBuilder();
44+
info.append( '[' );
4545
if ( entityName == null ) {
46-
s.append( "<null entity name>" );
46+
info.append( "unknown entity name" );
4747
}
4848
else {
49-
s.append( entityName );
49+
info.append( entityName );
5050
}
51-
s.append( '#' );
5251

5352
if ( id == null ) {
54-
s.append( "<null>" );
53+
info.append( " with null id" );
5554
}
5655
else {
57-
s.append( id );
56+
info.append( " with id '" ).append( id ).append( "'" );
5857
}
59-
s.append( ']' );
60-
61-
return s.toString();
58+
info.append( ']' );
59+
return info.toString();
6260
}
6361

6462
/**
@@ -73,38 +71,39 @@ public static String infoString(
7371
@Nullable EntityPersister persister,
7472
@Nullable Object id,
7573
@Nullable SessionFactoryImplementor factory) {
76-
StringBuilder s = new StringBuilder();
77-
s.append( '[' );
74+
final StringBuilder info = new StringBuilder();
75+
info.append( '[' );
7876
Type idType;
7977
if( persister == null ) {
80-
s.append( "<null EntityPersister>" );
78+
info.append( "unknown entity" );
8179
idType = null;
8280
}
8381
else {
84-
s.append( persister.getEntityName() );
82+
info.append( persister.getEntityName() );
8583
idType = persister.getIdentifierType();
8684
}
87-
s.append( '#' );
8885

8986
if ( id == null ) {
90-
s.append( "<null>" );
87+
info.append( " with null id" );
9188
}
9289
else {
90+
info.append( " with id '" ).append( id ).append( "'" );
9391
if ( idType == null ) {
94-
s.append( id );
92+
info.append( id );
9593
}
9694
else {
9795
if ( factory != null ) {
98-
s.append( idType.toLoggableString( id, factory ) );
96+
info.append( idType.toLoggableString( id, factory ) );
9997
}
10098
else {
101-
s.append( "<not loggable>" );
99+
info.append( "<not loggable>" );
102100
}
103101
}
102+
info.append( "'" );
104103
}
105-
s.append( ']' );
104+
info.append( ']' );
106105

107-
return s.toString();
106+
return info.toString();
108107

109108
}
110109

@@ -122,25 +121,24 @@ public static String infoString(
122121
@Nullable Object id,
123122
Type identifierType,
124123
SessionFactoryImplementor factory) {
125-
StringBuilder s = new StringBuilder();
126-
s.append( '[' );
124+
final StringBuilder info = new StringBuilder();
125+
info.append( '[' );
127126
if( persister == null ) {
128-
s.append( "<null EntityPersister>" );
127+
info.append( "unknown entity" );
129128
}
130129
else {
131-
s.append( persister.getEntityName() );
130+
info.append( persister.getEntityName() );
132131
}
133-
s.append( '#' );
134132

135133
if ( id == null ) {
136-
s.append( "<null>" );
134+
info.append( " with null id" );
137135
}
138136
else {
139-
s.append( identifierType.toLoggableString( id, factory ) );
137+
info.append( " with id '" ).append( identifierType.toLoggableString( id, factory ) ).append( "'" );
140138
}
141-
s.append( ']' );
139+
info.append( ']' );
142140

143-
return s.toString();
141+
return info.toString();
144142
}
145143

146144
/**
@@ -155,25 +153,25 @@ public static String infoString(
155153
@Nullable EntityPersister persister,
156154
Object[] ids,
157155
SessionFactoryImplementor factory) {
158-
StringBuilder s = new StringBuilder();
159-
s.append( '[' );
160-
if( persister == null ) {
161-
s.append( "<null EntityPersister>" );
156+
final StringBuilder info = new StringBuilder();
157+
info.append( '[' );
158+
if ( persister == null ) {
159+
info.append( "unknown entity" );
162160
}
163161
else {
164-
s.append( persister.getEntityName() );
165-
s.append( "#<" );
162+
info.append( persister.getEntityName() );
163+
info.append( " with ids " );
166164
for ( int i=0; i<ids.length; i++ ) {
167-
s.append( persister.getIdentifierType().toLoggableString( ids[i], factory ) );
165+
info.append( "'" )
166+
.append( persister.getIdentifierType().toLoggableString( ids[i], factory ) )
167+
.append( "'" );
168168
if ( i < ids.length-1 ) {
169-
s.append( ", " );
169+
info.append( ", " );
170170
}
171171
}
172-
s.append( '>' );
173172
}
174-
s.append( ']' );
175-
176-
return s.toString();
173+
info.append( ']' );
174+
return info.toString();
177175

178176
}
179177

@@ -184,16 +182,16 @@ public static String infoString(
184182
* @return An info string, in the form [FooBar]
185183
*/
186184
public static String infoString(@Nullable EntityPersister persister) {
187-
StringBuilder s = new StringBuilder();
188-
s.append( '[' );
185+
final StringBuilder info = new StringBuilder();
186+
info.append( '[' );
189187
if ( persister == null ) {
190-
s.append( "<null EntityPersister>" );
188+
info.append( "unknown entity" );
191189
}
192190
else {
193-
s.append( persister.getEntityName() );
191+
info.append( persister.getEntityName() );
194192
}
195-
s.append( ']' );
196-
return s.toString();
193+
info.append( ']' );
194+
return info.toString();
197195
}
198196

199197
/**
@@ -206,21 +204,20 @@ public static String infoString(@Nullable EntityPersister persister) {
206204
* @return An info string, in the form [Foo.bars#1]
207205
*/
208206
public static String infoString(String entityName, String propertyName, @Nullable Object key) {
209-
StringBuilder s = new StringBuilder()
207+
final StringBuilder info = new StringBuilder()
210208
.append( '[' )
211209
.append( entityName )
212210
.append( '.' )
213-
.append( propertyName )
214-
.append( '#' );
211+
.append( propertyName );
215212

216213
if ( key == null ) {
217-
s.append( "<null>" );
214+
info.append( " with null owner id" );
218215
}
219216
else {
220-
s.append( key );
217+
info.append( " with owner id '" ).append( key ).append( "'" );
221218
}
222-
s.append( ']' );
223-
return s.toString();
219+
info.append( ']' );
220+
return info.toString();
224221
}
225222

226223

@@ -242,36 +239,35 @@ public static String collectionInfoString(
242239
@Nullable PersistentCollection<?> collection,
243240
Object collectionKey,
244241
SharedSessionContractImplementor session ) {
245-
246-
StringBuilder s = new StringBuilder();
247-
s.append( '[' );
242+
final StringBuilder info = new StringBuilder();
243+
info.append( '[' );
248244
if ( persister == null ) {
249-
s.append( "<unreferenced>" );
245+
info.append( "unreferenced collection" );
250246
}
251247
else {
252-
s.append( persister.getRole() );
253-
s.append( '#' );
254-
255-
Type ownerIdentifierType = persister.getOwnerEntityPersister()
256-
.getIdentifierType();
257-
Object ownerKey;
248+
info.append( persister.getRole() );
249+
final Type ownerIdentifierType =
250+
persister.getOwnerEntityPersister().getIdentifierType();
251+
final Object ownerKey;
258252
// TODO: Is it redundant to attempt to use the collectionKey,
259253
// or is always using the owner id sufficient?
260254
if ( collectionKey.getClass().isAssignableFrom(
261255
ownerIdentifierType.getReturnedClass() ) ) {
262256
ownerKey = collectionKey;
263257
}
264258
else {
265-
Object collectionOwner = collection == null ? null : collection.getOwner();
266-
EntityEntry entry = collectionOwner == null ? null : session.getPersistenceContextInternal().getEntry(collectionOwner);
259+
final Object collectionOwner = collection == null ? null
260+
: collection.getOwner();
261+
final EntityEntry entry = collectionOwner == null ? null
262+
: session.getPersistenceContextInternal().getEntry( collectionOwner );
267263
ownerKey = entry == null ? null : entry.getId();
268264
}
269-
s.append( ownerIdentifierType.toLoggableString(
270-
ownerKey, session.getFactory() ) );
265+
info.append( " with owner id '" )
266+
.append( ownerIdentifierType.toLoggableString( ownerKey, session.getFactory() ) )
267+
.append( "'" );
271268
}
272-
s.append( ']' );
273-
274-
return s.toString();
269+
info.append( ']' );
270+
return info.toString();
275271
}
276272

277273
/**
@@ -287,24 +283,25 @@ public static String collectionInfoString(
287283
@Nullable CollectionPersister persister,
288284
Object[] ids,
289285
SessionFactoryImplementor factory) {
290-
StringBuilder s = new StringBuilder();
291-
s.append( '[' );
286+
final StringBuilder info = new StringBuilder();
287+
info.append( '[' );
292288
if ( persister == null ) {
293-
s.append( "<unreferenced>" );
289+
info.append( "unreferenced collection" );
294290
}
295291
else {
296-
s.append( persister.getRole() );
297-
s.append( "#<" );
292+
info.append( persister.getRole() );
293+
info.append( " with owner ids " );
298294
for ( int i = 0; i < ids.length; i++ ) {
299-
addIdToCollectionInfoString( persister, ids[i], factory, s );
295+
info.append( "'" );
296+
addIdToCollectionInfoString( persister, ids[i], factory, info );
297+
info.append( "'" );
300298
if ( i < ids.length-1 ) {
301-
s.append( ", " );
299+
info.append( ", " );
302300
}
303301
}
304-
s.append( '>' );
305302
}
306-
s.append( ']' );
307-
return s.toString();
303+
info.append( ']' );
304+
return info.toString();
308305
}
309306

310307
/**
@@ -320,25 +317,24 @@ public static String collectionInfoString(
320317
@Nullable CollectionPersister persister,
321318
@Nullable Object id,
322319
SessionFactoryImplementor factory) {
323-
StringBuilder s = new StringBuilder();
324-
s.append( '[' );
320+
final StringBuilder info = new StringBuilder();
321+
info.append( '[' );
325322
if ( persister == null ) {
326-
s.append( "<unreferenced>" );
323+
info.append( "unreferenced collection" );
327324
}
328325
else {
329-
s.append( persister.getRole() );
330-
s.append( '#' );
331-
326+
info.append( persister.getRole() );
332327
if ( id == null ) {
333-
s.append( "<null>" );
328+
info.append( " with null owner id" );
334329
}
335330
else {
336-
addIdToCollectionInfoString( persister, id, factory, s );
331+
info.append( " with owner id '" );
332+
addIdToCollectionInfoString( persister, id, factory, info );
333+
info.append( "'" );
337334
}
338335
}
339-
s.append( ']' );
340-
341-
return s.toString();
336+
info.append( ']' );
337+
return info.toString();
342338
}
343339

344340
private static void addIdToCollectionInfoString(
@@ -353,8 +349,8 @@ private static void addIdToCollectionInfoString(
353349
// Also need to check that the expected identifier type matches
354350
// the given ID. Due to property-ref keys, the collection key
355351
// may not be the owner key.
356-
Type ownerIdentifierType = persister.getOwnerEntityPersister()
357-
.getIdentifierType();
352+
final Type ownerIdentifierType =
353+
persister.getOwnerEntityPersister().getIdentifierType();
358354
if ( id.getClass().isAssignableFrom(
359355
ownerIdentifierType.getReturnedClass() ) ) {
360356
s.append( ownerIdentifierType.toLoggableString( id, factory ) );
@@ -375,24 +371,22 @@ private static void addIdToCollectionInfoString(
375371
* @return An info string, in the form [Foo.bars#1]
376372
*/
377373
public static String collectionInfoString(@Nullable String role, @Nullable Object id) {
378-
StringBuilder s = new StringBuilder();
379-
s.append( '[' );
374+
final StringBuilder info = new StringBuilder();
375+
info.append( '[' );
380376
if( role == null ) {
381-
s.append( "<unreferenced>" );
377+
info.append( "unreferenced collection" );
382378
}
383379
else {
384-
s.append( role );
385-
s.append( '#' );
386-
380+
info.append( role );
387381
if ( id == null ) {
388-
s.append( "<null>" );
382+
info.append( " with null owner id" );
389383
}
390384
else {
391-
s.append( id );
385+
info.append( " with owner id '" ).append( id ).append( "'" );
392386
}
393387
}
394-
s.append( ']' );
395-
return s.toString();
388+
info.append( ']' );
389+
return info.toString();
396390
}
397391

398392
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/collection/delayedOperation/DetachedBagDelayedOperationTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ public void testMergeDetachedCollectionWithQueuedOperations(
151151

152152
assertTrue( opDetachedWatcher.wasTriggered() );
153153
assertEquals(
154-
"HHH000496: Detaching an uninitialized collection with queued operations from a session: [org.hibernate.orm.test.collection.delayedOperation.DetachedBagDelayedOperationTest$Parent.children#1]",
154+
"HHH000496: Detaching an uninitialized collection with queued operations from a session: [org.hibernate.orm.test.collection.delayedOperation.DetachedBagDelayedOperationTest$Parent.children with owner id '1']",
155155
opDetachedWatcher.getFirstTriggeredMessage()
156156
);
157157
opDetachedWatcher.reset();
@@ -188,7 +188,7 @@ public void testMergeDetachedCollectionWithQueuedOperations(
188188
Parent p = session.merge( pWithQueuedOperations );
189189
assertTrue( opMergedWatcher.wasTriggered() );
190190
assertEquals(
191-
"HHH000494: Attempt to merge an uninitialized collection with queued operations; queued operations will be ignored: [org.hibernate.orm.test.collection.delayedOperation.DetachedBagDelayedOperationTest$Parent.children#1]",
191+
"HHH000494: Attempt to merge an uninitialized collection with queued operations; queued operations will be ignored: [org.hibernate.orm.test.collection.delayedOperation.DetachedBagDelayedOperationTest$Parent.children with owner id '1']",
192192
opMergedWatcher.getFirstTriggeredMessage()
193193
);
194194
opMergedWatcher.reset();

0 commit comments

Comments
 (0)