feat(sdk): add EntityIdentifier convenience constructors#346
feat(sdk): add EntityIdentifier convenience constructors#346marythought merged 2 commits intomainfrom
Conversation
Add EntityIdentifiers utility class with static helpers that mirror the Go SDK's ForEmail, ForClientID, ForUserName, and ForToken constructors. Reduces EntityIdentifier construction from ~10 lines of nested builders to a single method call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Mary Dickson <mary.dickson@virtru.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded a new non-instantiable utility class Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
sdk/src/test/java/io/opentdf/platform/sdk/EntityIdentifiersTest.java (1)
73-77: Add null-input contract tests once constructor behavior is finalized.If you keep/introduce explicit null preconditions, add
assertThrowstests forforEmail,forClientId,forUserName, andforTokenso the SDK contract stays enforced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/test/java/io/opentdf/platform/sdk/EntityIdentifiersTest.java` around lines 73 - 77, Add null-input contract tests in EntityIdentifiersTest: write assertThrows assertions for EntityIdentifier factory methods forEmail, forClientId, forUserName, and forToken to validate they throw on null input once the constructor/null-precondition behavior is finalized; update or add test methods that call EntityIdentifiers.forEmail(null), EntityIdentifiers.forClientId(null), EntityIdentifiers.forUserName(null), and EntityIdentifiers.forToken(null) and assertThrows the expected exception type (e.g., NullPointerException or IllegalArgumentException) to keep the SDK contract enforced.sdk/src/main/java/io/opentdf/platform/sdk/EntityIdentifiers.java (1)
35-69: Define explicit null-input behavior for public constructors.These helpers are part of the SDK surface; adding explicit
nullpreconditions would make failures predictable and error messages clearer.Proposed refactor
package io.opentdf.platform.sdk; import io.opentdf.platform.authorization.v2.EntityIdentifier; import io.opentdf.platform.entity.Entity; import io.opentdf.platform.entity.EntityChain; import io.opentdf.platform.entity.Token; +import java.util.Objects; @@ public static EntityIdentifier forEmail(String email) { + email = Objects.requireNonNull(email, "email must not be null"); return fromEntity(Entity.newBuilder() .setEmailAddress(email) .setCategory(Entity.Category.CATEGORY_SUBJECT) .build()); } @@ public static EntityIdentifier forClientId(String clientId) { + clientId = Objects.requireNonNull(clientId, "clientId must not be null"); return fromEntity(Entity.newBuilder() .setClientId(clientId) .setCategory(Entity.Category.CATEGORY_SUBJECT) .build()); } @@ public static EntityIdentifier forUserName(String userName) { + userName = Objects.requireNonNull(userName, "userName must not be null"); return fromEntity(Entity.newBuilder() .setUserName(userName) .setCategory(Entity.Category.CATEGORY_SUBJECT) .build()); } @@ public static EntityIdentifier forToken(String jwt) { + jwt = Objects.requireNonNull(jwt, "jwt must not be null"); return EntityIdentifier.newBuilder() .setToken(Token.newBuilder().setJwt(jwt).build()) .build(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/io/opentdf/platform/sdk/EntityIdentifiers.java` around lines 35 - 69, The public factory methods forEmail, forClientId, forUserName, and forToken should validate their inputs and fail fast with a clear exception; update each method (forEmail, forClientId, forUserName, forToken) to explicitly check for null (e.g., using Objects.requireNonNull or throwing an IllegalArgumentException/NullPointerException with a descriptive message) before building the Entity/Token, so callers receive a predictable error when a null argument is passed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@sdk/src/main/java/io/opentdf/platform/sdk/EntityIdentifiers.java`:
- Around line 35-69: The public factory methods forEmail, forClientId,
forUserName, and forToken should validate their inputs and fail fast with a
clear exception; update each method (forEmail, forClientId, forUserName,
forToken) to explicitly check for null (e.g., using Objects.requireNonNull or
throwing an IllegalArgumentException/NullPointerException with a descriptive
message) before building the Entity/Token, so callers receive a predictable
error when a null argument is passed.
In `@sdk/src/test/java/io/opentdf/platform/sdk/EntityIdentifiersTest.java`:
- Around line 73-77: Add null-input contract tests in EntityIdentifiersTest:
write assertThrows assertions for EntityIdentifier factory methods forEmail,
forClientId, forUserName, and forToken to validate they throw on null input once
the constructor/null-precondition behavior is finalized; update or add test
methods that call EntityIdentifiers.forEmail(null),
EntityIdentifiers.forClientId(null), EntityIdentifiers.forUserName(null), and
EntityIdentifiers.forToken(null) and assertThrows the expected exception type
(e.g., NullPointerException or IllegalArgumentException) to keep the SDK
contract enforced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e4a4d08-577a-4f51-85f1-8baa0f2d7e7c
📒 Files selected for processing (2)
sdk/src/main/java/io/opentdf/platform/sdk/EntityIdentifiers.javasdk/src/test/java/io/opentdf/platform/sdk/EntityIdentifiersTest.java
There was a problem hiding this comment.
Code Review
This pull request introduces the EntityIdentifiers utility class to the Java SDK, providing static factory methods for creating EntityIdentifier objects from emails, client IDs, usernames, and JWT tokens. Accompanying unit tests were also added. Review feedback recommends implementing explicit null checks for input parameters in the factory methods to prevent NullPointerException and suggests refactoring the token tests into a single parameterized test for improved consistency and maintainability.
sdk/src/test/java/io/opentdf/platform/sdk/EntityIdentifiersTest.java
Outdated
Show resolved
Hide resolved
Address CodeRabbit review: - Add Objects.requireNonNull to all four factory methods - Consolidate forToken tests into a parameterized test - Add null-input contract test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Mary Dickson <mary.dickson@virtru.com>
X-Test Failure Report |
|
|
Both nitpicks addressed in 9b8b01f:
|
X-Test Results✅ java-main |
🤖 I have created a release *beep* *boop* --- <details><summary>0.13.0</summary> ## [0.13.0](v0.12.0...v0.13.0) (2026-04-09) ### Features * **sdk:** add EntityIdentifier convenience constructors ([#346](#346)) ([eeb8805](eeb8805)) * **sdk:** DSPX-2418 add discovery convenience methods ([#339](#339)) ([8de6068](8de6068)) * **sdk:** expose SRT signer ([#329](#329)) ([f93d332](f93d332)) ### Bug Fixes * add a default assertion id if one is not specified ([#341](#341)) ([69d6a53](69d6a53)) * **docs:** DSPX-2409 replace SDK README code example with working code ([#336](#336)) ([0f224a6](0f224a6)) * **sdk:** Support kas keys with extended EC methods ([#344](#344)) ([982b287](982b287)) * **sdk:** Support RSA4096 Kas keys ([#343](#343)) ([dba9bbf](dba9bbf)) * **sdk:** Updates to proto version v0.16.0 ([#308](#308)) ([4660e27](4660e27)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>



Summary
EntityIdentifiersutility class with static helpers mirroring the Go SDK'sForEmail,ForClientID,ForUserName, andForTokenconstructorsBefore:
After:
Test plan
EntityIdentifiersTest— 8 tests covering all 4 helpers with valid and empty string inputs🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests