Skip to content

Commit

Permalink
Revert back all the 'autoCommit' stuff. It can't work properly as it …
Browse files Browse the repository at this point in the history
…conflicts with the JPA specification.
  • Loading branch information
guillaumebort committed Apr 7, 2011
1 parent 98b9438 commit b1cdeb9
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 45 deletions.
7 changes: 7 additions & 0 deletions documentation/manual/releasenotes-1.2.textile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ Migrating from Play 1.1.x is pretty straightforward. There are no changes to the

Some APIs have been removed in Play 1.2, after having been deprecated for a long time, but most of the public API is the same. If you have compilation errors that you don’t know how to solve, ask on the "Google group":https://groups.google.com/forum/#!forum/play-framework.

When you call a controller from a @FunctionalTest@, the action invocation now runs in its own transaction. Because the test itself also run in its own transaction, deadlocks can occur. So it's better to configure your database with *READ_UNCOMMITED* transactions while running tests. If you test again the in-memory H2 database, use the following database configuration:

bc. %test.db.url=jdbc:h2:mem:play;MODE=MYSQL;LOCK_MODE=0@

If you can't use *READ_UNCOMMITED* for your database transactions, you need to run every JPA access it their own transaction, either by managing JPA transaction by hand (using @JPA.em().getTransaction()@) or by spawning invocations using *Jobs*.


h2. Dependency management

"Play’s dependency management system":dependency allows you to express your application’s external dependencies in a single **dependencies.yml** file.
Expand Down
23 changes: 11 additions & 12 deletions framework/src/play/db/DBPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -179,29 +179,21 @@ private static void check(Properties p, String mode, String property) {
private static boolean changed() {
Properties p = Play.configuration;

if ("mem".equals(p.getProperty("db"))) {
// If db.url or db.user or db.pass or db.driver is set but db=mem warn the user
check(p, "memory", "db.driver");
check(p, "memory", "db.url");

if ("mem".equals(p.getProperty("db")) && p.getProperty("db.url") == null) {
p.put("db.driver", "org.h2.Driver");
p.put("db.url", "jdbc:h2:mem:play;MODE=MYSQL;DB_CLOSE_ON_EXIT=FALSE");
p.put("db.url", "jdbc:h2:mem:play;MODE=MYSQL");
p.put("db.user", "sa");
p.put("db.pass", "");
}

if ("fs".equals(p.getProperty("db"))) {
// If db.url or db.user or db.pass or db.driver is set but db=fs warn the user
check(p, "fs", "db.driver");
check(p, "fs", "db.url");

if ("fs".equals(p.getProperty("db")) && p.getProperty("db.url") == null) {
p.put("db.driver", "org.h2.Driver");
p.put("db.url", "jdbc:h2:" + (new File(Play.applicationPath, "db/h2/play").getAbsolutePath()) + ";MODE=MYSQL");
p.put("db.user", "sa");
p.put("db.pass", "");
}

if (p.getProperty("db", "").startsWith("java:")) {
if (p.getProperty("db", "").startsWith("java:") && p.getProperty("db.url") == null) {
if (DB.datasource == null) {
return true;
}
Expand All @@ -227,9 +219,16 @@ private static boolean changed() {
}
}

if(p.getProperty("db.url") != null && p.getProperty("db.url").startsWith("jdbc:h2:mem:")) {
p.put("db.driver", "org.h2.Driver");
p.put("db.user", "sa");
p.put("db.pass", "");
}

if ((p.getProperty("db.driver") == null) || (p.getProperty("db.url") == null)) {
return false;
}

if (DB.datasource == null) {
return true;
} else {
Expand Down
11 changes: 1 addition & 10 deletions framework/src/play/db/jpa/JPA.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.EntityTransaction;
import play.db.DB;

import play.exceptions.JPAException;
import play.exceptions.UnexpectedException;

/**
* JPA Support
Expand All @@ -30,7 +28,7 @@ static void clearContext() {
local.remove();
}

static void createContext(EntityManager entityManager, boolean readonly, boolean autoCommit) {
static void createContext(EntityManager entityManager, boolean readonly) {
if (local.get() != null) {
try {
local.get().entityManager.close();
Expand All @@ -42,14 +40,7 @@ static void createContext(EntityManager entityManager, boolean readonly, boolean
JPA context = new JPA();
context.entityManager = entityManager;
context.readonly = readonly;
context.autoCommit = autoCommit;
local.set(context);

try {
DB.getConnection().setAutoCommit(autoCommit);
} catch (Exception e) {
throw new UnexpectedException(e);
}
}

// ~~~~~~~~~~~
Expand Down
11 changes: 4 additions & 7 deletions framework/src/play/db/jpa/JPAPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.sql.Connection;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -300,13 +301,11 @@ public void beforeInvocation() {
}

boolean readOnly = false;
boolean autoCommit = false;
Transactional tx = InvocationContext.current().getAnnotation(Transactional.class);
if (tx != null) {
readOnly = tx.readOnly();
autoCommit = tx.autoCommit();
}
startTx(readOnly, autoCommit);
startTx(readOnly);
}

@Override
Expand All @@ -330,19 +329,17 @@ public void invocationFinally() {
* @param readonly true for a readonly transaction
* @param autoCommit true to automatically commit the DB transaction after each JPA statement
*/
public static void startTx(boolean readonly, boolean autoCommit) {
public static void startTx(boolean readonly) {
if (!JPA.isEnabled()) {
return;
}
EntityManager manager = JPA.entityManagerFactory.createEntityManager();
//if(Play.configuration.getProperty("future.bindJPAObjects", "false").equals("true")) {
manager.setFlushMode(FlushModeType.COMMIT);
manager.setProperty("org.hibernate.readOnly", readonly);
//}
if (autoTxs) {
manager.getTransaction().begin();
}
JPA.createContext(manager, readonly, autoCommit);
JPA.createContext(manager, readonly);
}

/**
Expand Down
1 change: 0 additions & 1 deletion framework/src/play/db/jpa/Transactional.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,5 @@
@Target(value={ElementType.METHOD,ElementType.TYPE})
public @interface Transactional {
public boolean readOnly() default false;
public boolean autoCommit() default false;
}

7 changes: 6 additions & 1 deletion framework/src/play/test/Fixtures.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ public static void delete(Class<? extends Model>... types) {
idCache.clear();
disableForeignKeyConstraints();
for (Class<? extends Model> type : types) {
Model.Manager.factoryFor(type).deleteAll();
try {
Model.Manager.factoryFor(type).deleteAll();
} catch(Exception e) {
Logger.error(e, "While deleting " + type + " instances");
}

}
enableForeignKeyConstraints();
Play.pluginCollection.afterFixtureLoad();
Expand Down
2 changes: 0 additions & 2 deletions modules/testrunner/app/controllers/TestRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import play.Logger;
import play.Play;
import play.cache.Cache;
import play.db.jpa.Transactional;
import play.libs.IO;
import play.libs.Mail;
import play.mvc.*;
Expand Down Expand Up @@ -44,7 +43,6 @@ public static void list() {
renderText(list);
}

@Transactional(autoCommit=true)
public static void run(String test) throws Exception {
if (test.equals("init")) {
File testResults = Play.getFile("test-result");
Expand Down
4 changes: 2 additions & 2 deletions resources/application-skel/conf/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ mail.smtp=mock
# ~~~~~
#%test.module.cobertura=${play.path}/modules/cobertura
%test.application.mode=dev
%test.db=mem
%test.jpa.ddl=create-drop
%test.db.url=jdbc:h2:mem:play;MODE=MYSQL;LOCK_MODE=0
%test.jpa.ddl=create
%test.mail.smtp=mock

10 changes: 2 additions & 8 deletions samples-and-tests/just-test-cases/conf/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -125,21 +125,15 @@ play.editor=txmt://open?url=file://%s&line=%s
# Testing. Set up a custom configuration for test mode
# ~~~~~
%test.application.mode=dev
%test.db=mem
%test.jpa.ddl=create-drop
%test.db.url=jdbc:h2:mem:play;MODE=MYSQL;LOCK_MODE=0
%test.jpa.ddl=create
%test.mail.smtp=mock
%test.play.pool=2

# Date format for the application
date.format=yyyy-MM-dd
date.format.fr=dd-MM-yyyy

# These features will be automatically enabled in the 1.1 release
# For now you can enable them if you want
# ~~~~~
future.bindJPAObjects=true
future.escapeInTemplates=true

# For tests: defining a new mime type
mimtype.foobar=application/foobarstuff
# For tests: overriding a default mime type
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#{selenium 'Check invocation context resolution'}

open('@{Application.some1()}')
assertTextPresent('@play.db.jpa.Transactional(autoCommit=false, readOnly=false),')
assertTextPresent('@play.db.jpa.Transactional(readOnly=false),')

open('@{Application.some2()}')
assertTextPresent('@utils.Youhou(value=),@play.db.jpa.Transactional(autoCommit=false, readOnly=false),')
assertTextPresent('@utils.Youhou(value=),@play.db.jpa.Transactional(readOnly=false),')

open('@{Application.some3()}')
assertTextPresent('@utils.Youhou(value=fromJob),')
Expand Down

0 comments on commit b1cdeb9

Please sign in to comment.