HHH-8159 - Apply fixups indicated by analysis tools

hibernate-core/src/main/java/org/hibernate/engine/internal/Collections.java

@@ -47,29 +47,29 @@
  * @author Gavin King
  */
 public final class Collections {
-    private static final CoreMessageLogger LOG = Logger.getMessageLogger(CoreMessageLogger.class, Collections.class.getName());
+	private static final CoreMessageLogger LOG = Logger.getMessageLogger(
-
+			CoreMessageLogger.class,
-	private Collections() {
+			Collections.class.getName()
-	}
+	);
 
 	/**
 	 * record the fact that this collection was dereferenced
 	 *
 	 * @param coll The collection to be updated by un-reachability.
+	 * @param session The session
 	 */
-	@SuppressWarnings( {"JavaDoc"})
 	public static void processUnreachableCollection(PersistentCollection coll, SessionImplementor session) {
 		if ( coll.getOwner()==null ) {
-			processNeverReferencedCollection(coll, session);
+			processNeverReferencedCollection( coll, session );
 		}
 		else {
-			processDereferencedCollection(coll, session);
+			processDereferencedCollection( coll, session );
 		}
 	}
 
 	private static void processDereferencedCollection(PersistentCollection coll, SessionImplementor session) {
 		final PersistenceContext persistenceContext = session.getPersistenceContext();
-		CollectionEntry entry = persistenceContext.getCollectionEntry(coll);
+		final CollectionEntry entry = persistenceContext.getCollectionEntry( coll );
 		final CollectionPersister loadedPersister = entry.getLoadedPersister();
 
 		if ( loadedPersister != null && LOG.isDebugEnabled() ) {
@@ -82,15 +82,15 @@ private static void processDereferencedCollection(PersistentCollection coll, Ses
 		}
 
 		// do a check
-		boolean hasOrphanDelete = loadedPersister != null && loadedPersister.hasOrphanDelete();
+		final boolean hasOrphanDelete = loadedPersister != null && loadedPersister.hasOrphanDelete();
-		if (hasOrphanDelete) {
+		if ( hasOrphanDelete ) {
 			Serializable ownerId = loadedPersister.getOwnerEntityPersister().getIdentifier( coll.getOwner(), session );
 			if ( ownerId == null ) {
 				// the owning entity may have been deleted and its identifier unset due to
 				// identifier-rollback; in which case, try to look up its identifier from
 				// the persistence context
 				if ( session.getFactory().getSettings().isIdentifierRollbackEnabled() ) {
-					EntityEntry ownerEntry = persistenceContext.getEntry( coll.getOwner() );
+					final EntityEntry ownerEntry = persistenceContext.getEntry( coll.getOwner() );
 					if ( ownerEntry != null ) {
 						ownerId = ownerEntry.getId();
 					}
@@ -99,15 +99,15 @@ private static void processDereferencedCollection(PersistentCollection coll, Ses
 					throw new AssertionFailure( "Unable to determine collection owner identifier for orphan-delete processing" );
 				}
 			}
-			EntityKey key = session.generateEntityKey( ownerId, loadedPersister.getOwnerEntityPersister() );
+			final EntityKey key = session.generateEntityKey( ownerId, loadedPersister.getOwnerEntityPersister() );
-			Object owner = persistenceContext.getEntity(key);
+			final Object owner = persistenceContext.getEntity( key );
 			if ( owner == null ) {
 				throw new AssertionFailure(
 						"collection owner not associated with session: " +
 						loadedPersister.getRole()
 				);
 			}
-			EntityEntry e = persistenceContext.getEntry(owner);
+			final EntityEntry e = persistenceContext.getEntry( owner );
 			//only collections belonging to deleted entities are allowed to be dereferenced in the case of orphan delete
 			if ( e != null && e.getStatus() != Status.DELETED && e.getStatus() != Status.GONE ) {
 				throw new HibernateException(
@@ -118,23 +118,27 @@ private static void processDereferencedCollection(PersistentCollection coll, Ses
 		}
 
 		// do the work
-		entry.setCurrentPersister(null);
+		entry.setCurrentPersister( null );
-		entry.setCurrentKey(null);
+		entry.setCurrentKey( null );
 		prepareCollectionForUpdate( coll, entry, session.getFactory() );
 
 	}
 
 	private static void processNeverReferencedCollection(PersistentCollection coll, SessionImplementor session)
-	throws HibernateException {
+			throws HibernateException {
-
 		final PersistenceContext persistenceContext = session.getPersistenceContext();
-		CollectionEntry entry = persistenceContext.getCollectionEntry(coll);
+		final CollectionEntry entry = persistenceContext.getCollectionEntry( coll );
 
 		if ( LOG.isDebugEnabled() ) {
-			LOG.debugf( "Found collection with unloaded owner: %s",
+			LOG.debugf(
+					"Found collection with unloaded owner: %s",
 					MessageHelper.collectionInfoString( 
-							entry.getLoadedPersister(), coll,
+							entry.getLoadedPersister(),
-							entry.getLoadedKey(), session ) );
+							coll,
+							entry.getLoadedKey(),
+							session
+					)
+			);
 		}
 
 		entry.setCurrentPersister( entry.getLoadedPersister() );
@@ -154,13 +158,11 @@ private static void processNeverReferencedCollection(PersistentCollection coll,
      */
 	public static void processReachableCollection(
 			PersistentCollection collection,
-	        CollectionType type,
+			CollectionType type,
-	        Object entity,
+			Object entity,
-	        SessionImplementor session) {
+			SessionImplementor session) {
-
+		collection.setOwner( entity );
-		collection.setOwner(entity);
+		final CollectionEntry ce = session.getPersistenceContext().getCollectionEntry( collection );
-
-		CollectionEntry ce = session.getPersistenceContext().getCollectionEntry(collection);
 
 		if ( ce == null ) {
 			// refer to comment in StatefulPersistenceContext.addCollection()
@@ -175,30 +177,35 @@ public static void processReachableCollection(
 		if ( ce.isReached() ) {
 			// We've been here before
 			throw new HibernateException(
-					"Found shared references to a collection: " +
+					"Found shared references to a collection: " + type.getRole()
-					type.getRole()
 			);
 		}
-		ce.setReached(true);
+		ce.setReached( true );
 
-		SessionFactoryImplementor factory = session.getFactory();
+		final SessionFactoryImplementor factory = session.getFactory();
-		CollectionPersister persister = factory.getCollectionPersister( type.getRole() );
+		final CollectionPersister persister = factory.getCollectionPersister( type.getRole() );
-		ce.setCurrentPersister(persister);
+		ce.setCurrentPersister( persister );
-		ce.setCurrentKey( type.getKeyOfOwner(entity, session) ); //TODO: better to pass the id in as an argument?
+		//TODO: better to pass the id in as an argument?
+		ce.setCurrentKey( type.getKeyOfOwner( entity, session ) );
 
-        if (LOG.isDebugEnabled()) {
+		if ( LOG.isDebugEnabled() ) {
-            if (collection.wasInitialized()) LOG.debugf("Collection found: %s, was: %s (initialized)",
+			if ( collection.wasInitialized() ) {
-                                                        MessageHelper.collectionInfoString(persister, collection, ce.getCurrentKey(), session),
+				LOG.debugf(
-                                                        MessageHelper.collectionInfoString(ce.getLoadedPersister(), collection, 
+						"Collection found: %s, was: %s (initialized)",
-                                                                                           ce.getLoadedKey(),
+						MessageHelper.collectionInfoString( persister, collection, ce.getCurrentKey(), session ),
-                                                                                           session));
+						MessageHelper.collectionInfoString( ce.getLoadedPersister(), collection, ce.getLoadedKey(), session )
-            else LOG.debugf("Collection found: %s, was: %s (uninitialized)",
+				);
-                            MessageHelper.collectionInfoString(persister, collection, ce.getCurrentKey(), session),
+			}
-                            MessageHelper.collectionInfoString(ce.getLoadedPersister(), collection, ce.getLoadedKey(), session));
+			else {
-        }
+				LOG.debugf(
+						"Collection found: %s, was: %s (uninitialized)",
+						MessageHelper.collectionInfoString( persister, collection, ce.getCurrentKey(), session ),
+						MessageHelper.collectionInfoString( ce.getLoadedPersister(), collection, ce.getLoadedKey(), session )
+				);
+			}
+		}
 
 		prepareCollectionForUpdate( collection, ce, factory );
-
 	}
 
 	/**
@@ -209,9 +216,8 @@ public static void processReachableCollection(
 	@SuppressWarnings( {"JavaDoc"})
 	private static void prepareCollectionForUpdate(
 			PersistentCollection collection,
-	        CollectionEntry entry,
+			CollectionEntry entry,
-	        SessionFactoryImplementor factory) {
+			SessionFactoryImplementor factory) {
-
 		if ( entry.isProcessed() ) {
 			throw new AssertionFailure( "collection was processed twice by flush()" );
 		}
@@ -219,37 +225,33 @@ private static void prepareCollectionForUpdate(
 
 		final CollectionPersister loadedPersister = entry.getLoadedPersister();
 		final CollectionPersister currentPersister = entry.getCurrentPersister();
-		if ( loadedPersister != null || currentPersister != null ) {					// it is or was referenced _somewhere_
+		if ( loadedPersister != null || currentPersister != null ) {
+			// it is or was referenced _somewhere_
 
-			boolean ownerChanged = loadedPersister != currentPersister ||				// if either its role changed,
+			// if either its role changed, or its key changed
-			                       !currentPersister
+			final boolean ownerChanged = loadedPersister != currentPersister
-					                       .getKeyType().isEqual(                       // or its key changed
+					|| !currentPersister.getKeyType().isEqual( entry.getLoadedKey(), entry.getCurrentKey(), factory );
-													entry.getLoadedKey(),
-			                                        entry.getCurrentKey(),
-			                                        factory
-			                       );
-
-			if (ownerChanged) {
 
+			if ( ownerChanged ) {
 				// do a check
-				final boolean orphanDeleteAndRoleChanged = loadedPersister != null &&
+				final boolean orphanDeleteAndRoleChanged =
-				                                           currentPersister != null &&
+						loadedPersister != null && currentPersister != null && loadedPersister.hasOrphanDelete();
-				                                           loadedPersister.hasOrphanDelete();
 
 				if (orphanDeleteAndRoleChanged) {
 					throw new HibernateException(
-							"Don't change the reference to a collection with cascade=\"all-delete-orphan\": " +
+							"Don't change the reference to a collection with delete-orphan enabled : "
-							loadedPersister.getRole()
+									+ loadedPersister.getRole()
 					);
 				}
 
 				// do the work
 				if ( currentPersister != null ) {
-					entry.setDorecreate( true );	// we will need to create new entries
+					entry.setDorecreate( true );
 				}
 
 				if ( loadedPersister != null ) {
-					entry.setDoremove( true );		// we will need to remove ye olde entries
+					// we will need to remove ye olde entries
+					entry.setDoremove( true );
 					if ( entry.isDorecreate() ) {
 						LOG.trace( "Forcing collection initialization" );
 						collection.forceInitialization();
@@ -260,8 +262,12 @@ else if ( collection.isDirty() ) {
 				// the collection's elements have changed
 				entry.setDoupdate( true );
 			}
-
 		}
+	}
 
+	/**
+	 * Disallow instantiation
+	 */
+	private Collections() {
 	}
 }

hibernate-core/src/main/java/org/hibernate/engine/internal/EntityEntryContext.java

@@ -31,7 +31,6 @@
 
 import org.jboss.logging.Logger;
 
-import org.hibernate.AssertionFailure;
 import org.hibernate.LockMode;
 import org.hibernate.engine.spi.EntityEntry;
 import org.hibernate.engine.spi.ManagedEntity;
@@ -56,22 +55,31 @@ public class EntityEntryContext {
 
 	private transient ManagedEntity head;
 	private transient ManagedEntity tail;
-	private transient int count = 0;
+	private transient int count;
 
 	private transient IdentityHashMap<Object,ManagedEntity> nonEnhancedEntityXref;
 
 	@SuppressWarnings( {"unchecked"})
 	private transient Map.Entry<Object,EntityEntry>[] reentrantSafeEntries = new Map.Entry[0];
-	private transient boolean dirty = false;
+	private transient boolean dirty;
 
+	/**
+	 * Constructs a EntityEntryContext
+	 */
 	public EntityEntryContext() {
 	}
 
+	/**
+	 * Adds the entity and entry to this context, associating them together
+	 *
+	 * @param entity The entity
+	 * @param entityEntry The entry
+	 */
 	public void addEntityEntry(Object entity, EntityEntry entityEntry) {
 		// IMPORTANT!!!!!
 		//		add is called more than once of some entities.  In such cases the first
 		//		call is simply setting up a "marker" to avoid infinite looping from reentrancy
-		//
+
 		// any addition (even the double one described above) should invalidate the cross-ref array
 		dirty = true;
 
@@ -127,11 +135,26 @@ public void addEntityEntry(Object entity, EntityEntry entityEntry) {
 		}
 	}
 
+	/**
+	 * Does this entity exist in this context, associated with an EntityEntry?
+	 *
+	 * @param entity The entity to check
+	 *
+	 * @return {@code true} if it is associated with this context
+	 */
 	public boolean hasEntityEntry(Object entity) {
 		return getEntityEntry( entity ) != null;
 	}
 
+	/**
+	 * Retrieve the associated EntityEntry for the entity
+	 *
+	 * @param entity The entity to retrieve the EntityEntry for
+	 *
+	 * @return The associated EntityEntry
+	 */
 	public EntityEntry getEntityEntry(Object entity) {
+		// essentially resolve the entity to a ManagedEntity...
 		final ManagedEntity managedEntity;
 		if ( ManagedEntity.class.isInstance( entity ) ) {
 			managedEntity = (ManagedEntity) entity;
@@ -143,14 +166,23 @@ else if ( nonEnhancedEntityXref == null ) {
 			managedEntity = nonEnhancedEntityXref.get( entity );
 		}
 
+		// and get/return the EntityEntry from the ManagedEntry
 		return managedEntity == null
 				? null
 				: managedEntity.$$_hibernate_getEntityEntry();
 	}
 
+	/**
+	 * Remove an entity from the context, returning the EntityEntry which was associated with it
+	 *
+	 * @param entity The entity to remove
+	 *
+	 * @return Tjee EntityEntry
+	 */
 	public EntityEntry removeEntityEntry(Object entity) {
 		dirty = true;
 
+		// again, resolve the entity to a ManagedEntity (which may not be possible for non-enhanced)...
 		final ManagedEntity managedEntity;
 		if ( ManagedEntity.class.isInstance( entity ) ) {
 			managedEntity = (ManagedEntity) entity;
@@ -162,16 +194,18 @@ else if ( nonEnhancedEntityXref == null ) {
 			managedEntity = nonEnhancedEntityXref.remove( entity );
 		}
 
+		// if we could not resolve it, just return (it was not associated with this context)
 		if ( managedEntity == null ) {
 			return null;
 		}
 
 		// prepare for re-linking...
-		ManagedEntity previous = managedEntity.$$_hibernate_getPreviousManagedEntity();
+		final ManagedEntity previous = managedEntity.$$_hibernate_getPreviousManagedEntity();
-		ManagedEntity next = managedEntity.$$_hibernate_getNextManagedEntity();
+		final ManagedEntity next = managedEntity.$$_hibernate_getNextManagedEntity();
 		managedEntity.$$_hibernate_setPreviousManagedEntity( null );
 		managedEntity.$$_hibernate_setNextManagedEntity( null );
 
+		// re-link
 		count--;
 
 		if ( count == 0 ) {
@@ -203,11 +237,20 @@ else if ( nonEnhancedEntityXref == null ) {
 			}
 		}
 
-		EntityEntry theEntityEntry = managedEntity.$$_hibernate_getEntityEntry();
+		// finally clean out the ManagedEntity and return the associated EntityEntry
+		final EntityEntry theEntityEntry = managedEntity.$$_hibernate_getEntityEntry();
 		managedEntity.$$_hibernate_setEntityEntry( null );
 		return theEntityEntry;
 	}
 
+	/**
+	 * The main bugaboo with IdentityMap that warranted this class in the first place.
+	 *
+	 * Return an array of all the entity/EntityEntry pairs in this context.  The array is to make sure
+	 * that the iterators built off of it are safe from concurrency/reentrancy
+	 *
+	 * @return The safe array
+	 */
 	public Map.Entry<Object, EntityEntry>[] reentrantSafeEntityEntries() {
 		if ( dirty ) {
 			reentrantSafeEntries = new EntityEntryCrossRefImpl[count];
@@ -225,6 +268,9 @@ public Map.Entry<Object, EntityEntry>[] reentrantSafeEntityEntries() {
 		return reentrantSafeEntries;
 	}
 
+	/**
+	 * Clear this context of all managed entities
+	 */
 	public void clear() {
 		dirty = true;
 
@@ -250,6 +296,9 @@ public void clear() {
 		reentrantSafeEntries = null;
 	}
 
+	/**
+	 * Down-grade locks to NONE for all entities in this context
+	 */
 	public void downgradeLocks() {
 		if ( head == null ) {
 			return;
@@ -263,6 +312,13 @@ public void downgradeLocks() {
 		}
 	}
 
+	/**
+	 * JDK serialization hook for serializing
+	 *
+	 * @param oos The stream to write ourselves to
+	 *
+	 * @throws IOException Indicates an IO exception accessing the given stream
+	 */
 	public void serialize(ObjectOutputStream oos) throws IOException {
 		log.tracef( "Starting serialization of [%s] EntityEntry entries", count );
 		oos.writeInt( count );
@@ -281,7 +337,17 @@ public void serialize(ObjectOutputStream oos) throws IOException {
 		}
 	}
 
-	public static EntityEntryContext deserialize(ObjectInputStream ois, StatefulPersistenceContext rtn) throws IOException, ClassNotFoundException {
+	/**
+	 * JDK serialization hook for deserializing
+	 *
+	 * @param ois The stream to read ourselves from
+	 * @param rtn The persistence context we belong to
+	 *
+	 * @throws IOException Indicates an IO exception accessing the given stream
+	 * @throws ClassNotFoundException Problem reading stream data
+	 */
+	public static EntityEntryContext deserialize(ObjectInputStream ois, StatefulPersistenceContext rtn)
+			throws IOException, ClassNotFoundException {
 		final int count = ois.readInt();
 		log.tracef( "Starting deserialization of [%s] EntityEntry entries", count );
 
@@ -332,6 +398,9 @@ public int getNumberOfManagedEntities() {
 		return count;
 	}
 
+	/**
+	 * The wrapper for entity classes which do not implement ManagedEntity
+	 */
 	private static class ManagedEntityImpl implements ManagedEntity {
 		private final Object entityInstance;
 		private EntityEntry entityEntry;
@@ -378,6 +447,28 @@ public ManagedEntityImpl(Object entityInstance) {
 		}
 	}
 
+	/**
+	 * Used in building the {@link #reentrantSafeEntityEntries()} entries
+	 */
+	public static interface EntityEntryCrossRef extends Map.Entry<Object,EntityEntry> {
+		/**
+		 * The entity
+		 *
+		 * @return The entity
+		 */
+		public Object getEntity();
+
+		/**
+		 * The associated EntityEntry
+		 *
+		 * @return The EntityEntry associated with the entity in this context
+		 */
+		public EntityEntry getEntityEntry();
+	}
+
+	/**
+	 * Implementation of the EntityEntryCrossRef interface
+	 */
 	private static class EntityEntryCrossRefImpl implements EntityEntryCrossRef {
 		private final Object entity;
 		private EntityEntry entityEntry;
@@ -414,9 +505,4 @@ public EntityEntry setValue(EntityEntry entityEntry) {
 			return old;
 		}
 	}
-
-	public static interface EntityEntryCrossRef extends Map.Entry<Object,EntityEntry> {
-		public Object getEntity();
-		public EntityEntry getEntityEntry();
-	}
 }

hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java

@@ -40,70 +40,81 @@
 
 /**
  * Algorithms related to foreign key constraint transparency
- * 
+ *
  * @author Gavin King
  */
 public final class ForeignKeys {
-	
+
-	private ForeignKeys() {}
+	/**
-	
+	 * Delegate for handling nullifying ("null"ing-out) non-cascaded associations
+	 */
 	public static class Nullifier {
-	
 		private final boolean isDelete;
 		private final boolean isEarlyInsert;
 		private final SessionImplementor session;
 		private final Object self;
-		
+
+		/**
+		 * Constructs a Nullifier
+		 *
+		 * @param self The entity
+		 * @param isDelete Are we in the middle of a delete action?
+		 * @param isEarlyInsert Is this an early insert (INSERT generated id strategy)?
+		 * @param session The session
+		 */
 		public Nullifier(Object self, boolean isDelete, boolean isEarlyInsert, SessionImplementor session) {
 			this.isDelete = isDelete;
 			this.isEarlyInsert = isEarlyInsert;
 			this.session = session;
 			this.self = self;
 		}
-		
+
 		/**
-		 * Nullify all references to entities that have not yet 
+		 * Nullify all references to entities that have not yet been inserted in the database, where the foreign key
-		 * been inserted in the database, where the foreign key
+		 * points toward that entity.
-		 * points toward that entity
+		 *
+		 * @param values The entity attribute values
+		 * @param types The entity attribute types
 		 */
-		public void nullifyTransientReferences(final Object[] values, final Type[] types) 
+		public void nullifyTransientReferences(final Object[] values, final Type[] types)
-		throws HibernateException {
+				throws HibernateException {
 			for ( int i = 0; i < types.length; i++ ) {
 				values[i] = nullifyTransientReferences( values[i], types[i] );
 			}
 		}
 
 		/**
-		 * Return null if the argument is an "unsaved" entity (ie. 
+		 * Return null if the argument is an "unsaved" entity (ie. one with no existing database row), or the
-		 * one with no existing database row), or the input argument 
+		 * input argument otherwise.  This is how Hibernate avoids foreign key constraint violations.
-		 * otherwise. This is how Hibernate avoids foreign key constraint
+		 *
-		 * violations.
+		 * @param value An entity attribute value
+		 * @param type An entity attribute type
 		 */
-		private Object nullifyTransientReferences(final Object value, final Type type) 
+		private Object nullifyTransientReferences(final Object value, final Type type)
-		throws HibernateException {
+				throws HibernateException {
 			if ( value == null ) {
 				return null;
 			}
 			else if ( type.isEntityType() ) {
-				EntityType entityType = (EntityType) type;
+				final EntityType entityType = (EntityType) type;
 				if ( entityType.isOneToOne() ) {
 					return value;
 				}
 				else {
-					String entityName = entityType.getAssociatedEntityName();
+					final String entityName = entityType.getAssociatedEntityName();
-					return isNullifiable(entityName, value) ? null : value;
+					return isNullifiable( entityName, value ) ? null : value;
 				}
 			}
 			else if ( type.isAnyType() ) {
-				return isNullifiable(null, value) ? null : value;
+				return isNullifiable( null, value ) ? null : value;
 			}
 			else if ( type.isComponentType() ) {
-				CompositeType actype = (CompositeType) type;
+				final CompositeType actype = (CompositeType) type;
-				Object[] subvalues = actype.getPropertyValues(value, session);
+				final Object[] subvalues = actype.getPropertyValues( value, session );
-				Type[] subtypes = actype.getSubtypes();
+				final Type[] subtypes = actype.getSubtypes();
 				boolean substitute = false;
 				for ( int i = 0; i < subvalues.length; i++ ) {
-					Object replacement = nullifyTransientReferences( subvalues[i], subtypes[i] );
+					final Object replacement = nullifyTransientReferences( subvalues[i], subtypes[i] );
 					if ( replacement != subvalues[i] ) {
 						substitute = true;
 						subvalues[i] = replacement;
@@ -119,20 +130,25 @@ else if ( type.isComponentType() ) {
 				return value;
 			}
 		}
-	
+
 		/**
-		 * Determine if the object already exists in the database, 
+		 * Determine if the object already exists in the database,
 		 * using a "best guess"
+		 *
+		 * @param entityName The name of the entity
+		 * @param object The entity instance
 		 */
-		private boolean isNullifiable(final String entityName, Object object) 
+		private boolean isNullifiable(final String entityName, Object object)
-		throws HibernateException {
+				throws HibernateException {
-			
+			if ( object == LazyPropertyInitializer.UNFETCHED_PROPERTY ) {
-			if (object==LazyPropertyInitializer.UNFETCHED_PROPERTY) return false; //this is kinda the best we can do...
+				// this is the best we can do...
-			
+				return false;
+			}
+
 			if ( object instanceof HibernateProxy ) {
 				// if its an uninitialized proxy it can't be transient
-				LazyInitializer li = ( (HibernateProxy) object ).getHibernateLazyInitializer();
+				final LazyInitializer li = ( (HibernateProxy) object ).getHibernateLazyInitializer();
-				if ( li.getImplementation(session) == null ) {
+				if ( li.getImplementation( session ) == null ) {
 					return false;
 					// ie. we never have to null out a reference to
 					// an uninitialized proxy
@@ -142,100 +158,130 @@ private boolean isNullifiable(final String entityName, Object object)
 					object = li.getImplementation();
 				}
 			}
-	
+
 			// if it was a reference to self, don't need to nullify
 			// unless we are using native id generation, in which
 			// case we definitely need to nullify
 			if ( object == self ) {
-				return isEarlyInsert || (
+				return isEarlyInsert
-					isDelete &&
+						|| ( isDelete && session.getFactory().getDialect().hasSelfReferentialForeignKeyBug() );
-					session.getFactory()
-						.getDialect()
-						.hasSelfReferentialForeignKeyBug()
-				);
 			}
-	
+
 			// See if the entity is already bound to this session, if not look at the
 			// entity identifier and assume that the entity is persistent if the
 			// id is not "unsaved" (that is, we rely on foreign keys to keep
 			// database integrity)
-	
+
-			EntityEntry entityEntry = session.getPersistenceContext().getEntry(object);
+			final EntityEntry entityEntry = session.getPersistenceContext().getEntry( object );
-			if ( entityEntry==null ) {
+			if ( entityEntry == null ) {
-				return isTransient(entityName, object, null, session);
+				return isTransient( entityName, object, null, session );
 			}
 			else {
-				return entityEntry.isNullifiable(isEarlyInsert, session);
+				return entityEntry.isNullifiable( isEarlyInsert, session );
 			}
-	
+
 		}
 
 	}
 
 	/**
 	 * Is this instance persistent or detached?
-	 * If <tt>assumed</tt> is non-null, don't hit the database to make the 
+	 * <p/>
-	 * determination, instead assume that value; the client code must be 
+	 * If <tt>assumed</tt> is non-null, don't hit the database to make the determination, instead assume that
-	 * prepared to "recover" in the case that this assumed result is incorrect.
+	 * value; the client code must be prepared to "recover" in the case that this assumed result is incorrect.
+	 *
+	 * @param entityName The name of the entity
+	 * @param entity The entity instance
+	 * @param assumed The assumed return value, if avoiding database hit is desired
+	 * @param session The session
+	 *
+	 * @return {@code true} if the given entity is not transient (meaning it is either detached/persistent)
 	 */
-	public static boolean isNotTransient(String entityName, Object entity, Boolean assumed, SessionImplementor session) 
+	@SuppressWarnings("SimplifiableIfStatement")
-	throws HibernateException {
+	public static boolean isNotTransient(String entityName, Object entity, Boolean assumed, SessionImplementor session) {
-		if (entity instanceof HibernateProxy) return true;
+		if ( entity instanceof HibernateProxy ) {
-		if ( session.getPersistenceContext().isEntryFor(entity) ) return true;
+			return true;
-		return !isTransient(entityName, entity, assumed, session);
+		}
+
+		if ( session.getPersistenceContext().isEntryFor( entity ) ) {
+			return true;
+		}
+
+		// todo : shouldnt assumed be revered here?
+
+		return !isTransient( entityName, entity, assumed, session );
 	}
-	
+
 	/**
 	 * Is this instance, which we know is not persistent, actually transient?
-	 * If <tt>assumed</tt> is non-null, don't hit the database to make the 
+	 * <p/>
-	 * determination, instead assume that value; the client code must be 
+	 * If <tt>assumed</tt> is non-null, don't hit the database to make the determination, instead assume that
-	 * prepared to "recover" in the case that this assumed result is incorrect.
+	 * value; the client code must be prepared to "recover" in the case that this assumed result is incorrect.
+	 *
+	 * @param entityName The name of the entity
+	 * @param entity The entity instance
+	 * @param assumed The assumed return value, if avoiding database hit is desired
+	 * @param session The session
+	 *
+	 * @return {@code true} if the given entity is transient (unsaved)
 	 */
-	public static boolean isTransient(String entityName, Object entity, Boolean assumed, SessionImplementor session) 
+	@SuppressWarnings("UnnecessaryUnboxing")
-	throws HibernateException {
+	public static boolean isTransient(String entityName, Object entity, Boolean assumed, SessionImplementor session) {
-		
+		if ( entity == LazyPropertyInitializer.UNFETCHED_PROPERTY ) {
-		if (entity== LazyPropertyInitializer.UNFETCHED_PROPERTY) {
 			// an unfetched association can only point to
 			// an entity that already exists in the db
 			return false;
 		}
-		
+
 		// let the interceptor inspect the instance to decide
-		Boolean isUnsaved = session.getInterceptor().isTransient(entity);
+		Boolean isUnsaved = session.getInterceptor().isTransient( entity );
-		if (isUnsaved!=null) return isUnsaved.booleanValue();
+		if ( isUnsaved != null ) {
-		
+			return isUnsaved.booleanValue();
+		}
+
 		// let the persister inspect the instance to decide
-		EntityPersister persister = session.getEntityPersister(entityName, entity);
+		final EntityPersister persister = session.getEntityPersister( entityName, entity );
-		isUnsaved = persister.isTransient(entity, session);
+		isUnsaved = persister.isTransient( entity, session );
-		if (isUnsaved!=null) return isUnsaved.booleanValue();
+		if ( isUnsaved != null ) {
+			return isUnsaved.booleanValue();
+		}
 
 		// we use the assumed value, if there is one, to avoid hitting
 		// the database
-		if (assumed!=null) return assumed.booleanValue();
+		if ( assumed != null ) {
-		
+			return assumed.booleanValue();
+		}
+
 		// hit the database, after checking the session cache for a snapshot
-		Object[] snapshot = session.getPersistenceContext().getDatabaseSnapshot(
+		final Object[] snapshot = session.getPersistenceContext().getDatabaseSnapshot(
 				persister.getIdentifier( entity, session ),
 				persister
 		);
-		return snapshot==null;
+		return snapshot == null;
 
 	}
 
 	/**
 	 * Return the identifier of the persistent or transient object, or throw
 	 * an exception if the instance is "unsaved"
-	 *
+	 * <p/>
-	 * Used by OneToOneType and ManyToOneType to determine what id value should 
+	 * Used by OneToOneType and ManyToOneType to determine what id value should
-	 * be used for an object that may or may not be associated with the session. 
+	 * be used for an object that may or may not be associated with the session.
-	 * This does a "best guess" using any/all info available to use (not just the 
+	 * This does a "best guess" using any/all info available to use (not just the
 	 * EntityEntry).
+	 *
+	 * @param entityName The name of the entity
+	 * @param object The entity instance
+	 * @param session The session
+	 *
+	 * @return The identifier
+	 *
+	 * @throws TransientObjectException if the entity is transient (does not yet have an identifier)
 	 */
 	public static Serializable getEntityIdentifierIfNotUnsaved(
-			final String entityName, 
+			final String entityName,
-			final Object object, 
+			final Object object,
-			final SessionImplementor session)
+			final SessionImplementor session) throws TransientObjectException {
-	throws HibernateException {
 		if ( object == null ) {
 			return null;
 		}
@@ -245,10 +291,10 @@ public static Serializable getEntityIdentifierIfNotUnsaved(
 				// context-entity-identifier returns null explicitly if the entity
 				// is not associated with the persistence context; so make some
 				// deeper checks...
-				if ( isTransient(entityName, object, Boolean.FALSE, session) ) {
+				if ( isTransient( entityName, object, Boolean.FALSE, session ) ) {
 					throw new TransientObjectException(
 							"object references an unsaved transient instance - save the transient instance before flushing: " +
-							(entityName == null ? session.guessEntityName( object ) : entityName)
+									(entityName == null ? session.guessEntityName( object ) : entityName)
 					);
 				}
 				id = session.getEntityPersister( entityName, object ).getIdentifier( object, session );
@@ -265,9 +311,9 @@ public static Serializable getEntityIdentifierIfNotUnsaved(
 	 * @param entityName - the entity name
 	 * @param entity - the entity instance
 	 * @param values - insertable properties of the object (including backrefs),
-	 *                 possibly with substitutions
+	 * possibly with substitutions
 	 * @param isEarlyInsert - true if the entity needs to be executed as soon as possible
-	 *                        (e.g., to generate an ID)
+	 * (e.g., to generate an ID)
 	 * @param session - the session
 	 *
 	 * @return the transient unsaved entity dependencies that are non-nullable,
@@ -278,18 +324,16 @@ public static NonNullableTransientDependencies findNonNullableTransientEntities(
 			Object entity,
 			Object[] values,
 			boolean isEarlyInsert,
-			SessionImplementor session
+			SessionImplementor session) {
-	) {
+		final Nullifier nullifier = new Nullifier( entity, false, isEarlyInsert, session );
-		Nullifier nullifier = new Nullifier( entity, false, isEarlyInsert, session );
 		final EntityPersister persister = session.getEntityPersister( entityName, entity );
 		final String[] propertyNames = persister.getPropertyNames();
 		final Type[] types = persister.getPropertyTypes();
 		final boolean[] nullability = persister.getPropertyNullability();
-		NonNullableTransientDependencies nonNullableTransientEntities = new NonNullableTransientDependencies();
+		final NonNullableTransientDependencies nonNullableTransientEntities = new NonNullableTransientDependencies();
 		for ( int i = 0; i < types.length; i++ ) {
 			collectNonNullableTransientEntities(
 					nullifier,
-					i,
 					values[i],
 					propertyNames[i],
 					types[i],
@@ -303,7 +347,6 @@ public static NonNullableTransientDependencies findNonNullableTransientEntities(
 
 	private static void collectNonNullableTransientEntities(
 			Nullifier nullifier,
-			int i,
 			Object value,
 			String propertyName,
 			Type type,
@@ -311,33 +354,32 @@ private static void collectNonNullableTransientEntities(
 			SessionImplementor session,
 			NonNullableTransientDependencies nonNullableTransientEntities) {
 		if ( value == null ) {
-			return; // EARLY RETURN
+			return;
 		}
-		if ( type.isEntityType()  ) {
+
-			EntityType entityType = (EntityType) type;
+		if ( type.isEntityType() ) {
-			if ( ! isNullable &&
+			final EntityType entityType = (EntityType) type;
-					! entityType.isOneToOne() &&
+			if ( !isNullable
-					nullifier.isNullifiable( entityType.getAssociatedEntityName(), value ) ) {
+					&& !entityType.isOneToOne()
+					&& nullifier.isNullifiable( entityType.getAssociatedEntityName(), value ) ) {
 				nonNullableTransientEntities.add( propertyName, value );
 			}
 		}
 		else if ( type.isAnyType() ) {
-			if ( ! isNullable &&
+			if ( !isNullable && nullifier.isNullifiable( null, value ) ) {
-					nullifier.isNullifiable( null, value ) ) {
 				nonNullableTransientEntities.add( propertyName, value );
 			}
 		}
 		else if ( type.isComponentType() ) {
-			CompositeType actype = (CompositeType) type;
+			final CompositeType actype = (CompositeType) type;
-			boolean[] subValueNullability = actype.getPropertyNullability();
+			final boolean[] subValueNullability = actype.getPropertyNullability();
 			if ( subValueNullability != null ) {
-				String[] subPropertyNames = actype.getPropertyNames();
+				final String[] subPropertyNames = actype.getPropertyNames();
-				Object[] subvalues = actype.getPropertyValues(value, session);
+				final Object[] subvalues = actype.getPropertyValues( value, session );
-				Type[] subtypes = actype.getSubtypes();
+				final Type[] subtypes = actype.getSubtypes();
 				for ( int j = 0; j < subvalues.length; j++ ) {
 					collectNonNullableTransientEntities(
 							nullifier,
-							j,
 							subvalues[j],
 							subPropertyNames[j],
 							subtypes[j],
@@ -349,4 +391,11 @@ else if ( type.isComponentType() ) {
 			}
 		}
 	}
+
+	/**
+	 * Disallow instantiation
+	 */
+	private ForeignKeys() {
+	}
+
 }

hibernate-core/src/main/java/org/hibernate/engine/internal/JoinHelper.java

@@ -36,45 +36,40 @@
  * @author Gavin King
  */
 public final class JoinHelper {
-	
-	private JoinHelper() {}
-	
 	/**
-	 * Get the aliased columns of the owning entity which are to 
+	 * Get the aliased columns of the owning entity which are to
 	 * be used in the join
 	 */
 	public static String[] getAliasedLHSColumnNames(
-			AssociationType type, 
+			AssociationType type,
-			String alias, 
+			String alias,
-			int property, 
+			int property,
 			OuterJoinLoadable lhsPersister,
-			Mapping mapping
+			Mapping mapping) {
-	) {
+		return getAliasedLHSColumnNames( type, alias, property, 0, lhsPersister, mapping );
-		return getAliasedLHSColumnNames(type, alias, property, 0, lhsPersister, mapping);
 	}
-	
+
 	/**
-	 * Get the columns of the owning entity which are to 
+	 * Get the columns of the owning entity which are to
 	 * be used in the join
 	 */
 	public static String[] getLHSColumnNames(
-			AssociationType type, 
+			AssociationType type,
-			int property, 
+			int property,
 			OuterJoinLoadable lhsPersister,
-			Mapping mapping
+			Mapping mapping) {
-	) {
+		return getLHSColumnNames( type, property, 0, lhsPersister, mapping );
-		return getLHSColumnNames(type, property, 0, lhsPersister, mapping);
 	}
-	
+
 	/**
-	 * Get the aliased columns of the owning entity which are to 
+	 * Get the aliased columns of the owning entity which are to
 	 * be used in the join
 	 */
 	public static String[] getAliasedLHSColumnNames(
 			AssociationType associationType,
 			String columnQualifier,
 			int propertyIndex,
-			int begin, 
+			int begin,
 			OuterJoinLoadable lhsPersister,
 			Mapping mapping) {
 		if ( associationType.useLHSPrimaryKey() ) {
@@ -90,7 +85,8 @@ public static String[] getAliasedLHSColumnNames(
 				);
 			}
 			else {
-				return ( (PropertyMapping) lhsPersister ).toColumns(columnQualifier, propertyName); //bad cast
+				//bad cast
+				return ( (PropertyMapping) lhsPersister ).toColumns( columnQualifier, propertyName );
 			}
 		}
 	}
@@ -112,41 +108,57 @@ private static String[] toColumns(OuterJoinLoadable persister, String columnQual
 	}
 
 	/**
-	 * Get the columns of the owning entity which are to 
+	 * Get the columns of the owning entity which are to be used in the join
-	 * be used in the join
+	 *
+	 * @param type The type representing the join
+	 * @param property The property index for the association type
+	 * @param begin ?
+	 * @param lhsPersister The persister for the left-hand-side of the join
+	 * @param mapping The mapping object (typically the SessionFactory)
+	 *
+	 * @return The columns for the left-hand-side of the join
 	 */
 	public static String[] getLHSColumnNames(
-			AssociationType type, 
+			AssociationType type,
-			int property, 
+			int property,
-			int begin, 
+			int begin,
 			OuterJoinLoadable lhsPersister,
-			Mapping mapping
+			Mapping mapping) {
-	) {
 		if ( type.useLHSPrimaryKey() ) {
 			//return lhsPersister.getSubclassPropertyColumnNames(property);
 			return lhsPersister.getIdentifierColumnNames();
 		}
 		else {
-			String propertyName = type.getLHSPropertyName();
+			final String propertyName = type.getLHSPropertyName();
-			if (propertyName==null) {
+			if ( propertyName == null ) {
 				//slice, to get the columns for this component
 				//property
 				return ArrayHelper.slice(
 						property < 0
 								? lhsPersister.getIdentifierColumnNames()
-								: lhsPersister.getSubclassPropertyColumnNames(property),
+								: lhsPersister.getSubclassPropertyColumnNames( property ),
-						begin, 
+						begin,
-						type.getColumnSpan(mapping) 
+						type.getColumnSpan( mapping )
-					);
+				);
 			}
 			else {
 				//property-refs for associations defined on a
 				//component are not supported, so no need to slice
-				return lhsPersister.getPropertyColumnNames(propertyName);
+				return lhsPersister.getPropertyColumnNames( propertyName );
 			}
 		}
 	}
-	
+
+	/**
+	 * Determine the name of the table that is the left-hand-side of the join.  Usually this is the
+	 * name of the main table from the left-hand-side persister.  But that is not the case with property-refs.
+	 *
+	 * @param type The type representing the join
+	 * @param propertyIndex The property index for the type
+	 * @param lhsPersister The persister for the left-hand-side of the join
+	 *
+	 * @return The table name
+	 */
 	public static String getLHSTableName(
 			AssociationType type,
 			int propertyIndex,
@@ -155,17 +167,17 @@ public static String getLHSTableName(
 			return lhsPersister.getTableName();
 		}
 		else {
-			String propertyName = type.getLHSPropertyName();
+			final String propertyName = type.getLHSPropertyName();
-			if (propertyName==null) {
+			if ( propertyName == null ) {
 				//if there is no property-ref, assume the join
 				//is to the subclass table (ie. the table of the
 				//subclass that the association belongs to)
-				return lhsPersister.getSubclassPropertyTableName(propertyIndex);
+				return lhsPersister.getSubclassPropertyTableName( propertyIndex );
 			}
 			else {
 				//handle a property-ref
-				String propertyRefTable = lhsPersister.getPropertyTableName(propertyName);
+				String propertyRefTable = lhsPersister.getPropertyTableName( propertyName );
-				if (propertyRefTable==null) {
+				if ( propertyRefTable == null ) {
 					//it is possible that the tree-walking in OuterJoinLoader can get to
 					//an association defined by a subclass, in which case the property-ref
 					//might refer to a property defined on a subclass of the current class
@@ -173,25 +185,32 @@ public static String getLHSTableName(
 					//assumes that the property-ref refers to a property of the subclass
 					//table that the association belongs to (a reasonable guess)
 					//TODO: fix this, add: OuterJoinLoadable.getSubclassPropertyTableName(String propertyName)
-					propertyRefTable = lhsPersister.getSubclassPropertyTableName(propertyIndex);
+					propertyRefTable = lhsPersister.getSubclassPropertyTableName( propertyIndex );
 				}
 				return propertyRefTable;
 			}
 		}
 	}
-	
+
 	/**
-	 * Get the columns of the associated table which are to 
+	 * Get the columns of the associated table which are to be used in the join
-	 * be used in the join
+	 *
+	 * @param type The type
+	 * @param factory The SessionFactory
+	 *
+	 * @return The columns for the right-hand-side of the join
 	 */
 	public static String[] getRHSColumnNames(AssociationType type, SessionFactoryImplementor factory) {
-		String uniqueKeyPropertyName = type.getRHSUniqueKeyPropertyName();
+		final String uniqueKeyPropertyName = type.getRHSUniqueKeyPropertyName();
-		Joinable joinable = type.getAssociatedJoinable(factory);
+		final Joinable joinable = type.getAssociatedJoinable( factory );
-		if (uniqueKeyPropertyName==null) {
+		if ( uniqueKeyPropertyName == null ) {
 			return joinable.getKeyColumnNames();
 		}
 		else {
-			return ( (OuterJoinLoadable) joinable ).getPropertyColumnNames(uniqueKeyPropertyName);
+			return ( (OuterJoinLoadable) joinable ).getPropertyColumnNames( uniqueKeyPropertyName );
 		}
 	}
+
+	private JoinHelper() {
+	}
 }

hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java

@@ -25,7 +25,6 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
@@ -39,46 +38,368 @@
 import org.hibernate.type.AssociationType;
 
 /**
+ * A sequence of {@link Join} delegates to make it "easier" to work with joins.  The "easier" part is obviously
+ * subjective ;)
+ * <p/>
+ * Additionally JoinSequence is a directed graph of other JoinSequence instances, as represented by the
+ * {@link #next} ({@link #setNext(JoinSequence)}) pointer.
+ *
  * @author Gavin King
+ * @author Steve Ebersole
+ *
+ * @see JoinFragment
  */
 public class JoinSequence {
-
 	private final SessionFactoryImplementor factory;
-	private final List<Join> joins = new ArrayList<Join>();
+
-	private boolean useThetaStyle = false;
 	private final StringBuilder conditions = new StringBuilder();
+	private final List<Join> joins = new ArrayList<Join>();
+
+	private boolean useThetaStyle;
 	private String rootAlias;
 	private Joinable rootJoinable;
 	private Selector selector;
 	private JoinSequence next;
-	private boolean isFromPart = false;
+	private boolean isFromPart;
 
-	@Override
+	/**
-    public String toString() {
+	 * Constructs a JoinSequence
-		StringBuilder buf = new StringBuilder();
+	 *
-		buf.append( "JoinSequence{" );
+	 * @param factory The SessionFactory
-		if ( rootJoinable != null ) {
+	 */
-			buf.append( rootJoinable )
+	public JoinSequence(SessionFactoryImplementor factory) {
-					.append( '[' )
+		this.factory = factory;
-					.append( rootAlias )
-					.append( ']' );
-		}
-		for ( int i = 0; i < joins.size(); i++ ) {
-			buf.append( "->" ).append( joins.get( i ) );
-		}
-		return buf.append( '}' ).toString();
 	}
 
-	public final class Join {
+	/**
+	 * Retrieve a JoinSequence that represents just the FROM clause parts
+	 *
+	 * @return The JoinSequence that represents just the FROM clause parts
+	 */
+	public JoinSequence getFromPart() {
+		final JoinSequence fromPart = new JoinSequence( factory );
+		fromPart.joins.addAll( this.joins );
+		fromPart.useThetaStyle = this.useThetaStyle;
+		fromPart.rootAlias = this.rootAlias;
+		fromPart.rootJoinable = this.rootJoinable;
+		fromPart.selector = this.selector;
+		fromPart.next = this.next == null ? null : this.next.getFromPart();
+		fromPart.isFromPart = true;
+		return fromPart;
+	}
 
+	/**
+	 * Create a full, although shallow, copy.
+	 *
+	 * @return The copy
+	 */
+	public JoinSequence copy() {
+		final JoinSequence copy = new JoinSequence( factory );
+		copy.joins.addAll( this.joins );
+		copy.useThetaStyle = this.useThetaStyle;
+		copy.rootAlias = this.rootAlias;
+		copy.rootJoinable = this.rootJoinable;
+		copy.selector = this.selector;
+		copy.next = this.next == null ? null : this.next.copy();
+		copy.isFromPart = this.isFromPart;
+		copy.conditions.append( this.conditions.toString() );
+		return copy;
+	}
+
+	/**
+	 * Add a join to this sequence
+	 *
+	 * @param associationType The type of the association representing the join
+	 * @param alias The RHS alias for the join
+	 * @param joinType The type of join (INNER, etc)
+	 * @param referencingKey The LHS columns for the join condition
+	 *
+	 * @return The Join memento
+	 *
+	 * @throws MappingException Generally indicates a problem resolving the associationType to a {@link Joinable}
+	 */
+	public JoinSequence addJoin(
+			AssociationType associationType,
+			String alias,
+			JoinType joinType,
+			String[] referencingKey) throws MappingException {
+		joins.add( new Join( factory, associationType, alias, joinType, referencingKey ) );
+		return this;
+	}
+
+	/**
+	 * Generate a JoinFragment
+	 *
+	 * @return The JoinFragment
+	 *
+	 * @throws MappingException Indicates a problem access the provided metadata, or incorrect metadata
+	 */
+	public JoinFragment toJoinFragment() throws MappingException {
+		return toJoinFragment( Collections.EMPTY_MAP, true );
+	}
+
+	/**
+	 * Generate a JoinFragment
+	 *
+	 * @param enabledFilters The filters associated with the originating session to properly define join conditions
+	 * @param includeExtraJoins Should {@link #addExtraJoins} to called.  Honestly I do not understand the full
+	 * ramifications of this argument
+	 *
+	 * @return The JoinFragment
+	 *
+	 * @throws MappingException Indicates a problem access the provided metadata, or incorrect metadata
+	 */
+	public JoinFragment toJoinFragment(Map enabledFilters, boolean includeExtraJoins) throws MappingException {
+		return toJoinFragment( enabledFilters, includeExtraJoins, null, null );
+	}
+
+	/**
+	 * Generate a JoinFragment
+	 *
+	 * @param enabledFilters The filters associated with the originating session to properly define join conditions
+	 * @param includeExtraJoins Should {@link #addExtraJoins} to called.  Honestly I do not understand the full
+	 * ramifications of this argument
+	 * @param withClauseFragment The with clause (which represents additional join restrictions) fragment
+	 * @param withClauseJoinAlias The
+	 *
+	 * @return The JoinFragment
+	 *
+	 * @throws MappingException Indicates a problem access the provided metadata, or incorrect metadata
+	 */
+	public JoinFragment toJoinFragment(
+			Map enabledFilters,
+			boolean includeExtraJoins,
+			String withClauseFragment,
+			String withClauseJoinAlias) throws MappingException {
+		final QueryJoinFragment joinFragment = new QueryJoinFragment( factory.getDialect(), useThetaStyle );
+		if ( rootJoinable != null ) {
+			joinFragment.addCrossJoin( rootJoinable.getTableName(), rootAlias );
+			final String filterCondition = rootJoinable.filterFragment( rootAlias, enabledFilters );
+			// JoinProcessor needs to know if the where clause fragment came from a dynamic filter or not so it
+			// can put the where clause fragment in the right place in the SQL AST.   'hasFilterCondition' keeps track
+			// of that fact.
+			joinFragment.setHasFilterCondition( joinFragment.addCondition( filterCondition ) );
+			if ( includeExtraJoins ) {
+				//TODO: not quite sure about the full implications of this!
+				addExtraJoins( joinFragment, rootAlias, rootJoinable, true );
+			}
+		}
+
+		Joinable last = rootJoinable;
+
+		for ( Join join : joins ) {
+			final String on = join.getAssociationType().getOnCondition( join.getAlias(), factory, enabledFilters );
+			String condition;
+			if ( last != null
+					&& isManyToManyRoot( last )
+					&& ((QueryableCollection) last).getElementType() == join.getAssociationType() ) {
+				// the current join represents the join between a many-to-many association table
+				// and its "target" table.  Here we need to apply any additional filters
+				// defined specifically on the many-to-many
+				final String manyToManyFilter = ( (QueryableCollection) last ).getManyToManyFilterFragment(
+						join.getAlias(),
+						enabledFilters
+				);
+				condition = "".equals( manyToManyFilter )
+						? on
+						: "".equals( on ) ? manyToManyFilter : on + " and " + manyToManyFilter;
+			}
+			else {
+				condition = on;
+			}
+
+			if ( withClauseFragment != null ) {
+				if ( join.getAlias().equals( withClauseJoinAlias ) ) {
+					condition += " and " + withClauseFragment;
+				}
+			}
+
+			joinFragment.addJoin(
+					join.getJoinable().getTableName(),
+					join.getAlias(),
+					join.getLHSColumns(),
+					JoinHelper.getRHSColumnNames( join.getAssociationType(), factory ),
+					join.joinType,
+					condition
+			);
+
+			//TODO: not quite sure about the full implications of this!
+			if ( includeExtraJoins ) {
+				addExtraJoins(
+						joinFragment,
+						join.getAlias(),
+						join.getJoinable(),
+						join.joinType == JoinType.INNER_JOIN
+				);
+			}
+			last = join.getJoinable();
+		}
+
+		if ( next != null ) {
+			joinFragment.addFragment( next.toJoinFragment( enabledFilters, includeExtraJoins ) );
+		}
+
+		joinFragment.addCondition( conditions.toString() );
+
+		if ( isFromPart ) {
+			joinFragment.clearWherePart();
+		}
+
+		return joinFragment;
+	}
+
+	@SuppressWarnings("SimplifiableIfStatement")
+	private boolean isManyToManyRoot(Joinable joinable) {
+		if ( joinable != null && joinable.isCollection() ) {
+			return ( (QueryableCollection) joinable ).isManyToMany();
+		}
+		return false;
+	}
+
+	private void addExtraJoins(JoinFragment joinFragment, String alias, Joinable joinable, boolean innerJoin) {
+		final boolean include = isIncluded( alias );
+		joinFragment.addJoins(
+				joinable.fromJoinFragment( alias, innerJoin, include ),
+				joinable.whereJoinFragment( alias, innerJoin, include )
+		);
+	}
+
+	private boolean isIncluded(String alias) {
+		return selector != null && selector.includeSubclasses( alias );
+	}
+
+	/**
+	 * Add a condition to this sequence.
+	 *
+	 * @param condition The condition
+	 *
+	 * @return {@link this}, for method chaining
+	 */
+	public JoinSequence addCondition(String condition) {
+		if ( condition.trim().length() != 0 ) {
+			if ( !condition.startsWith( " and " ) ) {
+				conditions.append( " and " );
+			}
+			conditions.append( condition );
+		}
+		return this;
+	}
+
+	/**
+	 * Add a condition to this sequence.  Typical usage here might be:
+	 * <pre>
+	 *     addCondition( "a", {"c1", "c2"}, "?" )
+	 * </pre>
+	 * to represent:
+	 * <pre>
+	 *     "... a.c1 = ? and a.c2 = ? ..."
+	 * </pre>
+	 *
+	 * @param alias The alias to apply to the columns
+	 * @param columns The columns to add checks for
+	 * @param condition The conditions to check against the columns
+	 *
+	 * @return {@link this}, for method chaining
+	 */
+	public JoinSequence addCondition(String alias, String[] columns, String condition) {
+		for ( String column : columns ) {
+			conditions.append( " and " )
+					.append( alias )
+					.append( '.' )
+					.append( column )
+					.append( condition );
+		}
+		return this;
+	}
+
+	/**
+	 * Set the root of this JoinSequence.  In SQL terms, this would be the driving table.
+	 *
+	 * @param joinable The entity/collection that is the root of this JoinSequence
+	 * @param alias The alias associated with that joinable.
+	 *
+	 * @return {@link this}, for method chaining
+	 */
+	public JoinSequence setRoot(Joinable joinable, String alias) {
+		this.rootAlias = alias;
+		this.rootJoinable = joinable;
+		return this;
+	}
+
+	/**
+	 * Sets the next join sequence
+	 *
+	 * @param next The next JoinSequence in the directed graph
+	 *
+	 * @return {@code this}, for method chaining
+	 */
+	public JoinSequence setNext(JoinSequence next) {
+		this.next = next;
+		return this;
+	}
+
+	/**
+	 * Set the Selector to use to determine how subclass joins should be applied.
+	 *
+	 * @param selector The selector to apply
+	 *
+	 * @return {@code this}, for method chaining
+	 */
+	public JoinSequence setSelector(Selector selector) {
+		this.selector = selector;
+		return this;
+	}
+
+	/**
+	 * Should this JoinSequence use theta-style joining (both a FROM and WHERE component) in the rendered SQL?
+	 *
+	 * @param useThetaStyle {@code true} indicates that theta-style joins should be used.
+	 *
+	 * @return {@code this}, for method chaining
+	 */
+	public JoinSequence setUseThetaStyle(boolean useThetaStyle) {
+		this.useThetaStyle = useThetaStyle;
+		return this;
+	}
+
+	public boolean isThetaStyle() {
+		return useThetaStyle;
+	}
+
+	public Join getFirstJoin() {
+		return joins.get( 0 );
+	}
+
+	/**
+	 * A subclass join selector
+	 */
+	public static interface Selector {
+		/**
+		 * Should subclasses be included in the rendered join sequence?
+		 *
+		 * @param alias The alias
+		 *
+		 * @return {@code true} if the subclass joins should be included
+		 */
+		public boolean includeSubclasses(String alias);
+	}
+
+	/**
+	 * Represents a join
+	 */
+	public static final class Join {
 		private final AssociationType associationType;
 		private final Joinable joinable;
 		private final JoinType joinType;
 		private final String alias;
 		private final String[] lhsColumns;
 
-		Join(AssociationType associationType, String alias, JoinType joinType, String[] lhsColumns)
+		Join(
-				throws MappingException {
+				SessionFactoryImplementor factory,
+				AssociationType associationType,
+				String alias,
+				JoinType joinType,
+				String[] lhsColumns) throws MappingException {
 			this.associationType = associationType;
 			this.joinable = associationType.getAssociatedJoinable( factory );
 			this.alias = alias;
@@ -107,195 +428,24 @@ public String[] getLHSColumns() {
 		}
 
 		@Override
-        public String toString() {
+		public String toString() {
 			return joinable.toString() + '[' + alias + ']';
 		}
 	}
 
-	public JoinSequence(SessionFactoryImplementor factory) {
+	@Override
-		this.factory = factory;
+	public String toString() {
-	}
+		final StringBuilder buf = new StringBuilder();
-
+		buf.append( "JoinSequence{" );
-	public JoinSequence getFromPart() {
-		JoinSequence fromPart = new JoinSequence( factory );
-		fromPart.joins.addAll( this.joins );
-		fromPart.useThetaStyle = this.useThetaStyle;
-		fromPart.rootAlias = this.rootAlias;
-		fromPart.rootJoinable = this.rootJoinable;
-		fromPart.selector = this.selector;
-		fromPart.next = this.next == null ? null : this.next.getFromPart();
-		fromPart.isFromPart = true;
-		return fromPart;
-	}
-
-	public JoinSequence copy() {
-		JoinSequence copy = new JoinSequence( factory );
-		copy.joins.addAll( this.joins );
-		copy.useThetaStyle = this.useThetaStyle;
-		copy.rootAlias = this.rootAlias;
-		copy.rootJoinable = this.rootJoinable;
-		copy.selector = this.selector;
-		copy.next = this.next == null ? null : this.next.copy();
-		copy.isFromPart = this.isFromPart;
-		copy.conditions.append( this.conditions.toString() );
-		return copy;
-	}
-
-	public JoinSequence addJoin(AssociationType associationType, String alias, JoinType joinType, String[] referencingKey)
-			throws MappingException {
-		joins.add( new Join( associationType, alias, joinType, referencingKey ) );
-		return this;
-	}
-
-	public JoinFragment toJoinFragment() throws MappingException {
-		return toJoinFragment( Collections.EMPTY_MAP, true );
-	}
-
-	public JoinFragment toJoinFragment(Map enabledFilters, boolean includeExtraJoins) throws MappingException {
-		return toJoinFragment( enabledFilters, includeExtraJoins, null, null );
-	}
-
-	public JoinFragment toJoinFragment(
-			Map enabledFilters,
-	        boolean includeExtraJoins,
-	        String withClauseFragment,
-	        String withClauseJoinAlias) throws MappingException {
-		QueryJoinFragment joinFragment = new QueryJoinFragment( factory.getDialect(), useThetaStyle );
 		if ( rootJoinable != null ) {
-			joinFragment.addCrossJoin( rootJoinable.getTableName(), rootAlias );
+			buf.append( rootJoinable )
-			String filterCondition = rootJoinable.filterFragment( rootAlias, enabledFilters );
+					.append( '[' )
-			// JoinProcessor needs to know if the where clause fragment came from a dynamic filter or not so it
+					.append( rootAlias )
-			// can put the where clause fragment in the right place in the SQL AST.   'hasFilterCondition' keeps track
+					.append( ']' );
-			// of that fact.
-			joinFragment.setHasFilterCondition( joinFragment.addCondition( filterCondition ) );
-			if (includeExtraJoins) { //TODO: not quite sure about the full implications of this!
-				addExtraJoins( joinFragment, rootAlias, rootJoinable, true );
-			}
 		}
-
+		for ( Join join : joins ) {
-		Joinable last = rootJoinable;
+			buf.append( "->" ).append( join );
-
-		for ( Join join: joins ) {
-			String on = join.getAssociationType().getOnCondition( join.getAlias(), factory, enabledFilters );
-			String condition = null;
-			if ( last != null &&
-			        isManyToManyRoot( last ) &&
-			        ( ( QueryableCollection ) last ).getElementType() == join.getAssociationType() ) {
-				// the current join represents the join between a many-to-many association table
-				// and its "target" table.  Here we need to apply any additional filters
-				// defined specifically on the many-to-many
-				String manyToManyFilter = ( ( QueryableCollection ) last )
-				        .getManyToManyFilterFragment( join.getAlias(), enabledFilters );
-				condition = "".equals( manyToManyFilter )
-						? on
-						: "".equals( on )
-								? manyToManyFilter
-								: on + " and " + manyToManyFilter;
-			}
-			else {
-				condition = on;
-			}
-			if ( withClauseFragment != null ) {
-				if ( join.getAlias().equals( withClauseJoinAlias ) ) {
-					condition += " and " + withClauseFragment;
-				}
-			}
-			joinFragment.addJoin(
-			        join.getJoinable().getTableName(),
-					join.getAlias(),
-					join.getLHSColumns(),
-					JoinHelper.getRHSColumnNames( join.getAssociationType(), factory ),
-					join.joinType,
-					condition
-			);
-			if (includeExtraJoins) { //TODO: not quite sure about the full implications of this!
-				addExtraJoins( joinFragment, join.getAlias(), join.getJoinable(), join.joinType == JoinType.INNER_JOIN );
-			}
-			last = join.getJoinable();
 		}
-		if ( next != null ) {
+		return buf.append( '}' ).toString();
-			joinFragment.addFragment( next.toJoinFragment( enabledFilters, includeExtraJoins ) );
-		}
-		joinFragment.addCondition( conditions.toString() );
-		if ( isFromPart ) joinFragment.clearWherePart();
-		return joinFragment;
-	}
-
-	private boolean isManyToManyRoot(Joinable joinable) {
-		if ( joinable != null && joinable.isCollection() ) {
-			QueryableCollection persister = ( QueryableCollection ) joinable;
-			return persister.isManyToMany();
-		}
-		return false;
-	}
-
-	private boolean isIncluded(String alias) {
-		return selector != null && selector.includeSubclasses( alias );
-	}
-
-	private void addExtraJoins(JoinFragment joinFragment, String alias, Joinable joinable, boolean innerJoin) {
-		boolean include = isIncluded( alias );
-		joinFragment.addJoins( joinable.fromJoinFragment( alias, innerJoin, include ),
-				joinable.whereJoinFragment( alias, innerJoin, include ) );
-	}
-
-	public JoinSequence addCondition(String condition) {
-		if ( condition.trim().length() != 0 ) {
-			if ( !condition.startsWith( " and " ) ) conditions.append( " and " );
-			conditions.append( condition );
-		}
-		return this;
-	}
-
-	public JoinSequence addCondition(String alias, String[] columns, String condition) {
-		for ( int i = 0; i < columns.length; i++ ) {
-			conditions.append( " and " )
-					.append( alias )
-					.append( '.' )
-					.append( columns[i] )
-					.append( condition );
-		}
-		return this;
-	}
-
-	public JoinSequence setRoot(Joinable joinable, String alias) {
-		this.rootAlias = alias;
-		this.rootJoinable = joinable;
-		return this;
-	}
-
-	public JoinSequence setNext(JoinSequence next) {
-		this.next = next;
-		return this;
-	}
-
-	public JoinSequence setSelector(Selector s) {
-		this.selector = s;
-		return this;
-	}
-
-	public JoinSequence setUseThetaStyle(boolean useThetaStyle) {
-		this.useThetaStyle = useThetaStyle;
-		return this;
-	}
-
-	public boolean isThetaStyle() {
-		return useThetaStyle;
-	}
-
-	public int getJoinCount() {
-		return joins.size();
-	}
-
-	public Iterator iterateJoins() {
-		return joins.iterator();
-	}
-
-	public Join getFirstJoin() {
-		return joins.get( 0 );
-	}
-
-	public static interface Selector {
-		public boolean includeSubclasses(String alias);
 	}
 }