apache / apache/grails-core

Wrong response when an Exception happens when an object will be saved in a Controller with Transactional annotation

Open
#14,544 7 comments 0 reactions 0 assignees View on GitHub
relates-to: gorm
Dominant language
Groovy
Stars
2.9k
Forks
975
Avg merge
1d 22h
Merged PRs (30d)
92

Description

I have a Person class which does an evaluation inside of `beforeDelete()` method an depending on the result could throw an exception. Something like that:

```
class Person {
String name
static constraints = {
name unique: true
}
void beforeDelete() {
throw new RuntimeException('just for test')
}
}
```

In addition, I have a controller which has Transactional annotation and this `delete()` action:

```
@Transactional(readOnly = true)
class PersonController { // this doesn't work as I expected

//....
@Transactional
def delete(Long id) {
if (id == null) {
render status: NOT_FOUND
return
}

personService.delete(id)

respond ([status: OK], [message: 'Object deleted'])
}
}
```

**Problem**
When I want to delete a Person object using PersonController, I don't have any exception before to send the response. So, the server response is a `200` and the exception happens then (when the `beforeDelete` is executed)

**Expected**
I expect that when I want to delete a Person object using PersonController, I will have the exception before to send the response and the response will be a `500`.

Note: When I was trying to reproduce this error, first I used the generate-all grails command to create the controller. The command created this (I changed the name):

```
class PersonWithoutTransactionController { // this works as I expected
//...
def delete(Long id) {
if (id == null) {
render status: NOT_FOUND
return
}

personService.delete(id)

respond ([status: OK], [message: 'Object deleted'])
}
}
```

This controller doens't have the Transactional annotation and works I expected. What is the correct way to do it? Should works the same in both cases?

**Reproduce the error**
To reproduce the error I have created these tests:
```
class PersonFunctionalSpec extends GebSpec {

RestBuilder getRestBuilder() {
new RestBuilder()
}

void setup() {
if (!Person.findByName('Tomy')) {
new Person(name: 'Tomy').save(flush:true, failOnError:true)
}
}

void "Test the delete action with Transaction Notation"() {
when:
def id = Person.findByName('Tomy').id
def response = restBuilder.delete("${baseUrl}/person/$id")

then:"you should have an error"
response.status == INTERNAL_SERVER_ERROR.value() // <-- this fail
}

void "Test the delete action without Transaction Notation"() {
when:
def id = Person.findByName('Tomy').id
def response = restBuilder.delete("${baseUrl}/personWithoutTransaction/$id")

then:"you should have an error"
response.status == INTERNAL_SERVER_ERROR.value()
}
}
```
Here is the sample app: https://github.com/AmaliaMV/neo4j-transactional

The versions I am using are:
```
grailsVersion=3.3.8
gormVersion=6.1.11.BUILD-SNAPSHOT
grailsNeo4jPluginVersion=6.2.0.BUILD-SNAPSHOT
```

Contributor guide

Open the contributing guide

Research direction

Start with the sample app and its PersonFunctionalSpec, especially the transactional and non-transactional delete tests. Trace the PersonController delete entry point and compare the two response-status assertions. Done means the transactional delete test also receives INTERNAL_SERVER_ERROR rather than a successful response when beforeDelete throws.

Written by the indexing model from the issue text.

Assessment

Tech stack
groovy, neo4j
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.