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

Support `@UnitOfWork` outside of Jersey resources #1361

Merged
merged 2 commits into from Jan 12, 2016

Conversation

Projects
None yet
7 participants
@arteam
Member

arteam commented Nov 27, 2015

This PR provides the ability to use the @UnitOfWork annotation outside of Jersey resources.
It consists from two changes:

  • In first we move operations over @UnitOfWork methods from UnitOfWorkApplicationListener into an aspect. It allows to perform these operations outside of the listener (for example, in proxies or AOP providers).
  • In second we provide a factory for creating @UnitOfWork aware proxies. A created proxy will
    detect the annotation on component methods and will open/close a Hibernate session around them.
    It uses aforementioned aspect for this. Javassist is used for creating proxies, because it has a
    simple API for that and it's shipped with Hibernate.

Now to use @UnitOfWork in authenticators, one need just to create proxy like this:

SessionDao dao = new SessionDao(hibernateBundle.getSessionFactory());
ExampleAuthenticator exampleAuthenticator = 
                new UnitOfWorkAwareProxyFactory(hibernateBundle)
                .create(ExampleAuthenticator.class, SessionDao.class, dao);

This change is intended to 0.9.* branch. Reference issues: #1334 , #1240

@jplock

This comment has been minimized.

Member

jplock commented Dec 7, 2015

Personally I don't have any way to test this change even though there's been a lot of chatter about it. Does anyone else have a way to validate this and/or merge it in?

@jplock jplock added the improvement label Dec 7, 2015

arteam added some commits Nov 26, 2015

Generalize operations over `@unitofwork` methods
Extract operations that should be performed on a `@unitofwork` method
in an aspect. The aspect provides a public API for 3 operations:

* Open session/open transaction before a method invocation;
* Close session/commit transaction after the method invocation;
* Close session/rollback transaction in case of an error.

It will allow to create proxies/AOP solutions for handling the
`@unitofwork` annotation outside of Jersey resources.
Add `@unitofwork` aware proxy factory
Add a factory for creating proxies for components with the
`@unitofwork` annotations on methods. This will allow to
users to use Hibernate data access objects with declarative
transactions outside Jersey resources.

A new created proxy will:

* detect the annotation on a component method;
* open a transaction before starting the method;
* commit the transcation after ending the method;
* rollback tre transaction in a case of a faulure;

Javassist is used for the proxy generation.

Resolves #1334, #1240
@arteam

This comment has been minimized.

Member

arteam commented Dec 9, 2015

A simple way to test this is to try to use @UnitOfWork with Hibernate sessions in authenticators. Authentification is now performed outside of Jersey lifecycle for security reasons, so the process should fail without a proxy.

@jplock

This comment has been minimized.

Member

jplock commented Jan 7, 2016

@harshil07, @paukiatwee, @dotCipher, @yiweig, @vvondra can anyone confirm if this PR resolves your issue?

@harshil07

This comment has been minimized.

harshil07 commented Jan 11, 2016

Happy to test @jplock but I'm not entirely sure on how to build the module for java 8.

@arteam

This comment has been minimized.

Member

arteam commented Jan 11, 2016

This PR is against the 0.9 branch, which still uses Java 7. Something like this should work, though.

git checkout -b arteam-generalize_unit_of_work release/0.9.x
git pull https://github.com/arteam/dropwizard.git generalize_unit_of_work
cd ./dropwizard-bom
mvn install
cd ../
mvn install -DskipTests=true -P dev
@vvondra

This comment has been minimized.

Contributor

vvondra commented Jan 12, 2016

@jplock In the end, since I use dropwizard-guice, I went with a simple proxy using https://github.com/google/guice/wiki/AOP or I actually inject a TransactionProvider class which has a method I can pass a callback to which will be executed transactionally

@paukiatwee

This comment has been minimized.

paukiatwee commented Jan 12, 2016

I tested this PR and it is working! Passed all my tests. My app at Github.

Set up

TokenAuthenticator tokenAuthenticator = new UnitOfWorkAwareProxyFactory(hibernate).create(TokenAuthenticator.class, FinanceService.class, financeService);
// auth
final OAuthCredentialAuthFilter<User> authFilter =
        new OAuthCredentialAuthFilter.Builder<User>()
                .setAuthenticator(tokenAuthenticator)
                .setPrefix("Bearer")
                .setAuthorizer(new DefaultAuthorizer())
                .setUnauthorizedHandler(new DefaultUnauthorizedHandler())
                .buildAuthFilter();

Authenticator

public class TokenAuthenticator implements Authenticator<String, User> {

    private final FinanceService financeService;

    public TokenAuthenticator(FinanceService financeService) {
        this.financeService = financeService;
    }

    @UnitOfWork
    @Override
    public Optional<User> authenticate(String token) throws AuthenticationException {
        java.util.Optional<User> option = financeService.findUserByToken(token);
        if(option.isPresent()) {
            return Optional.of(option.get());
        } else {
            return Optional.absent();
        }
    }
}

Previously financeService.findUserByToken(token) will throw error.

@jplock

This comment has been minimized.

Member

jplock commented Jan 12, 2016

Excellent! Thank you for confirming @paukiatwee.

jplock added a commit that referenced this pull request Jan 12, 2016

@jplock jplock merged commit 0f258b0 into dropwizard:release/0.9.x Jan 12, 2016

@jplock

This comment has been minimized.

Member

jplock commented Jan 12, 2016

@arteam are you going to cherry pick this into master as well for 1.0.0?

@arteam

This comment has been minimized.

Member

arteam commented Jan 12, 2016

Yes, will do.

@arteam

This comment has been minimized.

Member

arteam commented Jan 12, 2016

Commits 39a857d and 0a5534e are cherry-picked to the master branch.

@arteam arteam added this to the 0.9.2 milestone Jan 12, 2016

@arteam

This comment has been minimized.

Member

arteam commented Jan 12, 2016

@dropwizard/committers, what do you think about releasing 0.9.2 with this workaround?

@jplock

This comment has been minimized.

Member

jplock commented Jan 12, 2016

+1 from me

@shashank261

This comment has been minimized.

shashank261 commented Jan 12, 2016

+1

Thanks & Regards
Shashank Harwalkar

On Tue, Jan 12, 2016 at 10:19 PM, Justin Plock notifications@github.com
wrote:

+1 from me


Reply to this email directly or view it on GitHub
#1361 (comment)
.

@kkamkou

This comment has been minimized.

kkamkou commented Jan 13, 2016

+1

@arteam arteam deleted the arteam:generalize_unit_of_work branch Jan 24, 2016

arteam added a commit that referenced this pull request Jun 15, 2016

Document the using of @unitofwork annotation outside Jersey resources
Add a section in the docs about how to use declarative transactions in authenticators.
It's not really obvious and many users expect they will work out of box.

See #1334, #1361.

arteam added a commit that referenced this pull request Jun 15, 2016

Document the using of @unitofwork annotation outside Jersey resources
Add a section in the docs about how to use declarative transactions in authenticators.
It's not really obvious and many users expect they will work out of box.

See #1334, #1361.

[ciskip]

arteam added a commit that referenced this pull request Jun 15, 2016

Document using @unitofwork outside Jersey resources
Add a section in the docs about how to use declarative transactions in authenticators.
It's not really obvious and many users expect they will work out of box.

See #1334, #1361.

[ciskip]

arteam added a commit that referenced this pull request Jun 15, 2016

Document using @unitofwork outside Jersey resources
Add a section in the docs about how to use declarative transactions in authenticators.
It's not really obvious and many users expect they will work out of box.

See #1334, #1361.

[ci skip]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment