Keyfactor / signserver-ce

SignServer – Open source, PKI-based signing software to sign code, documents, timestamps and more.
https://www.signserver.org
GNU Lesser General Public License v2.1
304 stars 32 forks source link

Transaction support for signing and timed service #76

Open 3keyroman opened 5 months ago

3keyroman commented 5 months ago

There are use cases, when siging worker needs to be executed in transaction because it interacts with the database. Typically, when the keys are stored in the database, or some other information needs to be updated during signing execution.

This implementation introduces feature to turn on the transaction handling for other use cases then for key usage counter or archiving. Specifically for signing and timed services.

There is a new interface requiresTransaction implemented, which is false by default.

mlundblad commented 4 months ago

Hi!

This looks very interesting!

There's quite a lot of whitespace/formatting changes that are unrelated to the actual feature. Could you break these out into a separate PR? This makes it easier to track the changes and review the changes.

Thanks!

3keyroman commented 4 months ago

Hi!

This looks very interesting!

There's quite a lot of whitespace/formatting changes that are unrelated to the actual feature. Could you break these out into a separate PR? This makes it easier to track the changes and review the changes.

Thanks!

It was automatically formatted by IDE I believe, I will try to remove it and keep it clear.

3keyroman commented 4 months ago

@mlundblad hopefully the formatting is better now.

3keyroman commented 2 months ago

@mlundblad I am just wondering if this PR is ok and will be merged into the main branch. Is there anything I should clarify?

netmackan commented 4 days ago

There are a lot of commits in this PR, maybe it needs to be rebased?

netmackan commented 4 days ago

Perhaps the large amount of commits could be due to have we handle the Git repository?

@3keyroman Assuming f556d526df9663c7c2434c2c536aa24eb442c036 is the only relevant commit for this PR?, then there are still a number of whitespace and formatting changes that would be good to leave out to make it easier to review and to discuss further.

3keyroman commented 3 days ago

@netmackan my last commit resolving the whitespaces and formatting was https://github.com/Keyfactor/signserver-ce/pull/76/commits/a5696d3bf1694e8cd279e5d9b266b09ee58f9027. I am not able to do merge or rebase because of the same conflicts with the html files.

netmackan commented 2 days ago

@3keyroman Ok, thanks I see it now. This is likely an issue with our repo. I think I need to create a new PR and just add your commit(s).