gitgoodjhe
commented
1 month ago I tested a bit and found no regressions. Good job!
Hi @CRoberto1926 thank you for your review! I answered in the comment section to your questions. If you agree with the ones where I don't intend to change them, could you please mark them as "resolved"?
CRoberto1926
…ons simultaneously
-Write operations are now performed with the regular TaskanEngine and its' SqlSession and TransactionFactory which provides the needed transactionality and doesn't open multiple connections
Some additional info on this PR:
for this new check in the TaskanaEngineImpl: if (transactionFactory.getClass().getSimpleName().equals("SpringManagedTransactionFactory")) { sessionManager.startManagedSession(); }
SONAR would like to use "instanceof" instead of comparing the class name with a String. Problem is, that we have no dependency at this point in the core, so the SpringManagedTransactionFactory.class is not known
The JobScheduler gets initialized in the TaskanaEngineImpl. Unfortunately, it currently creates its' own TaskanaEngineImpl: if (this.taskanaConfiguration.isJobSchedulerEnabled()) { TaskanaConfiguration configuration = new TaskanaConfiguration.Builder(this.taskanaConfiguration) .jobSchedulerEnabled(false) .build(); TaskanaEngine taskanaEngine = TaskanaEngine.buildTaskanaEngine(configuration, EXPLICIT, transactionFactory);
Issue: The HistoryEventManager (and all other SPis) initialization logic follows at the end of the TaskanaEngineImpl constructor. Therefore the initialize() will get called twice and therefore we need the if checks when adding the mappers in the new initialization logic of the SimpleHistoryServiceImpl, e.g.: if (!sqlSession .getConfiguration() .getMapperRegistry() .hasMapper(TaskHistoryEventMapper.class)) { sqlSession.getConfiguration().addMapper(TaskHistoryEventMapper.class); }
Release Notes:
### For the submitter: - [ ] I updated the [documentation](https://taskana.atlassian.net/wiki/spaces/TAS/overview) and will supply links to the specific files - [x] I did not update the [documentation](https://taskana.atlassian.net/wiki/spaces/TAS/overview) - [ ] I included a link to the [SonarCloud branch analysis](https://taskana.atlassian.net/wiki/spaces/TAS/pages/1019969636/SonarCloud+Integration) - [ ] After integration of the PR, I added a description of changes on the [current release notes](https://taskana.atlassian.net/wiki/spaces/TAS/pages/1281392672/Current+Release+Notes+Taskana) - [ ] I did not update the [current release notes](https://taskana.atlassian.net/wiki/spaces/TAS/pages/1281392672/Current+Release+Notes+Taskana) - [x ] I put the ticket in review - [ ] After integration of the pull request, I verified our [bluemix test environment](http://taskana.mybluemix.net/taskana) is not broken ### Verified by the reviewer: - [ ] Commit message format → (Closes) #<Issue Number>: Your commit message. - [ ] Submitter's update to [documentation](https://taskana.atlassian.net/wiki/spaces/TAS/overview) is sufficient - [ ] SonarCloud analysis meets our standards - [ ] Update of the [current release notes](https://taskana.atlassian.net/wiki/spaces/TAS/pages/1281392672/Current+Release+Notes+Taskana) reflects changes - [ ] PR fulfills the ticket - [ ] Edge cases and unwanted side effects are tested - [ ] Readability