project-husky / husky

Health Usability Key
https://project-husky.github.io/husky/
Eclipse Public License 1.0
15 stars 8 forks source link

[https://github.com/project-husky/husky/issues/94] Bugfix Concurrency #95

Closed ch-fuchs closed 6 months ago

ch-fuchs commented 6 months ago
qligier commented 6 months ago

Thank you for the PR! In the long term, a better solution would be to extract the ProvideAndRegisterDocumentSet-building methods to a new class, something like ConvenientProvideAndRegisterDocumentSetBuilder. Then ConvenienceCommunication would only contain transmission-related code, and most of concurrency issues would be fixed. For the affirmityDomain, I don't see the concurrency issue with it yet. But making it ThreadLocal would go against its current use as common configuration. If needed, we could implement a clone method on it. Could you handle those changes? Otherwise I can do it another day.

ch-fuchs commented 6 months ago

The goal of the hotfix was to have a minimal change with the current API.

We are currently working on a complete refactoring of the API which will handle all those parts and should be part of the next main release, therefore I would not recommend to make big changes now and keep the code as it is.

For the affirmatyDomain (AD) - there are different URLs per repository, i.e. the destination within the AD is different. Assume there is someone writing a document (e.g. just calling submit) while a different thread does a PDQ query. It can happen that the AD is changed before the submit is executed resulting in the submit being performed for the PDQ repository destination. We saw this behaviour in our tests with the Post.

qligier commented 6 months ago

Okay, if that is being improved in the refactoring, then hot fixing it for now is fine IMHO.

ralych commented 6 months ago

Creating a builder is possible, but has some impacts on the submit methods in the ConvenienceCommunicatioXY implementations of CH and AT. The builder of IPF has to be checked before implementation for gen,ch and at.