-
-
Notifications
You must be signed in to change notification settings - Fork 962
Fix/hibernate session integration tests 14920 #15002
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
base: 7.0.x
Are you sure you want to change the base?
Changes from all commits
9f421cf
461f4a9
a4f0e49
96837fc
f0adc35
52ed702
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ class GrailsTestInterceptor { | |
private String[] testClassSuffixes | ||
|
||
private GrailsTestTransactionInterceptor transactionInterceptor | ||
private GrailsTestSessionInterceptor sessionInterceptor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not compiling. |
||
private GrailsTestRequestEnvironmentInterceptor requestEnvironmentInterceptor | ||
|
||
GrailsTestInterceptor(Object test, GrailsTestMode mode, ApplicationContext appCtx, String[] testClassSuffixes) { | ||
|
@@ -41,12 +42,14 @@ class GrailsTestInterceptor { | |
|
||
void init() { | ||
autowireIfNecessary() | ||
initSessionIfNecessary() | ||
initTransactionIfNecessary() | ||
initRequestEnvironmentIfNecessary() | ||
} | ||
|
||
void destroy() { | ||
destroyTransactionIfNecessary() | ||
destroySessionIfNecessary() | ||
requestEnvironmentInterceptor?.destroy() | ||
} | ||
|
||
|
@@ -63,6 +66,25 @@ class GrailsTestInterceptor { | |
if (mode.autowire) createAutowirer().autowire(test) | ||
} | ||
|
||
protected initSessionIfNecessary() { | ||
// Check if we should bind sessions without transactions | ||
// This happens when: | ||
// 1. Not wrapping in transaction (to avoid conflict) | ||
// 2. Either mode.bindSession is true OR test has @WithSession annotation | ||
if (!mode.wrapInTransaction) { | ||
def localSessionInterceptor = createSessionInterceptor() | ||
if (mode.bindSession || localSessionInterceptor.shouldBindSessions(test)) { | ||
sessionInterceptor = localSessionInterceptor | ||
sessionInterceptor.init() | ||
} | ||
} | ||
} | ||
|
||
protected destroySessionIfNecessary() { | ||
sessionInterceptor?.destroy() | ||
sessionInterceptor = null | ||
} | ||
|
||
protected initTransactionIfNecessary() { | ||
def localTransactionInterceptor = createTransactionInterceptor() | ||
if (mode.wrapInTransaction && localTransactionInterceptor.isTransactional(test)) { | ||
|
@@ -101,6 +123,10 @@ class GrailsTestInterceptor { | |
new GrailsTestTransactionInterceptor(appCtx) | ||
} | ||
|
||
protected GrailsTestSessionInterceptor createSessionInterceptor() { | ||
new GrailsTestSessionInterceptor(appCtx) | ||
} | ||
|
||
protected GrailsTestRequestEnvironmentInterceptor createRequestEnvironmentInterceptor() { | ||
new GrailsTestRequestEnvironmentInterceptor(appCtx) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you 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 | ||
* | ||
* https://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. | ||
*/ | ||
package functional.tests | ||
|
||
import grails.testing.mixin.integration.Integration | ||
import grails.testing.mixin.integration.WithSession | ||
import grails.gorm.transactions.Rollback | ||
import org.hibernate.SessionFactory | ||
import spock.lang.Specification | ||
import spock.lang.Ignore | ||
import org.springframework.beans.factory.annotation.Autowired | ||
|
||
/** | ||
* Comparison tests showing different session/transaction binding behaviors. | ||
* This demonstrates the problem described in the issue and how @WithSession solves it. | ||
*/ | ||
@Integration | ||
class SessionBindingComparisonSpec extends Specification { | ||
|
||
@Autowired | ||
SessionFactory sessionFactory | ||
|
||
// Test 1: Without @Rollback and without @WithSession | ||
// This test will fail with "No HibernateSession bound to thread" | ||
@Ignore("Expected to fail - demonstrates the problem") | ||
void "test without rollback and without session binding fails"() { | ||
given: | ||
static transactional = false | ||
|
||
when: "Try to perform a SELECT operation" | ||
TestComparisonDomain.count() | ||
|
||
then: "Fails with No HibernateSession bound to thread" | ||
thrown(IllegalStateException) | ||
} | ||
|
||
// Test 2: With @Rollback - provides both session and transaction | ||
@Rollback | ||
void "test with rollback provides session and transaction"() { | ||
when: "Perform operations" | ||
def count = TestComparisonDomain.count() | ||
def domain = new TestComparisonDomain(name: "With Rollback") | ||
domain.save(flush: true) // This works because transaction is active | ||
|
||
then: "Both operations succeed" | ||
count >= 0 | ||
domain.id != null | ||
sessionFactory.currentSession != null | ||
sessionFactory.currentSession.transaction.active | ||
} | ||
|
||
// Test 3: With @WithSession - provides session only, no transaction | ||
@WithSession | ||
void "test with session annotation provides session without transaction"() { | ||
given: | ||
static transactional = false | ||
|
||
when: "Perform SELECT operation" | ||
def count = TestComparisonDomain.count() | ||
|
||
then: "SELECT works with session only" | ||
count >= 0 | ||
sessionFactory.currentSession != null | ||
!sessionFactory.currentSession.transaction.active | ||
|
||
when: "Try save without flush" | ||
def domain = new TestComparisonDomain(name: "Session Only") | ||
domain.save() | ||
|
||
then: "Save without flush works" | ||
domain.id != null | ||
|
||
when: "Try save with flush" | ||
domain.save(flush: true) | ||
|
||
then: "Save with flush fails without transaction" | ||
thrown(Exception) | ||
} | ||
|
||
// Test 4: Method-level @WithSession annotation | ||
void "test method level session annotation"() { | ||
given: | ||
static transactional = false | ||
|
||
expect: "This specific test method has session bound" | ||
withSessionMethod() | ||
} | ||
|
||
@WithSession | ||
private boolean withSessionMethod() { | ||
TestComparisonDomain.count() >= 0 | ||
sessionFactory.currentSession != null | ||
!sessionFactory.currentSession.transaction.active | ||
} | ||
} | ||
|
||
// Test domain class for comparison tests | ||
class TestComparisonDomain { | ||
String name | ||
|
||
static constraints = { | ||
name nullable: false | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you 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 | ||
* | ||
* https://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. | ||
*/ | ||
package functional.tests | ||
|
||
import grails.testing.mixin.integration.Integration | ||
import grails.testing.mixin.integration.WithSession | ||
import grails.gorm.transactions.Transactional | ||
import org.hibernate.SessionFactory | ||
import spock.lang.Specification | ||
import org.springframework.beans.factory.annotation.Autowired | ||
|
||
/** | ||
* Test demonstrating the @WithSession annotation functionality. | ||
* This test class shows how to have a Hibernate session bound without a transaction, | ||
* matching the runtime OISV behavior. | ||
*/ | ||
@Integration | ||
@WithSession | ||
class WithSessionIntegrationSpec extends Specification { | ||
|
||
@Autowired | ||
SessionFactory sessionFactory | ||
|
||
@Autowired | ||
TestService testService | ||
|
||
// Set transactional to false to prevent automatic transaction wrapping | ||
static transactional = false | ||
|
||
void "test that session is bound without transaction"() { | ||
given: "A test domain object" | ||
def testDomain = new TestDomain(name: "Test Session Binding") | ||
|
||
when: "We perform a SELECT operation (which requires session but not transaction)" | ||
def count = TestDomain.count() | ||
|
||
then: "The operation succeeds because a session is bound" | ||
count >= 0 | ||
sessionFactory.currentSession != null | ||
!sessionFactory.currentSession.transaction.active | ||
} | ||
|
||
void "test that save without flush works with session only"() { | ||
given: "A domain object" | ||
def testDomain = new TestDomain(name: "Session Only Save") | ||
|
||
when: "We save without flush (no transaction required)" | ||
testDomain.save() | ||
|
||
then: "Save succeeds because session is available" | ||
testDomain.id != null | ||
!sessionFactory.currentSession.transaction.active | ||
} | ||
|
||
void "test that save with flush fails without transaction"() { | ||
given: "A domain object" | ||
def testDomain = new TestDomain(name: "Flush Test") | ||
|
||
when: "We try to save with flush (requires transaction)" | ||
testDomain.save(flush: true) | ||
|
||
then: "An exception is thrown because no transaction is active" | ||
thrown(Exception) | ||
} | ||
|
||
void "test service method without @Transactional behaves correctly"() { | ||
when: "Calling a non-transactional service method that does SELECT" | ||
def result = testService.performNonTransactionalRead() | ||
|
||
then: "The method succeeds because session is bound" | ||
result != null | ||
!sessionFactory.currentSession.transaction.active | ||
} | ||
|
||
void "test service method with @Transactional creates transaction"() { | ||
when: "Calling a transactional service method" | ||
def result = testService.performTransactionalOperation() | ||
|
||
then: "The method runs in a transaction" | ||
result != null | ||
// Transaction will be committed after method completes | ||
} | ||
} | ||
|
||
// Test domain class | ||
class TestDomain { | ||
String name | ||
|
||
static constraints = { | ||
name nullable: false | ||
} | ||
} | ||
|
||
// Test service class | ||
@grails.gorm.services.Service(TestDomain) | ||
abstract class TestService { | ||
|
||
// Non-transactional method - relies on session from OISV | ||
def performNonTransactionalRead() { | ||
return TestDomain.count() | ||
} | ||
|
||
// Transactional method - creates its own transaction | ||
@Transactional | ||
def performTransactionalOperation() { | ||
def domain = new TestDomain(name: "Transactional Save") | ||
domain.save(flush: true) | ||
return domain | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,7 @@ dependencies { | |
api project(':grails-databinding') | ||
api project(':grails-datamapping-core') | ||
api project(':grails-test-core') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test-core is meant to be the parent to this dependency. Why are you removing it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Restored grails-test-core dependency in grails-testing-support-core |
||
api('org.spockframework:spock-core') { transitive = false } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is incudes by test-core so it should be unnecessary |
||
api 'org.springframework.boot:spring-boot-test' | ||
api('org.spockframework:spock-spring') { transitive = false } | ||
api 'org.junit.jupiter:junit-jupiter-api' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The application behavior depends if OSIV is configured and if hibernate is used. We should have our tests default to best behaviors, or even better, detect the configuration and then only apply it based on the application configuration.