[backend] adding @Transactional to the ExecuteInject method to fix issue/2220 #2236
Conversation
heditar
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/current #2236 +/- ##
==================================================
Coverage 32.90% 32.90%
Complexity 1512 1512
==================================================
Files 582 582
Lines 17938 17938
Branches 1154 1154
==================================================
Hits 5903 5903
Misses 11771 11771
Partials 264 264 ☔ View full report in Codecov by Sentry. |
| @@ -380,6 +381,7 @@ public void updateExercise(String exerciseId) { | |||
| exerciseRepository.save(exercise); | |||
| } | |||
|
|
|||
| @Transactional | |||
There was a problem hiding this comment.
It seems risky to position it that high.
Were you able to analyze the impacts in relation to all the methods involved? We can work on this together if needed.
There was a problem hiding this comment.
I didn't spend too much time investigating potential impact as it seems to make sense to me that one "execute" was part of on transaction. What kind of impact are you worried about ? performance ?
|
i think I encounter the same problem with my task because InjectStatus gets updated throughout the process (e.g., adding traces), but in the catch block, the InjectStatus fetched from inject.getStatus() isn't in its latest state.. |
Proposed changes
adding the @Transactionnal annotation to the InjectsExecutionJob.execute method to fix an issue with the Lazy initialization that was happening when an InjectStatus was saved
adding the @JsonIgnore annotation to Base.isListened method as the class StatusPayload as a an attribute payloadOutput that needs to be serialized and deserialized and having a property with a get a no set breaks the deserialization.
This issue is only impacting FileDrop payload as the issue with serialization deserialization is on the Document class.
This PR doesn't include test as having a test that would be relevant to this bug would cost too much and would require refactoring.
Related issues
Checklist
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...