-->
These old forums are deprecated now and set to read-only. We are waiting for you on our new forums!
More modern, Discourse-based and with GitHub/Google/Twitter authentication built-in.

All times are UTC - 5 hours [ DST ]



Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 8 posts ] 
Author Message
 Post subject: Feedback on an equals/hashCode approach
PostPosted: Fri Oct 07, 2005 2:14 pm 
Newbie

Joined: Thu Oct 06, 2005 7:23 pm
Posts: 7
I'd like to present and gain feedback on an alternate approach that we've come up with to the whole equals/hashCode problem. We haven't done extensive testing, and I'd like to hear about gotchas from those in the know prior to making a significant investment.

After reading what's available (forums, Hib In Action, etc.), one thing's certainly clear - there's simply no perfect solution. In it's absence, however, we've attempted to come up with one that's optimized to our situation and constraints, but it's not without it's limitations. In fact, I admit it's complete hackery in some respects.

Our goals were the following:
1. Simplify the creation of new entities by eliminating the need for an immutable logical (business) key and the construction of customized eq/hc methods. Many of our objects simply don't naturally fit this model.
2. Maintain equality across session boundaries which mandates deviation from the standard Java model of object identity.

Our constraints include the following:
1. All entities will have surrogate keys
2. The database will either be PostgreSQL or Oracle and all surrogate keys will be supplied from sequences. Note that this enables the retrieval of new identifiers without previously inserting a table row.
3. We're building a web app and a session will typically be available when performing manipulations of the domain objects (creation, adding to collections, and what not). We expect rich manipulation of detached objects to be virtually non-existent.

Our approach:
1. We use the standard DAO pattern, but extend it with an additional generate() method that returns a fresh object with an id allocated from the database. Note that it's not persistent yet, but does contain a unique id. Logically makes sense, but getting it to work is the hackery you'll see below.
2. All entities extend an AbstractEntity class that contains final implementations of the equals and hashCode methods along with an abstract getEntityClass() method.
3. Entities that will be added to a collection must be generated rather than simply created. Other than that, all works as usual.

Comments:
- Of course, we could use the save/flush approach, but it's more awkward and error prone for some of our more junior developers not to mention polluting code with session dependencies and the performance implications.
- Unit testing is an issue we need to address with this approach. Ideas include simply having a dependency injected that generates ids using a simple counter during testing.
- I know this is a hack and it stands the hairs on the back of my neck up too, but I'll sacrifice elegance for a real world solution with known shortcomings that works in our environment and reduces code bloat.

Client Code:
Code:
Set<User> users = new HashSet<User>();
UserDao userDao = new UserDao();
User newUser = userDao.generate();
// Set some properties, etc.
// Might want to have the create automatically scheduled upon generate?
userDao.create(newUser);
users.add(newUser);


Entity Code:
Yes, I know I'm dangerously casting objects in the getIdentifierGenerator() method, but it's the only way I know of to get to the generators.
Code:
public abstract class AbstractEntityDao {
   private static Map<Class, IdentifierGenerator> entityIdentifierGenerators = new HashMap<Class, IdentifierGenerator>();

   public AbstractEntityDao() {
   }

   /**
    * The concrete persistent class for which this DAO manages persistence.
    * Each DAO subclass must override this method to specify the persistent
    * class it persists.
    * The entity class is used in obtaining the id generator for the entity.
    */
   public abstract Class getEntityClass();
   
   protected Long generateEntityIdentifier() {
      // IMPORTANT - Note the null object used in id generation.  This only works with specific identifier
      // generators such as sequence and the like that don't rely on a row first being created in the database 
      return (Long) getIdentifierGenerator().generate((SessionImpl)HibernateUtil.getCurrentSession(), null);
   }
   
   protected IdentifierGenerator getIdentifierGenerator() {
      IdentifierGenerator idGenerator = entityIdentifierGenerators.get(getEntityClass());
      if (idGenerator == null) {
         synchronized (entityIdentifierGenerators) {
            idGenerator = entityIdentifierGenerators.get(getEntityClass());
            if (idGenerator == null) {
               EntityPersister persister = (AbstractEntityPersister) HibernateUtil.getSessionFactory().getClassMetadata(getEntityClass());
               idGenerator = persister.getIdentifierGenerator();
               entityIdentifierGenerators.put(getEntityClass(), idGenerator);
            }
         }
      }
      return idGenerator;
   }
}


Code:
public class UserDao extends AbstractEntityDao {
   public UserDao() {
      super();
   }
   public User generate() {
      return new User(generateEntityIdentifier());
   }
   @Override
   public Class getEntityClass() {
      return User.class;
   }
}


The eq/hc implementations need more work, but you get the idea.
Code:
/**
* The base class for all domain entities.
* All entities are required to have a surrogate primary key of type Long.
*/
public abstract class AbstractEntity {
   private Long id;
   
   public AbstractEntity() {}
   public AbstractEntity(Long id) {
      this.id = id;
   }
   
   /**
    * The concrete persistent class that this entity represents.
    * Each concrete entity subclass must override this method to specify
    * the persistent class it represents.  A persistent class is one that's
    * mapped in Hibernate to a table containing a surrogate identifier.
    * The entity class is used for implementing equals.
    */
   public abstract Class getEntityClass();

   public Long getId() {
      return id;
   }

   private void setId(Long id) {
      this.id = id;
   }
   
   @Override
   public final boolean equals(Object that) {
      if (this == that) return true;
      if (!(getEntityClass().isInstance(that))) return false;
      // If you get the following exception, you're likely trying to add a transient entity to a collection
      // Use the generate() method of the entity's respective DAO instead to generate an entity with a valid id
      if (id == null) throw new IllegalStateException("id not set; use DAO generation instead of creation to obtain an entity with a valid id");
      return  id.equals(((AbstractEntity) that).getId());
   }
   
   @Override
   public final int hashCode() {
      // If you get the following exception, you're likely trying to add a transient entity to a collection
      // Use the generate() method of the entity's respective DAO instead to generate an entity with a valid id
      if (id == null) throw new IllegalStateException("id not set; use DAO generation instead of creation to obtain an entity with a valid id");
      return id.hashCode();
   }
}


Details omitted for brevity, but creation of new entities is straightforward and reasonably constrained.
Code:
/**
* A persistent entity class that represents a user of the system 
*/
public class User extends AbstractEntity {
   public User() {
      super();
   }
   public User(Long id) {
      super(id);
   }

   @Override
   public Class getEntityClass() {
      return User.class;
   }
}


Top
 Profile  
 
 Post subject: Re: Feedback on an equals/hashCode approach
PostPosted: Fri Oct 07, 2005 2:23 pm 
Expert
Expert

Joined: Mon Feb 14, 2005 12:32 pm
Posts: 609
Location: Atlanta, GA - USA
Why do you want to generate ID's prior to persisting the Object ?

_________________
Preston

Please don't forget to give credit if/when you get helpful information.


Top
 Profile  
 
 Post subject: Re: Feedback on an equals/hashCode approach
PostPosted: Fri Oct 07, 2005 2:47 pm 
Newbie

Joined: Thu Oct 06, 2005 7:23 pm
Posts: 7
pksiv wrote:
Why do you want to generate ID's prior to persisting the Object ?


So they can safely be added to sets while preserving the semantics of equals/hashCode. This is the whole problem and difficulty behind a proper implementation of these methods for persistent objects and the reason why having a "stable" business key is currently the recommended best practice.

Our goal was to keep the semantics and implementation of eq/hc simple (they just depend on the entity's id and not some business key that needs to be determined and coded for each entity) while preserving their behavior across persistence and session boundaries. Once set, an entity's id never changes regardless of whether it's transient and its hash code, as a result, never changes as well.


Top
 Profile  
 
 Post subject: Re: Feedback on an equals/hashCode approach
PostPosted: Fri Oct 07, 2005 2:55 pm 
Expert
Expert

Joined: Mon Feb 14, 2005 12:32 pm
Posts: 609
Location: Atlanta, GA - USA
lance2010 wrote:
pksiv wrote:
Why do you want to generate ID's prior to persisting the Object ?


So they can safely be added to sets while preserving the semantics of equals/hashCode. This is the whole problem and difficulty behind a proper implementation of these methods for persistent objects and the reason why having a "stable" business key is currently the recommended best practice.

Our goal was to keep the semantics and implementation of eq/hc simple (they just depend on the entity's id and not some business key that needs to be determined and coded for each entity) while preserving their behavior across persistence and session boundaries. Once set, an entity's id never changes regardless of whether it's transient and its hash code, as a result, never changes as well.


I guess I can understand your logic, although I personally still prefer eq/hc to have business meaning, it also means that you could churn through ID's that are never actually persisted.

And don't you also have to keep track of whether an item in the collection is new or not since you'll have to use the "assigned" IdGenerator ?

_________________
Preston

Please don't forget to give credit if/when you get helpful information.


Top
 Profile  
 
 Post subject: Re: Feedback on an equals/hashCode approach
PostPosted: Fri Oct 07, 2005 4:14 pm 
Newbie

Joined: Thu Oct 06, 2005 7:23 pm
Posts: 7
pksiv wrote:
I guess I can understand your logic, although I personally still prefer eq/hc to have business meaning, it also means that you could churn through ID's that are never actually persisted.

I'm not worried about churning the ids - generated objects are expected to be persisted shortly thereafter. More a theoretical rather than a practical problem.
Note that this scheme doesn't prohibit the creation of purely transient instances with no id. The rule is more like "if you're gonna add an object to a collection, generate don't instaniate"

Quote:
And don't you also have to keep track of whether an item in the collection is new or not since you'll have to use the "assigned" IdGenerator ?

Very good point! After looking through Hibernate source for a while, I'll have to get back to you on this. I thought that performing a save on an entity would automatically queue it for an insert if it wasn't in its map of retrieved objects, but after looking further, it seems that's not the case.
Options I'm considering:
1. Just make all entities versioned since the version unsaved-value takes precedence for determining transient status (see AbstractEntityPersister.isTransient()). Probably not a bad idea anyway, but I don't like the smell of this.
2. Use assigned identifiers as you mention, but it's a less attractive option.
3. Write a custom interceptor and override isTransient() - yuck!

Any other (hopefully simple) ideas?


Top
 Profile  
 
 Post subject: Re: Feedback on an equals/hashCode approach
PostPosted: Sun Oct 09, 2005 6:08 pm 
Beginner
Beginner

Joined: Thu Jun 02, 2005 5:09 am
Posts: 22
Quote:
Options I'm considering:
1. Just make all entities versioned since the version unsaved-value takes precedence for determining transient status (see AbstractEntityPersister.isTransient()). Probably not a bad idea anyway, but I don't like the smell of this.
2. Use assigned identifiers as you mention, but it's a less attractive option.
3. Write a custom interceptor and override isTransient() - yuck!

Any other (hopefully simple) ideas?


In our app, which has similar requirements as yours, we use application-assigned ID's with Hibernate-managed versions (your 1 and 2 combined, I guess). The versions determine if an instance is transient or not. The ID is assigned in the constructor, we have an abstract superclass that also implements equals and hashCode, like your solution.
It's a solution that works perfectly and is relatively clean, since most of our entity classes do not have any properties that make for a good business key. In our case, there's an extra reason for using assigned ID's (we have a client with its own Hibernate-managed database, and objects present on both client and server need the same ID) but even if this isn't the case, I'd still recommend this solution.

BTW, on the client we do not need versioning (single user app) and optimistic locking just gets in the way, so we use a slightly different (generated) mapping without a <version> and a custom interceptor that simply sets a normal (i.e. not persistent) boolean property in onLoad and onSave and checks that property in isTransient. IMHO it's not yuck, it's a very nice way to use the same object on both client and server with different mappings but with the same ID's, which implies that they're the same according to equals().

Joris


Top
 Profile  
 
 Post subject: Re: Feedback on an equals/hashCode approach
PostPosted: Sun Oct 09, 2005 9:15 pm 
Newbie

Joined: Thu Oct 06, 2005 7:23 pm
Posts: 7
Got it!
I just got a chance to take another look at this and polish off what I think is a relatively clean solution to this whole problem. Thanks again to those that replied and especially pksiv for mentioning assigned identifiers. Though not the answer, it did set me off in the right direction and is a significant part of the solution being considered.
Details follow for those whom may benefit.
Additional comments welcome.

The modified approach is simply the following:
1. Use the DAO generation approach originally described.
2. Create an interface for obtaining the generated status of any entity. This is just an isGenerated() method that indicates whether the object's id was assigned upon construction (our chosen contract for generation).
3. Use a custom, delegate id generator that's a hybrid between the assigned strategy and the identifier generation strategy of choice.

Comments:
- Only after weeding my way through the code (open source!) did I realize that I needed a different identifier generation strategy and that it's possible to roll your own (I know it's documented - I forgot).
- I prefer this strategy to interception in that its influence is limited to creation and id generation. After that, it's business as usual.

Code:

Generated Entity Interface - Pretty self-explanatory
Code:
/**
* Implemented by entities that support generation in addition to creation.
* A generated entity is one whose persistent identifier is allocated at
* creation while the object is still transient.  This strategy has the
* advantage of simplifying equals and hashCode implementations for
* persistent objects by ensuring a stable identifier over an object's
* lifetime.<br>
* <b>IMPORTANT - </b>Note the following design requirements for using
* generated entities:
* <ol>
* <li>TODO</li>
* </ol>
*/
public interface GeneratedEntity {
   /**
    * True if the identifier for this object was assigned at creation.<br>
    * Note that generated entities are expected to provide a constructor
    * that takes the initial id as an argument.<br>
    * Note that the generated status of an entity does *not* imply a
    * specific persistence status.  All generated entities start as
    * transient instances, but are expected to be persisted shortly
    * thereafter.
    */
   public boolean isGenerated();
}


Delegate Id Generator
Note that this custom generator aggregates both the assigned and whatever generation strategy is appropriate for the specific business object. The only caveat is that the "real" generation strategy must support id generation without an actual object. Sequences, the only generators we care about, support this, but others may as well.
Note also that all configuration params are also passed wholesale to the delegate generator for its use.
Code:
public class DelegateGenerator implements IdentifierGenerator, Configurable {
   
   /**
    * The generator parameter specifies the underlying id generator to use
    * when an identifier hasn't already been assigned.
    */
   public static final String DELEGATE = "delegate";
   
   private IdentifierGenerator assignedGenerator;
   private IdentifierGenerator delegateGenerator;
   
   public Serializable generate(SessionImplementor session, Object object)
         throws HibernateException {
      if (object == null) {
         // Use the underlying id generation strategy if no object is specified
         // Note that this should *only* occur when using DAO generation
         return delegateGenerator.generate(session, object);
      } else {
         if (object instanceof GeneratedEntity) {
            if (((GeneratedEntity)object).isGenerated()) {
               return assignedGenerator.generate(session, object);
            } else {
               return delegateGenerator.generate(session, object);
            }
         } else {
            throw new IdentifierGenerationException("the delegate generator is only intended for use with subclasses of AbstractEntity");
         }
      }
   }

   public void configure(Type type, Properties params, Dialect d)
         throws MappingException {
      String generatorName = params.getProperty(DELEGATE);
      if (generatorName == null) throw new MappingException("param named \"delegate\" is required for delegate generation strategy");
      
      // Create the assigned and delegate id generators
      assignedGenerator = IdentifierGeneratorFactory.create("assigned", type, params, d);
      delegateGenerator = IdentifierGeneratorFactory.create(generatorName, type, params, d);
   }
}


Sample Descriptor
Code:
<id name="id" type="long" column="user_id">
        <generator class="test.DelegateGenerator">
                <param name="delegate">hilo</param>
                <param name="table">hi_value</param>
                <param name="column">next_value</param>
                <param name="max_lo">100</param>
        </generator>
</id>


Modified Abstract Entity
Code:
public abstract class AbstractEntity implements GeneratedEntity {
   private Long id;
   private boolean generated = false;
   
   public AbstractEntity() {}
   public AbstractEntity(Long id) {
      this.id = id;
      generated = true;
   }
   
   public boolean isGenerated() {
      return generated;
   }
   // Details omitted
}


Top
 Profile  
 
 Post subject: Re: Feedback on an equals/hashCode approach
PostPosted: Thu Nov 24, 2005 3:17 pm 
Beginner
Beginner

Joined: Thu Nov 24, 2005 2:58 pm
Posts: 21
jkuipers wrote:
In our app, which has similar requirements as yours, we use application-assigned ID's with Hibernate-managed versions (your 1 and 2 combined, I guess). The versions determine if an instance is transient or not. The ID is assigned in the constructor, we have an abstract superclass that also implements equals and hashCode, like your solution.


I agree completely. This is similar to the approach I'm currently trying to implement but without much success :( I was hoping to use the builtin generators albeit with the HBM marked as an assigned generator.

Something like this can bootstrap the right generator (based on the dialect)

Code:
ResourceBundle bundle = ResourceBundle.getBundle("hibernate");
String d = bundle.getString("hibernate.dialect");
Dialect dialect = DialectFactory.buildDialect(d);
Class clazz = dialect.getNativeIdentifierGeneratorClass();
idgen = (IdentifierGenerator) clazz.newInstance();


More preciesly I want to assign IDs in the obj ctor (use versioning to resolve the isTransient() issue) and ideally use one of the builtin generators to make the ID - preferably native so its DB centric. What I cant figure out is the sessionimplementor and obj args that the generate() call needs.

Seems to me hibernate has enough of the code to do this and provide it as a utility method 'Util.getNextId()' or whatever and mark the generators are assigned

In the absence of such any ideas on how best to do this would be much appreciated?


Top
 Profile  
 
Display posts from previous:  Sort by  
Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 8 posts ] 

All times are UTC - 5 hours [ DST ]


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum

Search for:
© Copyright 2014, Red Hat Inc. All rights reserved. JBoss and Hibernate are registered trademarks and servicemarks of Red Hat, Inc.