Guard EntityFactory against null ResultSet and -1 id sentinel#142
Merged
Conversation
EntityFactory omitted two safety checks that EnvironmentFactory performs: - getNextEntityId() called rs.next() without a null guard, so a failed nextval query (DbInteractions.query returns null on failure) threw NPE instead of returning -1. Added `if (rs == null) return -1;`. - createEntity() never checked the -1 sentinel before building the INSERT, so on failure it would INSERT a row with entity_id = -1. Added an `if (id == -1)` guard that logs and throws EntityCreationException, matching EnvironmentFactory. Adds EntityFactoryTest (mocks DbInteractions): success path plus the two guarded cases, which assert no INSERT (dbInteractions.update) is attempted. Both guard tests fail without the fix (NPE / INSERT-with-(-1)). Closes #138 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Summary
EntityFactoryimplemented the same "get next id from a sequence, then INSERT" pattern asEnvironmentFactorybut omitted two safety checks the latter performs, leaving two defects on the live entity-creation path (POST /api/v1/entities/{name}andDebugController):ResultSet→ NPE.getNextEntityId()calledrs.next()without checkingrs.DbInteractions.query(...)returnsnullon failure, so a failed/locked query threwNullPointerExceptioninstead of returning-1. Fixed withif (rs == null) return -1;(matchesEnvironmentFactory).-1sentinel → bad INSERT.createEntity()never checkedid == -1before building the INSERT, so on failure it would write a row withentity_id = -1. Fixed with anif (id == -1)guard that logs and throwsEntityCreationException(matchesEnvironmentFactory).Tests
Adds
EntityFactoryTest(mocksDbInteractions, mirroring the repository tests'@SpringBootTest+@MockBeanstyle):update/INSERT is attempted;-1) → throws, and asserts noupdate/INSERT is attempted;Both guard tests were confirmed to fail without the fix (NPE on the null case; the
-1case attempts the INSERT and trips thenever().update(...)verification).Test plan
./mvnw test— 220/220 pass (was 216; +4), JDK 21EntityFactory.javatomainmakes the two guard tests failsrc/main/pythonchange)Closes #138
🤖 Generated with Claude Code