fix(policy): omit empty attribute values from create responses

[jakedoublev]

Prevent attribute queries from returning a synthetic empty value when attributes are created without values.

Adds regression coverage for create, get, and list.

Summary by CodeRabbit

• Bug Fixes

  • API responses no longer include empty/null attribute-value entries; attribute/obligation/namespace retrievals and listings omit absent values/keys.
  • Creation/validation now permits attribute definitions without provided values.

• Tests

  • Added integration and unit tests to verify attributes, obligations, and namespaces created without values/keys are returned without empty/null entries.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds integration tests ensuring created resources with nil/absent collections don't return empty entries, permits attributes with empty Values in validation and PDP registration, and updates SQL aggregations to filter out NULL attribute-value rows from JSON_AGG results.

Changes

Cohort / File(s)	Summary
Integration — Attributes service/integration/attributes_test.go	Added Test_CreateAttribute_WithoutValues_DoesNotReturnEmptyValue to assert attributes created without Values return empty Values and appear in ListAttributes with empty Values.
Integration — Namespaces service/integration/namespaces_test.go	Added Test_CreateNamespace_WithoutPublicKeys_DoesNotReturnKeys to assert created namespace with no public keys returns empty kas_keys in create and subsequent get calls; uses deferred unsafe deletion cleanup.
Integration — Obligations service/integration/obligations_test.go	Added Test_CreateObligation_WithoutValues_DoesNotReturnValues to assert obligations created with Values: nil return empty Values when fetched and listed; includes deferred cleanup.
PDP & Validation Tests service/internal/access/v2/pdp_test.go, service/internal/access/v2/validators_test.go	Added TestNewPolicyDecisionPoint_AllowsAttributeDefinitionsWithoutValues; updated TestValidateAttribute expectations to accept empty or nil Values.
Validation Logic service/internal/access/v2/validators.go	Relaxed validateAttribute to permit empty Values (only validate when values are present), and retained per-value checks (non-nil, FQN prefix).
SQL Queries & Generated Code service/policy/db/queries/attributes.sql, service/policy/db/attributes.sql.go	Updated listAttributesDetail and getAttribute aggregations to add FILTER (WHERE avt.id IS NOT NULL) to JSON_AGG(...) ORDER BY ..., excluding NULL attribute-value rows produced by LEFT JOINs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through queries, tests in tow,
NULLs trimmed gently so outputs glow,
Values may sleep, but definitions stand,
Clean aggregates now across the land,
🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: fixing the issue where attribute creation without values incorrectly returns empty values in responses.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/empty-attribute-values

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Summary of Changes

Hello, 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 resolves an issue where the system would return an empty, synthetic value for attributes that were created without any explicit values. By modifying the underlying database queries to explicitly filter out null attribute value IDs, the system now correctly omits these empty values from responses. Comprehensive integration tests have been added to validate this fix across attribute creation, retrieval, and listing operations, ensuring data consistency and preventing unexpected empty value representations.

Highlights

• Fix for Empty Attribute Values: Addressed an issue where attribute queries returned synthetic empty values for attributes created without explicit values.
• Enhanced Query Filtering: Implemented a FILTER (WHERE avt.id IS NOT NULL) clause in SQL queries to prevent the inclusion of null attribute values in responses.
• Improved Test Coverage: Added a new regression test case to ensure attributes created without values correctly return empty value lists across create, get, and list operations.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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.

---

Values, none assigned, No empty ghost appears, Clean data now flows.

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 correctly addresses an issue where creating an attribute without values would result in a synthetic empty value in query responses. The fix, which involves adding a FILTER clause to the JSON_AGG function in the SQL queries, is appropriate and effective for the GetAttribute and ListAttributes code paths. The addition of the Test_CreateAttribute_WithoutValues_DoesNotReturnEmptyValue regression test is also a great improvement.

I've identified one area where the fix might be incomplete, related to another query that performs a similar aggregation. My review comment provides details on this for the sake of consistency and future-proofing.

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: an unexpected error occurred
Total Time	25.231726ms

Benchmark Statistics

Name	№ Requests	Avg Duration	Min Duration	Max Duration

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	213.202204ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	25.550355958s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

[github-actions]

X-Test Failure Report

✅ go-v0.9.0
✅ java-v0.9.0
✅ js-v0.9.0
❌ go-pull-3193
❌ java-pull-3193
❌ js-pull-3193
test-cases-mapping-report
bats-test-results
cukes-report
opentdfplatformXCXWZ5.dockerbuild

[github-actions]

X-Test Failure Report

opentdfplatformWIABFE.dockerbuild

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: an unexpected error occurred
Total Time	24.109585ms

Benchmark Statistics

Name	№ Requests	Avg Duration	Min Duration	Max Duration

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	222.073041ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	25.447882571s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

[github-actions]

X-Test Failure Report

❌ go-pull-3193
❌ java-pull-3193
❌ js-pull-3193
✅ go-v0.9.0
✅ java-v0.9.0
✅ js-v0.9.0
❌ go-pull-3193
❌ java-pull-3193
❌ js-pull-3193
test-cases-mapping-report
bats-test-results
cukes-report
opentdfplatformXCXWZ5.dockerbuild

[github-actions]

X-Test Failure Report

❌ go-pull-3193
❌ java-pull-3193
❌ js-pull-3193
✅ go-v0.9.0
✅ java-v0.9.0
✅ js-v0.9.0
❌ go-pull-3193
❌ java-pull-3193
❌ js-pull-3193
test-cases-mapping-report
bats-test-results
cukes-report
opentdfplatformXCXWZ5.dockerbuild

[github-actions]

X-Test Failure Report

✅ go-v0.9.0
✅ java-v0.9.0
✅ js-v0.9.0
❌ go-pull-3193
❌ java-pull-3193
❌ js-pull-3193
cukes-report
test-cases-mapping-report
bats-test-results
opentdfplatformWIABFE.dockerbuild

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Benchmark Statistics

Name	№ Requests	Avg Duration	Min Duration	Max Duration

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	391.621639ms
Throughput	255.35 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	39.811087234s
Average Latency	396.675792ms
Throughput	125.59 requests/second

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

service/internal/access/v2/validators.go (2)

111-113: ⚠️ Potential issue | 🟠 Major

Tighten attribute-value FQN validation to prevent sibling-prefix false positives.

At line 111, strings.HasPrefix(value.GetFqn(), attribute.GetFqn()) incorrectly accepts sibling attribute definitions. For example, "https://example.org/attr/name2/value/x" would match against "https://example.org/attr/name" due to string prefix matching. This validation bug is not covered by existing tests.

Suggested fix

-		if !strings.HasPrefix(value.GetFqn(), attribute.GetFqn()) {
+		expectedPrefix := strings.TrimSuffix(attribute.GetFqn(), "/") + "/value/"
+		if !strings.HasPrefix(value.GetFqn(), expectedPrefix) {
 			return fmt.Errorf("attribute value FQN must be of definition FQN: %w", ErrInvalidAttributeDefinition)
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@service/internal/access/v2/validators.go` around lines 111 - 113, The current
check using strings.HasPrefix(value.GetFqn(), attribute.GetFqn()) in
validators.go allows sibling FQNs to match; change the validation in the
function that performs this check so the value FQN matches the attribute FQN as
a path segment boundary — e.g., after confirming
strings.HasPrefix(value.GetFqn(), attribute.GetFqn()), also verify that either
the two FQNs are equal or the next character in value.GetFqn() after
attribute.GetFqn() is a '/' (or other defined separator), and return
fmt.Errorf("attribute value FQN must be of definition FQN: %w",
ErrInvalidAttributeDefinition) if not; reference value.GetFqn(),
attribute.GetFqn(), and ErrInvalidAttributeDefinition when locating and updating
the logic.

---

100-119: ⚠️ Potential issue | 🔴 Critical

Define missing error constants to fix golangci-lint compilation errors.

The code at lines 100-119 uses ErrInvalidAttributeDefinition, but this constant (along with ErrInvalidResource, ErrInvalidSubjectMapping, ErrInvalidRegisteredResource, and ErrInvalidRegisteredResourceValue) is not defined in the file. This causes 18 typecheck errors. Add these error constants to the var block at the top of the file, following the pattern of the existing errors:

var (
	ErrInvalidAction                = errors.New("access: invalid action")
	ErrInvalidEntityChain           = errors.New("access: invalid entity chain")
	ErrInvalidEntitledFQNsToActions = errors.New("access: invalid entitled FQNs to actions")
	ErrInvalidResource              = errors.New("access: invalid resource")
	ErrInvalidSubjectMapping        = errors.New("access: invalid subject mapping")
	ErrInvalidAttributeDefinition   = errors.New("access: invalid attribute definition")
	ErrInvalidRegisteredResource    = errors.New("access: invalid registered resource")
	ErrInvalidRegisteredResourceValue = errors.New("access: invalid registered resource value")
)

After adding these constants, run golangci-lint run ./service/internal/access/v2/validators.go to verify all 18 errors are resolved, then run go test ./service/internal/access/v2/... to ensure tests pass.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@service/internal/access/v2/validators.go` around lines 100 - 119, Add the
missing error variables to the top-level var block in the file so the validators
referencing ErrInvalidAttributeDefinition, ErrInvalidResource,
ErrInvalidSubjectMapping, ErrInvalidRegisteredResource, and
ErrInvalidRegisteredResourceValue compile; specifically add ErrInvalidResource,
ErrInvalidSubjectMapping, ErrInvalidAttributeDefinition,
ErrInvalidRegisteredResource, and ErrInvalidRegisteredResourceValue following
the existing pattern used for ErrInvalidAction and ErrInvalidEntityChain (i.e.,
errors.New("access: invalid <name>")), ensuring the var block contains all of:
ErrInvalidAction, ErrInvalidEntityChain, ErrInvalidEntitledFQNsToActions,
ErrInvalidResource, ErrInvalidSubjectMapping, ErrInvalidAttributeDefinition,
ErrInvalidRegisteredResource, and ErrInvalidRegisteredResourceValue so functions
like validateAttribute and others can reference them without typecheck errors.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@service/internal/access/v2/pdp_test.go`:
- Around line 894-895: The test currently asserts that
pdp.allEntitleableAttributesByValueFQN does not contain emptyAttrFQN but misses
catching synthetic empty-string keys; update the assertion logic in the test
(around pdp, allEntitleableAttributesByValueFQN, and emptyAttrFQN) to also
assert that the empty string key ("") is not present in
pdp.allEntitleableAttributesByValueFQN (i.e., add a NotContains/False check for
"" or verify the map does not have "" as a key) so the test fails if a synthetic
empty-value entry is introduced.

---

Outside diff comments:
In `@service/internal/access/v2/validators.go`:
- Around line 111-113: The current check using strings.HasPrefix(value.GetFqn(),
attribute.GetFqn()) in validators.go allows sibling FQNs to match; change the
validation in the function that performs this check so the value FQN matches the
attribute FQN as a path segment boundary — e.g., after confirming
strings.HasPrefix(value.GetFqn(), attribute.GetFqn()), also verify that either
the two FQNs are equal or the next character in value.GetFqn() after
attribute.GetFqn() is a '/' (or other defined separator), and return
fmt.Errorf("attribute value FQN must be of definition FQN: %w",
ErrInvalidAttributeDefinition) if not; reference value.GetFqn(),
attribute.GetFqn(), and ErrInvalidAttributeDefinition when locating and updating
the logic.
- Around line 100-119: Add the missing error variables to the top-level var
block in the file so the validators referencing ErrInvalidAttributeDefinition,
ErrInvalidResource, ErrInvalidSubjectMapping, ErrInvalidRegisteredResource, and
ErrInvalidRegisteredResourceValue compile; specifically add ErrInvalidResource,
ErrInvalidSubjectMapping, ErrInvalidAttributeDefinition,
ErrInvalidRegisteredResource, and ErrInvalidRegisteredResourceValue following
the existing pattern used for ErrInvalidAction and ErrInvalidEntityChain (i.e.,
errors.New("access: invalid <name>")), ensuring the var block contains all of:
ErrInvalidAction, ErrInvalidEntityChain, ErrInvalidEntitledFQNsToActions,
ErrInvalidResource, ErrInvalidSubjectMapping, ErrInvalidAttributeDefinition,
ErrInvalidRegisteredResource, and ErrInvalidRegisteredResourceValue so functions
like validateAttribute and others can reference them without typecheck errors.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5008dfad-635b-477a-a533-685bb442acd6

📥 Commits

Reviewing files that changed from the base of the PR and between 81156be and dae3f1b.

📒 Files selected for processing (3)

• service/internal/access/v2/pdp_test.go
• service/internal/access/v2/validators.go
• service/internal/access/v2/validators_test.go

[github-actions]

X-Test Results

✅ go-pull-3193
✅ go-v0.9.0
✅ java-pull-3193
✅ java-v0.9.0
✅ js-pull-3193
✅ js-v0.9.0
test-cases-mapping-report
bats-test-results
cukes-report
opentdfplatformSHAJNA.dockerbuild