refactor(common): Lombok @Getter/@Setter for TaskDef and WorkflowDef, PK-based equals/hashCode#1154
Open
v1r3n wants to merge 3 commits into
Open
refactor(common): Lombok @Getter/@Setter for TaskDef and WorkflowDef, PK-based equals/hashCode#1154v1r3n wants to merge 3 commits into
v1r3n wants to merge 3 commits into
Conversation
… PK-based equals/hashCode Replace hand-written getters/setters in TaskDef and WorkflowDef with Lombok @Getter/@Setter, reducing boilerplate by ~400 lines while preserving all custom logic (null-coalescing getters, business methods, toString). equals/hashCode keyed on PK (name + version) for both classes. TaskDef.getVersion() defaults null to 1, so unversioned and version=1 records are treated as the same PK. Also adds new TaskDef fields (version, metadata) with @protofield annotations, wires them into AbstractProtoMapper, and extends both JSON schemas to cover all fields. Tests: comprehensive coverage of equals/hashCode, version defaulting, null-coalescing getters, all validation constraints, and edge cases for both classes.
v1r3n
force-pushed
the
refactor/lombok-taskdef-workflowdef-pk
branch
from
1a8b442 to
1894cb3
Compare
…oto Value mapping ProtoMapper.toProto(Object) only accepted Double, so any Integer/Long/Float/ BigDecimal/BigInteger in a Map<String,Object> field (inputTemplate, variables, outputParameters, and the newly added metadata/data) threw ClassCastException. Jackson deserializes JSON integers in generic maps as Integer/Long, so this was reachable whenever such a definition was fetched over gRPC. - Accept any Number, widening to double (protobuf Struct only has number_value); precision loss beyond 2^53 is an inherent Struct limitation, now documented. - Coerce map keys via String.valueOf so non-String keys (Map<Integer,?>) and a stray null key serialize instead of throwing CCE / NPE at putFields. Adds a full value-converter test matrix (numeric types, nesting, empty containers, mixed lists, key coercion, rejection of non-JSON types), which was previously entirely uncovered.
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
1 participant
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.
Summary
TaskDefandWorkflowDefwith Lombok@Getter/@Setterat the class level, keeping all custom logic intactequals/hashCodeon both classes now key exclusively on the primary key (name+version) — written as explicit methods for readabilityTaskDef.getVersion()coalescesnull → 1, so unversioned records andversion=1records are always the same PKversionandmetadatafields toTaskDefwith@ProtoFieldannotations; wire both intoAbstractProtoMapper(proto generation completes the mapping that was previously incomplete)schemas/TaskDef.jsonandschemas/WorkflowDef.jsonto cover all Java fields (previously missing:version,metadata,maxRetryDelaySeconds,backoffJitterMs,taskStatusListenerEnabledin TaskDef;RateLimitConfig.policyin WorkflowDef)What was preserved (not replaced by Lombok)
TaskDefgetVersion(),getRateLimitPerFrequency(),getRateLimitFrequencyInSeconds(),concurrencyLimit()TaskDefequals,hashCode,toStringWorkflowDefequals,hashCode,toStringWorkflowDefkey(),getKey(),containsType(),getNextTask(),getTaskByRefName(),collectTasks()Tests
TaskDefTest— 32 tests covering: default field values, version null→1 defaulting, equals/hashCode (reflexive, symmetric, null, different class, same PK with differing fields, different name/version, hashCode consistency),HashSetdedup, null-coalescing getters, all@Min/@NotEmpty/timeout constraint validations, and edge cases.WorkflowDefValidatorTest— 16 new tests added alongside existing validation tests: default values, equals/hashCode,HashSetdedup,key()/getKey()correctness,toStringcontent.Test plan
./gradlew :conductor-common:test— all 32 TaskDef + 16 WorkflowDef new tests pass./gradlew :conductor-grpc:compileJava— proto mapper compiles cleanlyTaskDefwith noversionset equals one withversion=1explicitlyTaskDefs with the same name+version but differentretryCount/ownerEmailare considered equal (PK only)🤖 Generated with Claude Code