feat(policy): Create/Update scs to use transaction.

[c-r33d]

Proposed Changes

1.) Add transactions around subject-condition-set creation/update.

Note

I believe this is the cause for all the intermittent/flaky otdfctl
test failures that have been occurring.

Checklist

• I have added or updated unit tests
• I have added or updated integration tests (if appropriate)
• I have added or updated documentation

Testing Instructions

[gemini-code-assist]

Summary of Changes

Hello @c-r33d, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces database transactions to the CreateSubjectConditionSet and UpdateSubjectConditionSet functions within the policy service. The primary goal is to improve the robustness and consistency of these critical operations, thereby mitigating observed intermittent test failures and ensuring data integrity during the lifecycle of Subject Condition Sets.

Highlights

• Transactional Operations: The creation and update operations for Subject Condition Sets (SCS) have been refactored to utilize database transactions. This ensures that all steps within these processes are treated as a single, atomic unit of work.
• Enhanced Reliability: This change is specifically aimed at resolving intermittent and flaky test failures related to otdfctl, which are believed to stem from a lack of transactional integrity during SCS creation and updates.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

Data must be safe, Transactions keep it sound, No more flaky tests.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces database transactions for creating and updating subject condition sets, which is a great step towards improving data consistency and resolving potential race conditions. My review focuses on refining the transaction handling to ensure audit logs are accurate. I've suggested moving the success audit logs to execute only after the transactions have successfully committed, which will prevent misleading audit trails in case of a commit failure.

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	217.883675ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	102.717416ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	366.164035ms
Throughput	273.10 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	40.440699823s
Average Latency	402.056158ms
Throughput	123.64 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	28.457725502s
Average Latency	283.880633ms
Throughput	175.70 requests/second

[github-actions]

X-Test Results

opentdfplatformW4DU64.dockerbuild
✅ js-v0.4.34
cukes-report
✅ go-v0.4.34
✅ java-v0.4.34
bats-test-results
✅ js-pull-2882
✅ java-pull-2882
✅ go-pull-2882

[c-r33d]

/gemini review

[gemini-code-assist]

Code Review

This pull request correctly introduces database transactions for creating and updating subject condition sets, which is a commendable practice for ensuring data atomicity and consistency. This change is likely to resolve the intermittent test failures as you've noted. My review includes a couple of suggestions to refine the transaction handling code to be more idiomatic and readable in Go, which should improve maintainability.

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	165.196708ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	84.433975ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	375.551124ms
Throughput	266.28 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	39.229310237s
Average Latency	390.448651ms
Throughput	127.46 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	27.552532568s
Average Latency	274.809259ms
Throughput	181.47 requests/second

[c-r33d]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces database transactions for creating and updating subject condition sets, which is a solid improvement for data consistency and to prevent potential race conditions. The implementation for CreateSubjectConditionSet is straightforward. For UpdateSubjectConditionSet, wrapping the get-then-update logic in a transaction correctly addresses the atomicity concern. I've identified a minor performance issue in UpdateSubjectConditionSet where the same object might be fetched twice from the database within the same transaction. My specific comment provides more detail on this.

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	187.578382ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	102.338612ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	366.698803ms
Throughput	272.70 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	39.348251138s
Average Latency	391.59188ms
Throughput	127.07 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	27.40411527s
Average Latency	272.902046ms
Throughput	182.45 requests/second

[github-actions]

X-Test Results

opentdfplatformNYPM71.dockerbuild
cukes-report
✅ js-v0.4.34
✅ java-v0.4.34
✅ go-v0.4.34
bats-test-results
test-cases-mapping-report
✅ js-pull-2882
✅ java-pull-2882
✅ go-pull-2882

[github-actions]

X-Test Results

opentdfplatform0TFFZS.dockerbuild
cukes-report
✅ js-v0.4.34
✅ java-v0.4.34
✅ go-v0.4.34
bats-test-results
test-cases-mapping-report
✅ js-pull-2882
✅ java-pull-2882
✅ go-pull-2882