Implementing a Generic DAO

I want to ask for opinions on this task given at the end of the course: Extract common DAO operations to an abstract class, e.g. HibernateDao<T>, then have each DAO implementation class extend this abstract class.

I managed to implement it, but it seems there are a few ways of doing it and not sure which way is considered the "best practice".

This is my github repo for this project, https://github.com/edison696/giflib-hibernate and the implementation is in the last commit at this time, which is this one https://github.com/edison696/giflib-hibernate/commit/546e119792814c3a03e675de96a2af4d06f02a7f

Now I realized that in order to add a Generic to my HibernateDao<T> abstract class and to be able to access the class T (for the criteria parameters), the convention is to have a field which stores the generic type class as it can't be accessed during runtime. And we set that field in a constructor, which I defined in each of the child Dao classes, namely GifDaoImpl and CategoryDaoImpl. I've also moved the session factory in the abstract class.

So, HibernateDao<T> looks like this:

public abstract class HibernateDao<T> {

    @Autowired
    SessionFactory sessionFactory;

    Class<T> typeParameterClass;

    public List<T> findAll() {
        Session session = sessionFactory.openSession();

        CriteriaBuilder builder = session.getCriteriaBuilder();
        CriteriaQuery<T> criteriaQuery = builder.createQuery(typeParameterClass);
        criteriaQuery.from(typeParameterClass);
        List<T> ts = session.createQuery(criteriaQuery).getResultList();

        session.close();
        return ts;
    }

    public void save(T t){
        Session session = sessionFactory.openSession();
        session.beginTransaction();
        session.saveOrUpdate(t);
        session.getTransaction().commit();
        session.close();
    }

    public void delete(T t) {
        Session session = sessionFactory.openSession();
        session.beginTransaction();
        session.delete(t);
        session.getTransaction().commit();
        session.close();
    }
}

I've added findAll, save and delete methods inside, since findById method has different implementations in GifDaoImpl and CategoryDaoImpl.

So, first question. Should I add findById in the abstract class as well, override it with the specific implementation in CategoryDaoImpl only and let GifDaoImpl use this "generic" version? Thinking about it, it seems a good idea to add it to the generic DAO class as additional entities we might add in the app will most likely use this generic version.

These are the two specific DAO classes after extending HibernateDao.

GifDaoImpl.java

@Repository
public class GifDaoImpl extends HibernateDao implements GifDao {

    public GifDaoImpl() {
        typeParameterClass = Gif.class;
    }

    @Override
    public Gif findById(Long id) {
        Session session = sessionFactory.openSession();
        Gif gif = session.get(Gif.class, id);
        session.close();
        return gif;
    }

    @Override
    public void save(Gif gif) {
        super.save(gif);
    }

    @Override
    public void delete(Gif gif) {
        super.delete(gif);
    }
}

CategoryDaoImpl.java

@Repository
public class CategoryDaoImpl extends HibernateDao implements CategoryDao {

    public CategoryDaoImpl() {
        typeParameterClass = Category.class;
    }

    @Override
    public Category findById(Long id) {
        Session session = sessionFactory.openSession();
        // don't be confused by the different first parameter of the session.get method, just testing the other version of the method here
        Category category = (Category) session.get("com.teamtreehouse.giflib.model.Category", id);
        Hibernate.initialize(category.getGifs());
        session.close();
        return category;
    }

    @Override
    public void save(Category category) {
        super.save(category);
    }

    @Override
    public void delete(Category category) {
        super.delete(category);
    }
}

I've added a no parameter constructor in each, which sets that typeParameterClass variable defined in the abstract class to be used in the findAll method. Would this be the best practice? I assumed this would work (and it does) because of that Spring mechanic (??) that calls the default constructor when instantiating an object of these classes.

Now, the trickiest decision in this implementation was whether should I mess up with each of the DAO interfaces or keep them as they are. Since the generic DAO class can't implement both DAO interfaces obviously, each DaoImpl class keeps implementing its respective interface.

What I actually decided to do is what's in the code above and in my current github repo. I kept each specific DAO interface intact, and I implemented each of the generic methods (save, delete) in their respective DaoImpl classes by simply calling the method defined in the abstract parent class using super (though this produced compiler warnings).

There's an option to change each interface and keep only the findAll method in each, so that each DaoImpl class would use the other methods from the abstract class without overriding them, but in that case, we should also mess up with the service layer (because it's calling respective interface methods), which I assume is not good. There's the option of making the specific DAO interfaces generic as well, which would also require updates in the service layer, but it seems to a much lesser extent. Like only changing the autowired variable GifDao gifDao to GifDao<Gif> gifDao (same for Category or any other entity).

In fact, now it does seem like a better approach than what I already did, and I already updated my repo with another commit :). This is the commit with these updates https://github.com/edison696/giflib-hibernate/commit/773e69e8cd0e75bdb18082ff27a6a5e849889fe5.

Can anyone share their thoughts or solutions for this task? Or any comments/suggestions regarding my implementation?

Thanks.