EVALSYS-1611 Rewrite DAO layer to remove genericdao

[ottenhoff]

This is meant to come after the thymeleaf rewrite

[danielmerino]

Overall this is a solid cleanup — removing genericdao eliminates a significant external dependency and replaces its proprietary Search/Restriction API and deprecated Hibernate Criteria classes (DetachedCriteria, Expression, Restrictions.disjunction()) with plain HQL queries. The reorganization of EvaluationDaoImpl from a 2000-line monolith into a chain of focused abstract classes is a clear improvement.

One minor observation:

transactionManager null-check in EvalDaoInvokerImpl: The guard if (transactionManager == null) { toInvoke.run(); } is handy for unit tests but silently runs outside a transaction in production if the bean is misconfigured. A log.warn there would make it easier to catch wiring mistakes early.

[ottenhoff]

Okay done. Did you have a chance to test? We need to remove genericdao to work on Sakai 26.x / master

[danielmerino]

Hi @ottenhoff I have tested your changes in Sakai 25. I paste the report.

1. HierarchyNode API incompatibility — ExternalHierarchyLogicImpl.java

PR #180 targets Sakai 26, where HierarchyNode was refactored to use getters/setters and HierarchyService gained new bulk methods. Sakai 25 still has the original design with public fields and node IDs pre-populated on the node object itself.

Affected code after applying PR #180 on Sakai 25

Location	PR #180 (Sakai 26)	Sakai 25 equivalent
createHierarchyRoot()	root.getId().toString()	root.id
makeEvalNode()	node.getId(), node.getTitle(), node.getDescription(), hierarchyService.getDirectChildNodeIds(String[]), .getChildNodeIds(), .getDirectParentNodeIds(), .getParentNodeIds()	node.id, node.title, node.description, node.directChildNodeIds, node.childNodeIds, node.directParentNodeIds, node.parentNodeIds
makeHierarchyNode()	node.setId(Long), node.setTitle(), node.setDescription(), node.setChildren(Set<HierarchyNode>), node.setParents(Set<HierarchyNode>)	direct public field assignment
helpers getNodeIds(), getHierarchyNodesByIds()	introduced by PR #180 to fetch child/parent sets via hierarchyService	not needed — sets are already on the node

Fix (patch parche_compat_sakai25_pr180.patch)

• Remove import java.util.Collections (only used by the removed helpers).
• In createHierarchyRoot: root.getId().toString() → root.id.
• Rewrite makeEvalNode to copy public fields directly from the node instead of calling hierarchyService.getDirectChildNodeIds() etc.
• Rewrite makeHierarchyNode to assign public fields directly instead of using setters/setChildren/setParents.
• Remove helpers getNodeIds(Map, String) and getHierarchyNodesByIds(Set<String>).

---

2. Hibernate proxy bug when deleting an evaluation — EvaluationDaoHibernateSupport.java

Symptom

Deleting an evaluation throws:

org.hibernate.MappingException: Unknown entity:
    org.sakaiproject.evaluation.model.EvalTemplate$HibernateProxy$1xVfnzJo
  at EvaluationDaoHibernateSupport.getPersistentId(EvaluationDaoHibernateSupport.java:187)
  at EvaluationDaoHibernateSupport.delete(EvaluationDaoHibernateSupport.java:169)

Root cause

delete(Object) calls getPersistentId(object), which does:

ClassMetadata metadata = getSessionFactory().getClassMetadata(object.getClass());

When the object was loaded lazily, object.getClass() returns the Hibernate proxy subclass (EvalTemplate$HibernateProxy$...), not EvalTemplate.class. SessionFactory.getClassMetadata() only knows the real entity class, so it returns null, which the next line converts into an IllegalArgumentException — but in practice Hibernate throws MappingException before reaching that check.

The same bug is present in delete(Object) at the branch delete(object.getClass(), id).

Fix (patch parche_hibernate_proxy_delete.patch)

// Add import:
import org.hibernate.Hibernate;

// getPersistentId() — line ~187:
- ClassMetadata metadata = getSessionFactory().getClassMetadata(object.getClass());
+ ClassMetadata metadata = getSessionFactory().getClassMetadata(Hibernate.getClass(object));

// delete(Object) — line ~173:
- delete(object.getClass(), id);
+ delete(Hibernate.getClass(object), id);

Hibernate.getClass(object) unwraps the proxy and returns the real entity class, which SessionFactory.getClassMetadata() can resolve correctly.

This bug is not Sakai-25-specific — it will affect any Sakai version where a lazy proxy reaches delete(). It is reproducible by deleting any evaluation whose template was fetched lazily.

[ottenhoff]

Well Sakai 25 and Sakai 26 are quite different now. I don't think this is necessary on Sakai 25 as genericdao still exists there. This is only necessary on Sakai 26 where genericdao is removed