Skip to content

Commit

Permalink
Separeted UnitOfWork and EntityManager
Browse files Browse the repository at this point in the history 
This modification continues work of Andreas Viedma
(andresviedma:unitofwork-annotation).

Current solution of the EntityManager doesn't robustly support use of
Transaction scoped EntityManager.If EntityManager is injected or
emProvider.get() is called outside of the Transactional annotation,
then this will lead to never closed EntityManager. (Current solution
would need boilerplate UnitOfWork.end() calls). This behaviour is also
different than in Guice 2.0 with warp-persist, where Transaction scoped
EntityManager works.

With this commit UnitOfWork and EntityManager are separated.
This means that getting of EntityManager doesn't start UnitOfWork.
If EntityManager scope needs to be different than transaction,
UnitOfWork has to be explicitly started.UnitOfWork can be started
with annotation or explicitly calling begin(). Annotation supports
nested UnitOfWorks. If begin is called directly from UnitOfWork then
it requires direct call to end() to end UnitOfWork.End call will always
end UnitOfWork.

Resolves issues: google#739, google#947
  • Loading branch information
@jjantunen
jjantunen committed Jan 27, 2016
1 parent 1bc3ee0 commit 96f6c93
Show file tree
Hide file tree
Showing 12 changed files with 516 additions and 297 deletions.
77 changes: 42 additions & 35 deletions extensions/persist/src/com/google/inject/persist/jpa/JpaLocalTxnInterceptor.java
View file
Original file line number Diff line number Diff line change
@@ -1,57 +1,62 @@
/**
* Copyright (C) 2010 Google, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/

package com.google.inject.persist.jpa;

import com.google.inject.Inject;
import com.google.inject.persist.Transactional;

import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;

import java.lang.reflect.Method;

import javax.persistence.EntityManager;
import javax.persistence.EntityTransaction;

import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;

import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.persist.Transactional;

/**
* @author Dhanji R. Prasanna (dhanji@gmail.com)
*/
class JpaLocalTxnInterceptor implements MethodInterceptor {

@Transactional
private static class Internal {}

private static class Internal {
}

// TODO(gak): Move this arg to the cxtor & make this final.
@Inject
private UnitOfWorkHandler unitOfWorkHandler;

private Provider<EntityManager> emProvider;

@Inject
private UnitOfWorkService unitOfWork;

@Override
public Object invoke(MethodInvocation methodInvocation) throws Throwable {

unitOfWorkHandler.requireUnitOfWork();
unitOfWork.requireUnitOfWork();

Transactional transactional = readTransactionMetadata(methodInvocation);
EntityManager em = unitOfWorkHandler.getEntityManager();
EntityManager em = emProvider.get();

// Allow 'joining' of transactions if there is an enclosing @Transactional method.
// Allow 'joining' of transactions if there is an enclosing
// @Transactional method.
if (em.getTransaction().isActive()) {
try {
return methodInvocation.proceed();
} finally {
unitOfWorkHandler.endRequireUnitOfWork();
unitOfWork.endRequireUnitOfWork();
}
}

Expand All @@ -63,28 +68,29 @@ public Object invoke(MethodInvocation methodInvocation) throws Throwable {
result = methodInvocation.proceed();

} catch (Exception e) {
//commit transaction only if rollback didnt occur
// commit transaction only if rollback didnt occur
if (rollbackIfNecessary(transactional, e, txn)) {
txn.commit();
}

//propagate whatever exception is thrown anyway
// propagate whatever exception is thrown anyway
throw e;
} finally {
if (!txn.isActive()) {
unitOfWorkHandler.endRequireUnitOfWork();
unitOfWork.endRequireUnitOfWork();
}
}

//everything was normal so commit the txn (do not move into try block above as it
// interferes with the advised method's throwing semantics)
// everything was normal so commit the txn (do not move into try block
// above as it
// interferes with the advised method's throwing semantics)
try {
txn.commit();
} finally {
unitOfWorkHandler.endRequireUnitOfWork();
unitOfWork.endRequireUnitOfWork();
}

//or return result
// or return result
return result;
}

Expand Down Expand Up @@ -118,28 +124,29 @@ private boolean rollbackIfNecessary(Transactional transactional, Exception e,
EntityTransaction txn) {
boolean commit = true;

//check rollback clauses
// check rollback clauses
for (Class<? extends Exception> rollBackOn : transactional.rollbackOn()) {

//if one matched, try to perform a rollback
// if one matched, try to perform a rollback
if (rollBackOn.isInstance(e)) {
commit = false;

//check ignore clauses (supercedes rollback clause)
// check ignore clauses (supercedes rollback clause)
for (Class<? extends Exception> exceptOn : transactional.ignore()) {
//An exception to the rollback clause was found, DON'T rollback
// An exception to the rollback clause was found, DON'T
// rollback
// (i.e. commit and throw anyway)
if (exceptOn.isInstance(e)) {
commit = true;
break;
}
}

//rollback only if nothing matched the ignore check
// rollback only if nothing matched the ignore check
if (!commit) {
txn.rollback();
}
//otherwise continue to commit
// otherwise continue to commit

break;
}
Expand Down
89 changes: 49 additions & 40 deletions extensions/persist/src/com/google/inject/persist/jpa/JpaPersistModule.java
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,32 @@
/**
* Copyright (C) 2010 Google, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/

package com.google.inject.persist.jpa;

import java.lang.reflect.AccessibleObject;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.List;
import java.util.Map;

import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;

import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;

import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.inject.Inject;
Expand All @@ -27,19 +38,6 @@
import com.google.inject.persist.finder.DynamicFinder;
import com.google.inject.persist.finder.Finder;

import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;

import java.lang.reflect.AccessibleObject;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.List;
import java.util.Map;

import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;

/**
* JPA provider for guice persist.
*
Expand All @@ -54,25 +52,26 @@ public JpaPersistModule(String jpaUnit) {
this.jpaUnit = jpaUnit;
}

private Map<?,?> properties;
private Map<?, ?> properties;
private MethodInterceptor transactionInterceptor;
private MethodInterceptor requiresUnitOfWorkInterceptor;

@Override protected void configurePersistence() {
@Override
protected void configurePersistence() {
bindConstant().annotatedWith(Jpa.class).to(jpaUnit);

bind(JpaPersistService.class).in(Singleton.class);
bind(UnitOfWorkService.class).in(Singleton.class);

bind(PersistService.class).to(JpaPersistService.class);
bind(UnitOfWork.class).to(JpaPersistService.class);
bind(UnitOfWork.class).to(UnitOfWorkService.class);
bind(EntityManager.class).toProvider(JpaPersistService.class);
bind(EntityManagerFactory.class)
.toProvider(JpaPersistService.EntityManagerFactoryProvider.class);
bind(UnitOfWorkHandler.class);

transactionInterceptor = new JpaLocalTxnInterceptor();
requestInjection(transactionInterceptor);

requiresUnitOfWorkInterceptor = new RequiresUnitOfWorkInterceptor();
requestInjection(requiresUnitOfWorkInterceptor);

Expand All @@ -82,26 +81,30 @@ public JpaPersistModule(String jpaUnit) {
}
}

@Override protected MethodInterceptor getTransactionInterceptor() {
@Override
protected MethodInterceptor getTransactionInterceptor() {
return transactionInterceptor;
}

@Override protected MethodInterceptor getRequiresUnitOfWorkInterceptor() {
@Override
protected MethodInterceptor getRequiresUnitOfWorkInterceptor() {
return requiresUnitOfWorkInterceptor;
}

@Provides @Jpa Map<?, ?> provideProperties() {
@Provides
@Jpa
Map<?, ?> provideProperties() {
return properties;
}

/**
* Configures the JPA persistence provider with a set of properties.
*
* @param properties A set of name value pairs that configure a JPA persistence
* provider as per the specification.
* @param properties A set of name value pairs that configure a JPA persistence provider as per
* the specification.
* @since 4.0 (since 3.0 with a parameter type of {@code java.util.Properties})
*/
public JpaPersistModule properties(Map<?,?> properties) {
public JpaPersistModule properties(Map<?, ?> properties) {
this.properties = properties;
return this;
}
Expand All @@ -124,35 +127,42 @@ private <T> void bindFinder(Class<T> iface) {
}

InvocationHandler finderInvoker = new InvocationHandler() {
@Inject JpaFinderProxy finderProxy;
@Inject
JpaFinderProxy finderProxy;

@Override
public Object invoke(final Object thisObject, final Method method, final Object[] args)
throws Throwable {

// Don't intercept non-finder methods like equals and hashcode.
if (!method.isAnnotationPresent(Finder.class)) {
// NOTE(dhanji): This is not ideal, we are using the invocation handler's equals
// and hashcode as a proxy (!) for the proxy's equals and hashcode.
// and hashcode as a proxy (!) for the proxy's equals and hashcode.
return method.invoke(this, args);
}

return finderProxy.invoke(new MethodInvocation() {
@Override
public Method getMethod() {
return method;
}

@Override
public Object[] getArguments() {
return null == args ? new Object[0] : args;
return null == args ? new Object[0] : args;
}

@Override
public Object proceed() throws Throwable {
return method.invoke(thisObject, args);
}

@Override
public Object getThis() {
throw new UnsupportedOperationException("Bottomless proxies don't expose a this.");
}

@Override
public AccessibleObject getStaticPart() {
throw new UnsupportedOperationException();
}
Expand All @@ -162,9 +172,8 @@ public AccessibleObject getStaticPart() {
requestInjection(finderInvoker);

@SuppressWarnings("unchecked") // Proxy must produce instance of type given.
T proxy = (T) Proxy
.newProxyInstance(Thread.currentThread().getContextClassLoader(), new Class<?>[] { iface },
finderInvoker);
T proxy = (T) Proxy.newProxyInstance(Thread.currentThread().getContextClassLoader(),
new Class<?>[] {iface}, finderInvoker);

bind(iface).toInstance(proxy);
}
Expand All @@ -179,8 +188,8 @@ private boolean isDynamicFinderValid(Class<?> iface) {
for (Method method : iface.getMethods()) {
DynamicFinder finder = DynamicFinder.from(method);
if (null == finder) {
addError("Dynamic Finder methods must be annotated with @Finder, but " + iface
+ "." + method.getName() + " was not");
addError("Dynamic Finder methods must be annotated with @Finder, but " + iface + "."
+ method.getName() + " was not");
valid = false;
}
}
Expand Down
Loading

0 comments on commit 96f6c93

Please sign in to comment.