Support Protobuf enums as identifier fields - #948
Merged
Conversation
Also: * Consolidate logic of supported primtitive type under `Identifier`.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65e0c1e0a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class support for using Protobuf enums as identifier (ID) values in base, and centralizes the “which Protobuf field types are eligible to be an ID” rule in Identifier. The PR also includes several build/tooling updates (Gradle repo scoping, license report cache invalidation on version changes, submodule bootstrap, and dependency/version bumps).
Changes:
- Introduce
IdType.ENUMand makeIdentifierenum-aware (isEmptytreats the number-zero constant as empty,toStringreturns constant name, enum pack/unpack support). - Add
Identifier.isSupportedIdType(...)(type/descriptor overloads) backed by a single authoritativeSUPPORTED_ID_TYPESset, plus tests/fixtures for enum IDs. - Update build/tooling and metadata files (repository content filtering, license report task inputs, init-submodules bootstrap script, dependency/version updates, docs/dependency report regeneration).
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| version.gradle.kts | Bumps published snapshot version. |
| init-submodules | Adds a bootstrap script to initialize missing submodules in fresh worktrees/shallow checkouts. |
| gradle.properties | Expands Gradle daemon JVM args (incl. Error Prone-related --add-exports/--add-opens). |
| docs/dependencies/pom.xml | Updates documented snapshot version. |
| docs/dependencies/dependencies.md | Regenerates dependency/license report with new version/date. |
| buildSrc/src/test/kotlin/io/spine/gradle/report/pom/DependencyWriterSpec.kt | Updates test configuration name used for dependency scope selection. |
| buildSrc/src/main/kotlin/io/spine/gradle/report/license/LicenseReporter.kt | Forces license report regeneration when project version changes (cache invalidation input). |
| buildSrc/src/main/kotlin/io/spine/gradle/repo/Repositories.kt | Reorders/scopes repositories to reduce resolution failures and unnecessary lookups. |
| buildSrc/src/main/kotlin/io/spine/gradle/github/pages/UpdateGitHubPages.kt | Updates docs about gh-pages branch creation behavior. |
| buildSrc/src/main/kotlin/io/spine/gradle/github/pages/RepositoryExtensions.kt | Seeds a newly created gh-pages branch with a CNAME file. |
| buildSrc/src/main/kotlin/io/spine/gradle/git/Repository.kt | Adds checkoutOrCreate and branch-seeding/push-adoption logic for docs publishing. |
| buildSrc/src/main/kotlin/io/spine/gradle/Cli.kt | Ensures process output reader threads are joined before evaluating buffers. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/ProtoTap.kt | Bumps ProtoTap version. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/CoreJvmCompiler.kt | Bumps core-jvm-compiler dogfooding/test versions. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/CoreJvm.kt | Bumps core-jvm version. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/Compiler.kt | Bumps compiler fallback versions. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/Base.kt | Bumps base dependency versions used by build scripts. |
| base/src/test/proto/spine/test/identifiers_test.proto | Adds TaskStatus enum + EnumFieldId fixture message. |
| base/src/test/kotlin/io/spine/base/IdTypeTest.kt | Adds enum-to-EnumValue conversion test. |
| base/src/test/java/io/spine/base/IdentifierTest.java | Adds enum ID tests and isSupportedIdType test coverage. |
| base/src/main/java/io/spine/base/IdType.java | Adds ENUM ID type and zero-value enum default handling. |
| base/src/main/java/io/spine/base/Identifier.java | Makes Identifier enum-aware and introduces isSupportedIdType(...) as the single source of truth. |
| AGENTS.md | Documents init-submodules bootstrap and updates skip-list. |
| .gitmodules | Adjusts submodule update strategy. |
| .claude/settings.json | Adds a SessionStart hook to run init-submodules. |
alexander-yevsyukov
enabled auto-merge (squash)
June 18, 2026 20:37
`Field.findIdField` guarded only the `MESSAGE` case with `sameMessageType`, so an enum ID search matched the first enum field regardless of its enum type — for a message with two enum fields of different types, searching for one would return the other. Add a parallel `sameEnumType` guard so an enum ID field is matched by its specific enum type, with a regression test over `TwoEnumFieldsId`, which declares an unrelated enum field before the matching one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`IdType.ENUM.matchClass` and `Identifier.unpack(Any, Class)` accepted any `ProtocolMessageEnum`-assignable class, including the `ProtocolMessageEnum` interface itself and other non-enum implementors. The interface has no enum constants, so `defaultValue()` would fail on `getEnumConstants()`, and `TypeConverter` cannot convert it. Require `idClass.isEnum()` in both places (matching `ProtoConverter.isProtoEnum`), with a regression test that the `ProtocolMessageEnum` interface is rejected as an ID class. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #948 +/- ##
==========================================
- Coverage 94.04% 94.02% -0.03%
==========================================
Files 192 192
Lines 4149 4185 +36
Branches 339 346 +7
==========================================
+ Hits 3902 3935 +33
- Misses 147 149 +2
- Partials 100 101 +1 🚀 New features to boost your workflow:
|
`IdType.ENUM.matchValue` accepted any `ProtocolMessageEnum` instance, so a non-enum implementor would be classified as an enum ID and later fail the `(Enum<?>)` cast in `Identifier.toString()`. Require the value to be a Java enum constant too, consistent with `matchClass`, with a regression test using a non-enum `ProtocolMessageEnum` stub. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover the previously-untested enum-ID branches: rejecting a plain Java enum (value, class, and unpack paths), the `UNRECOGNIZED` constant in `isEmpty`, and the `ENUM.fromMessage` contract. Simplify `IdType.zeroValue` to rely on the proto3 guarantee that the constant with number zero is declared first — removing the unreachable UNRECOGNIZED-skip and no-zero-throw branches and the duplicated `UNRECOGNIZED` literal. Collapse `isUndefinedEnum` now that `matchValue` guarantees the value is a Java enum. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per review, `zeroValue` relied on the proto3 rule that the number-zero constant is declared first. Locate it by number through the enum descriptor (`findValueByNumber(0)`) so it is correct regardless of declaration order, falling back to the first constant only when the enum declares no zero value (possible for proto2). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
armiol
approved these changes
Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds support for Protobuf enums as identifier (ID) fields, and makes
io.spine.base.Identifierthe single source of truth for which Protobuf fieldtypes may serve as an ID.
Enum IDs
IdType.ENUMrecognizes a Protobuf enum (aProtocolMessageEnum) as a validID type: value/class/field matching,
defaultValue(the number-zero constant),and packing to
com.google.protobuf.EnumValue.Identifieris enum-aware:isEmptytreats the number-zero constant as empty — by convention it isthe reserved "undefined" (null-like) value, so the implicit
(required)check rejects it, just like
""forStringand the default instance for aMessage.toStringreturns the constant name.unpack(Any, Class)reconstructs the enum constant viaTypeConverter.Single source of truth for supported ID types
Identifier.isSupportedIdType(FieldDescriptorProto.Type)and anIdentifier.isSupportedIdType(FieldDescriptor)overload, backed by oneauthoritative
SUPPORTED_ID_TYPESset next to the "Supported types" docs:string, every 32/64-bit integer encoding,enum, andMessage.core-jvm-compiler(production codeand its tests). Centralizing it on the documented gateway lets downstream
tools delegate instead of maintaining their own copy.
Tests & fixtures
TaskStatusenum +EnumFieldIdmessage fixtures.IdentifierTest/IdTypeTestcover enum classification,defaultValue,isEmpty,toString, pack/unpackround-trip, and the newisSupportedIdTypepredicate (per-type checks, a completeness guard over everyFieldDescriptorProto.Type, the descriptor overload, and consistency withIdType.matchField).Why
Enum-typed IDs are a requested ID shape. The zero enum value is reserved for the
"undefined" state, so it must be rejected as an empty ID. Hosting the
supported-type rule on
Identifier(the documented gateway) removes a cross-repoduplication and keeps the rule in one place.
Notes for reviewers
AnyasEnumValue(name + number only — not the enumtype), so the constant can only be restored when the class is known:
unpack(Any, Class)does this; the class-lessunpack(Any)returns the rawEnumValue(unchanged behavior).isSupportedIdTypeclassifies the type axis only — arepeated/mapfield is never an ID regardless of element type, and
google.protobuf.Emptyis handled separately by callers.
core-jvm-compiler(RequiredIdReactiondelegating tothis gateway and accepting enums at compile time) will follow as a separate PR,
gated on this change publishing.
🤖 Generated with Claude Code
Also in this branch: a
configsubmodule updateThis branch floats the shared
configsubmodule to its latest commit, so the diffalso contains the build/infra files that
configdistributes into every consumerrepo. These are not part of the enum-ID work and are reviewed in the
configrepository, not here (per the AGENTS.md code-review filter). They are included only
because the branch was rebuilt against current
config.A reviewer focused on the enum-ID change can limit attention to the
base/sources.