HashCode throws a nullpointer exception

I have a puzzle for you.

I am making a herb store web app and this is my database :

• The store can have many products
• A product can contain many herbs

These are my JPA classes :

public class StoreJPA {
...
    @OneToMany(mappedBy="storeJpa", cascade = CascadeType.ALL, orphanRemoval=true, fetch=FetchType.EAGER)
	private Set<ProductJPA> specialOffers = new HashSet<ProductJPA>();
...
}

public class ProductJPA {
    @ManyToOne
	@JoinColumn(name="store_id")
	private StoreJPA storeJpa;

    @OneToMany(mappedBy="productJpa", cascade = CascadeType.ALL, orphanRemoval=true, fetch=FetchType.EAGER)
	private Set<ContainsJPA> contains = new HashSet<ContainsJPA>();
...
    private Set<HerbJPA> getHerbs(){
		return contains.stream().map(h -> h.getHerbJpa()).collect(Collectors.toSet());
	}

    @Override
	public int hashCode(){
		long h = 1125899906842597L; // prime
		
		for(ProductHasHerbJPA phh : contains){
			h = 31*h + phh.getHerbJpa().getId();
		}
		
		return (int)(31*h + storeJpa.getId());
	}
	
	@Override
	public boolean equals(Object o){
		if(o!=null && o instanceof ProductJPA){
			if(o==this)
				return true;
			return ((ProductJPA)o).getStoreJpa().getId()==storeJpa.getId() && 
					((ProductJPA)o).getHerbs().equals(getHerbs()) // compare herbs they contain
		}
		return false;
	}
...
}

public class ContainsJPA {
    @Id
    private Long id;

    @ManyToOne
	@JoinColumn(name="product_id")
	private ProductJPA productJpa;
	
	@ManyToOne
	@JoinColumn(name="herb_id")
	private HerbJPA herbJpa;

...
    @Override
	public int hashCode(){
		long h = 1125899906842597L + productJpa.getId();	// <-- nullpointer exception	
		
		return (int)(31*h + herbJpa.getId());
	}
	
	@Override
	public boolean equals(Object o){
		if( o != null && o instanceof HerbLocaleJPA) {
			if(o==this) {
        		return true;
        	}
			return ((ProductHasHerbJPA)o).getHerbJpa().getId()==herbJpa.getId() && 
					((ProductHasHerbJPA)o).getProductJpa().getId()==productJpa.getId();
		}
		
		return false;
	}
...
}

Adding a new product with a list of herbs works fine.
But when i run this and try to get the products in the store, i get a NullPointerException :

> java.lang.NullPointerException at
> com.green.store.entities.ContainsJPA.hashCode(ContainsJPA.java:64)
> at java.util.HashMap.hash(HashMap.java:339) at
> java.util.HashMap.put(HashMap.java:612) at
> java.util.HashSet.add(HashSet.java:220) at
> java.util.AbstractCollection.addAll(AbstractCollection.java:344) at
> org.hibernate.collection.internal.PersistentSet.endRead(PersistentSet.java:327)
> at
> org.hibernate.engine.loading.internal.CollectionLoadContext.endLoadingCollection(CollectionLoadContext.java:234)
> at
> org.hibernate.engine.loading.internal.CollectionLoadContext.endLoadingCollections(CollectionLoadContext.java:221)
> at
> org.hibernate.engine.loading.internal.CollectionLoadContext.endLoadingCollections(CollectionLoadContext.java:194)
> at
> org.hibernate.loader.plan.exec.process.internal.CollectionReferenceInitializerImpl.endLoading(CollectionReferenceInitializerImpl.java:154)
> at
> org.hibernate.loader.plan.exec.process.internal.AbstractRowReader.finishLoadingCollections(AbstractRowReader.java:249)
> at
> org.hibernate.loader.plan.exec.process.internal.AbstractRowReader.finishUp(AbstractRowReader.java:212)
> at
> org.hibernate.loader.plan.exec.process.internal.ResultSetProcessorImpl.extractResults(ResultSetProcessorImpl.java:133)
> at
> org.hibernate.loader.plan.exec.internal.AbstractLoadPlanBasedLoader.executeLoad(AbstractLoadPlanBasedLoader.java:122)
> at
> org.hibernate.loader.plan.exec.internal.AbstractLoadPlanBasedLoader.executeLoad(AbstractLoadPlanBasedLoader.java:86)
> at
> org.hibernate.loader.entity.plan.AbstractLoadPlanBasedEntityLoader.load(AbstractLoadPlanBasedEntityLoader.java:167)
> at
> org.hibernate.persister.entity.AbstractEntityPersister.load(AbstractEntityPersister.java:4087)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.loadFromDatasource(DefaultLoadEventListener.java:508)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.doLoad(DefaultLoadEventListener.java:478)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.load(DefaultLoadEventListener.java:219)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.proxyOrLoad(DefaultLoadEventListener.java:278)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.doOnLoad(DefaultLoadEventListener.java:121)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.onLoad(DefaultLoadEventListener.java:89)
> at org.hibernate.internal.SessionImpl.fireLoad(SessionImpl.java:1239)
> at
> org.hibernate.internal.SessionImpl.internalLoad(SessionImpl.java:1122)
> at
> org.hibernate.type.EntityType.resolveIdentifier(EntityType.java:672)
> at org.hibernate.type.EntityType.resolve(EntityType.java:457) at
> org.hibernate.engine.internal.TwoPhaseLoad.doInitializeEntity(TwoPhaseLoad.java:165)
> at
> org.hibernate.engine.internal.TwoPhaseLoad.initializeEntity(TwoPhaseLoad.java:125)
> at
> org.hibernate.loader.plan.exec.process.internal.AbstractRowReader.performTwoPhaseLoad(AbstractRowReader.java:238)
> at
> org.hibernate.loader.plan.exec.process.internal.AbstractRowReader.finishUp(AbstractRowReader.java:209)
> at
> org.hibernate.loader.plan.exec.process.internal.ResultSetProcessorImpl.extractResults(ResultSetProcessorImpl.java:133)
> at
> org.hibernate.loader.plan.exec.internal.AbstractLoadPlanBasedLoader.executeLoad(AbstractLoadPlanBasedLoader.java:122)
> at
> org.hibernate.loader.plan.exec.internal.AbstractLoadPlanBasedLoader.executeLoad(AbstractLoadPlanBasedLoader.java:86)
> at
> org.hibernate.loader.entity.plan.AbstractLoadPlanBasedEntityLoader.load(AbstractLoadPlanBasedEntityLoader.java:167)
> at
> org.hibernate.persister.entity.AbstractEntityPersister.load(AbstractEntityPersister.java:4087)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.loadFromDatasource(DefaultLoadEventListener.java:508)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.doLoad(DefaultLoadEventListener.java:478)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.load(DefaultLoadEventListener.java:219)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.doOnLoad(DefaultLoadEventListener.java:116)
> at
> org.hibernate.event.internal.DefaultLoadEventListener.onLoad(DefaultLoadEventListener.java:89)
> at org.hibernate.internal.SessionImpl.fireLoad(SessionImpl.java:1239)
> at
> org.hibernate.internal.SessionImpl.immediateLoad(SessionImpl.java:1097)
> ...

The hashCode function of ContainsJPA throws this exception when getting the id for the product. Why is it doing that, considering that the 'contains' table in the DB has this id ?
I can't figure out why this is happening. Please help.

Your hashCode and equals implementations are incorrect.

The problems with it, in a nutshell:

• They do not adhere to the 'delegation' style (they do not delegate the job of determining equality to the relevant classes)
• They do not answer the central question of what the object represents: Row in a DB, or the notion that the row in the DB is trying to represent.

Delegate equality checks

Both hashCode and equals are specced to require that you do not throw NPEs out of them. For equals, that means you can't just call, say, a.equals(b) - you'd have to make that a == null ? b == null : a.equals(b) (and because this 'never throw' is transitive, a.equals(b) is fine, even if b is null), or use the helper Objects.equal(a, b) instead.

For hashcode, it means that null values must be defined as having some predefined value for the sake of hashing. Also, more generally, whenever you have a 'sub object' (e.g. a field of some non-primitive type', the general idea is for hashCode and equals to cascade: Use productJPA.hashCode() and not productJPA.getId().

Same goes for equals. Don't do this:

(ProductHasHerbJPA)o).getHerbJpa().getId()==herbJpa.getId()

but do this:

Objects.equals(o.getHerbJpa(), herbJpa);

And if 2 herb JPAs are to be considered equal if their IDs are equal, then the HerbJPA class's equals() method should be defined accordingly, and if not, then not. It is not the job of your ContainsJPA class to know how to calculate if 2 herbJPA instances are equal - herbJPA can do that, itself. In passing you avoid a ton of null issues by doing it this way.

Note, you can let lombok take care of all this boilerplate for you.

Next, we get to some hairy issues with JPA and equality in particular.

The common strategy to do equals/hashCode in the java ecosystem (outside of JPA/Hibernate) is to look at all fields that are part of an object's identity, which is usually all of them. The problem is, that doesn't work well with JPA: Most of the getter methods on a JPA object are proxies which cause DB queries if you invoke them. With a sufficiently interconnected db structure (lots of references), that means a single equals call ends up querying half your DB, takes a ton of memory, and half an hour to complete, obviously not a feasible solution.

The key question is: What does your object actually represent, and as far as I know, JPA does not give clear guidance.

An instance of HerbsJPA represents a row in a database

Then we can draw the following conclusions:

• As always, by spec, an object is always equal to itself: if (this == other) return true;. Otherwise...
• If either or both of the objects have no set unid, then they cannot be equal to each other - 2 unwritten rows, even if entirely identical for every field in the object, still does not represent 'the same row', therefore, not equal!
• If both objects have a set unid, then they are equal if the unids are, and otherwise, they are not. Regardless of all the other values! - 2 different rows with identical values are... still two different rows.

This view incidentally is also convenient in that you entirely avoid that 'whoops it queries the entire DB' issue. unids are not expensive to fetch, and are usually prefetched already.

An instance of HerbsJPA represents a 'herb'.

If this is the case, may I suggest your class is misnamed? It should be 'Herb', probably. Maybe 'HerbJpa' (NB: JPA in all-caps is a violation of the most common style rule).

Then the most sensical solution is to AVOID checking the unid entirely, and look only at all the other fields (or at least, all the other ones that represent something about the herb's identity. This is usually most of them, but sometimes you can get away with defining some property that would cause a storm of DB queries, such as 'a list of associated herbs', represented in the DB with a join table, as 'not part of the identity'. After all, 'the unid in the db' is an incidental implementation detail of the notion of a 'herb' and therefore couldn't possibly be part of the identity of it!

The downside of this view is of course that 'storm of DB calls' issue.

Generally I advise you treat these objects as representing 'row in a table' and not 'the actual herb', in which case, your equals and hashCode methods become relatively simple, and the name of the class is fine (well, it should be 'Jpa', not 'JPA', but other than that).

@Override public int hashCode() {
    return id == null ? super.hashCode() : (int) id;
    // note, other answer's id %1000 is silly;
    // it is needlessly inefficient, don't do it that way.
}

@Override public boolean equals(Object other) {
    if (other == this) return true;
    if (other == null || other.getClass() != ContainsJPA.class) return false;
    return id == null ? false : id.equals(other.id);
}

Not 100% sure, but doesn't AbstractRowReader first load the collection and then "hydrate" the associated entities?

AbstractRowReader#finishUp()

  ...
  // now we can finalize loading collections
  finishLoadingCollections( context );

  // finally, perform post-load operations
  postLoad( postLoadEvent, context, hydratedEntityRegistrations, afterLoadActionList 
);

Which means when creating the collection, the product_id is known, but the ProductJPA instance hasn't been hydrated yet.

tbh, I think it's not great practice to derive a hashcode from associated entities. I'd probably do something like

public class ContainsJPA {
  @Id
  private Long id;

  @Override
  public int hashCode(){
    return id == null ? super.hashCode() : id % 1000;
  }

to get some distribution (the '1000' is a magic number, depending on what typical collection sizes are).