Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DAOs should be proxyable #4854

Closed
hwellmann opened this issue Dec 27, 2015 · 6 comments
Closed

DAOs should be proxyable #4854

hwellmann opened this issue Dec 27, 2015 · 6 comments

Comments

@hwellmann
Copy link

Generated DAOs don't integrate very well with CDI in a Java EE context due to a lot of final methods in DAOImpl. Classes with final methods are unproxyable and cannot be used as normal scoped CDI beans.

This prevents use cases like

    @ApplicationScoped
    public class DaoProducers {

        @Inject
        private Configuration config;

        @javax.transaction.Transactional
        @ApplicationScoped
        public CountryDao countryDao() {
            return new CountryDao(config);
        }
    }
@lukaseder
Copy link
Member

Thank you very much for your suggestion. Indeed, we're aware of this limitation caused by some final keywords in DAOImpl, and we're hoping to fix this soon as #4696. However, we probably won't remove all final keywords as final usage is generally an important part in jOOQ's design strategy. Would you mind explaining how your particular use-case is prevented by final?

Also: I'm curious about the @Transactional annotation on your countryDao() constructor. What's the rationale behind this?

@lukaseder
Copy link
Member

Would you mind explaining how your particular use-case is prevented by final?

OK, I suspect you really want to inject the DAO, rather than do what you did in your example. The most sensible approach here would be to generate interfaces for each DAO. Does that match your expectations?

@hwellmann
Copy link
Author

Speaking from the CDI perspective, when you inject a bean, most of the time you don't get an instance of the bean class itself but an instance of a client proxy. When the bean class is a real class and not just an interface, the client proxy has a class generated by byte code manipulation. The proxy class extends the real bean class. Methods of the proxy class take care of any interceptors and decorators, then select the correct target instance of the real bean class and delegate method invocations to that target instance. If the target bean class has any final methods, the proxy class cannot be generated, since it needs to override any non-private method.

So the plan of #4696 to remove final from some methods would not solve this issue. It's all or nothing, I'm afraid.

Most likely, this issue is not specific to CDI. Other solutions working with client proxies may run into the same problem. I think Spring has a similar approach of generating target proxies via CGLIB.

Regarding @Transactional, working with a Java EE 7 DataSource, any modifying operation requires a transaction, as auto-commit is turned off. So CountryDao.insert() will cause an exception unless it is called from a transactional method.

In a classical layered architecture, you would have a CountryController in the web layer calling a @Stateless session bean CountryService (which is transactional by default) which then calls CountryDao.

However, in simple cases, you'll find that all CountryService methods do nothing but delegate to CountryDao methods, and in that case, you don't need the CountryService EJB at all, you can simply add the @Ŧransactional interceptor binding to the CountryDao class to make it transactional (provided that CountryDao is valid CDI bean class).

The last two paragraphs would apply to any hand-written classes. Now for the CountryDao generated by jOOQ, it needs to be populated with a Configuration, and I can't set the @Transactional annotation directly on the class, so for each of these two reasons, I need a producer method (not a constructor).

In other words, what I did in my example is the necessary prerequisite (but not sufficient, see below) to inject the DAO with CDI.

For CDI, it would be really convenient, if the code generator could be configured to generate classes like

@ApplicationScoped
@Transactional
public class CountryDao extends DaoImpl<Country> {

    @Inject @SomeQualifier
    private Configuration config;

    // methods
}

where the Configuration bean with the appropriate qualifier is provided by the application (probably by a producer method). Both the qualifier and the scope annotation should be configurable. Then there would be no need for a DAO producer method.

In fact, my producer example was wrong. I've checked the CDI spec:

The set of interceptor bindings for a producer method is not used to associate interceptors with the return value of the producer method.

So the transactional interceptor binding has no effect on the producer method, and that means that a DAO interface ICountryDao would not help either, since there is no way to bind the transactional interceptor to the class implementing that interface.

@lukaseder
Copy link
Member

Thank you very much for your additional info.

jOOQ's DAOs are among the "hottest" topics. The concrete implementation has often been criticised, people have been confused about how to use them "correctly", and they have not been designed to be used with Spring's @Autowired or with CDI. So, the improvement that should be implemented should be bigger than just making them proxyable.

Perhaps.

On the other hand, the exact semantics of each annotation is something that people will always want to fine-tune. For instance, your suggested @Transactional or @ApplicationScoped annotations make a lot of assumptions about the user's architecture / design. They may again not be what someone else wants DAOs to be.

I keep wondering if we shouldn't simply deprecate DAOs as they are today, and move forward with the improvements of the code generator, such that users can generate their own, very specific DAO style (they can already do this today, but we don't encourage extending the code generator, yet). In the end, we wouldn't need to discuss final, which is probably not the main topic / criticism here.

In any case, thanks again for your suggestion. We'll think about this. I cannot promise any quick win, as I doubt there is one. But I am sure that there is a thorough, strategic solution that jOOQ can offer for Spring and/or CDI.

@lukaseder
Copy link
Member

For the record, here's the discussion I've started on the user group regarding future strategic code generation improvements:
https://groups.google.com/forum/#!topic/jooq-user/aZ0CJwUfdkM

@lukaseder
Copy link
Member

I have given this some additional thought. While I believe that a more fundamental problem is present here, in the context of which DAO might even be removed for something better, I do agree that among all jOOQ types, DAO should be proxyable for extension, and the final keyword usage that has served us well in many other cases is a mistake here.

I will thus implement #4696 as "removing all usages of final" and close this as a duplicate of #4696

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants