Skip to content

Conversation

jonenst
Copy link
Contributor

@jonenst jonenst commented Sep 12, 2025

The @transactional was already unneeded, it didn't propagate to the executor thread Effectly it opened a transaction and instantly closed it.

Spring forces us to adapt the tests with boilerplate code (asyncStarted() and asyncDispatch()) as per https://docs.spring.io/spring-framework/reference/testing/mockmvc/hamcrest/async-requests.html to mimic the servlet 3.0 redispatch

….0+ AsyncContext)

The @transactional was already unneeded, it didn't propagate to the executor thread
Effectly it opened a transaction and instantly closed it.

Spring forces us to adapt the tests with boilerplate code (asyncStarted() and asyncDispatch()) as per
https://docs.spring.io/spring-framework/reference/testing/mockmvc/hamcrest/async-requests.html
to mimic the servlet 3.0 redispatch
Copy link

};
}

@Transactional(readOnly = true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we move this Transactional somewhere else ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest in a separate PR ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(also in this case it looks like all the computation is just a single repository call to the standard find method so it should be equivalent I think.. not 100% sure from the top of my head though. Also not sure what is the official code to do transactions in a separate thread.. probably it's just the same but I would need to double check..)

} else {
return getSubstationsByCountries(network, countrySet);
} catch (Exception e) {
throw new GeoDataException(FAILED_SUBSTATIONS_LOADING, e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it behave the same to throw inside or outside the async ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it's the same. And there are unit tests that verify that from an http client point of view, the thrown exception inside the completablefuture is correctly used by the servlet container asynchronously to return the associated error http reponse to the client

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants