Repository navigation
AuthorName: treat blank middle name as absent instead of throwing - #14
Conversation
Identities stored with an empty-string middleName (e.g. mah4006) crash the 3-arg constructor: middleName.trim().substring(0, 1) throws StringIndexOutOfBoundsException for "" or whitespace-only input. A blank middle name now takes the same path as null (middleName "", middleInitial ""). setMiddleName also derives the initial from the trimmed value so a whitespace-only middle name no longer stores a blank initial. Null and non-blank behavior is unchanged. Bump version to 3.0.3.
Observed on reciter-dev: PubMed article author data can carry an empty-string first name, which crashed the firstInitial derivation the same way a blank middle name did (StringIndexOutOfBoundsException at the substring).
|
Pushed a second commit extending the same guard to a blank first name: live testing on reciter-dev showed the identical |
| if (middleName == null) { | ||
| // A blank middle name (e.g. "" stored on an Identity in DynamoDB) is treated the | ||
| // same as null; substring(0, 1) on the trimmed value would otherwise throw. | ||
| if (middleName == null || middleName.trim().isEmpty()) { |
There was a problem hiding this comment.
Please consider using isBlank() handles both "" and whitespace-only strings.
There was a problem hiding this comment.
Done, both guards now use isBlank(). Re-verified against the rebuilt jar: blank and whitespace-only names take the null path, normal and null behavior unchanged.
| if (firstName == null) { | ||
| // A blank first name gets the same treatment as null (observed in PubMed | ||
| // article author data); substring(0, 1) on the trimmed value would throw. | ||
| if (firstName == null || firstName.trim().isEmpty()) { |
There was a problem hiding this comment.
Please consider using isBlank() handles both "" and whitespace-only strings.
There was a problem hiding this comment.
Done here as well (0ad20ef) — both guards use isBlank() now.
mrj4001
left a comment
mrj4001
left a comment
There was a problem hiding this comment.
Please address the code review comments.
|
Thanks for merging. Could you also run the Central publish for 3.0.3 when you get a chance? Once it's on repo1 I'll open the one-line pom bump (3.0.2 → 3.0.3) in ReCiter on both the development and Corretto17 lines. No rush — the ReCiter-side guards already deployed cover the known crash paths; this release is the constructor-level fix for any other AuthorName call sites. |
AuthorName: treat blank middle name as absent instead of throwing Incorporate the changes from PR #14.
2 participants
Problem
The 3-arg
AuthorNameconstructor guards onlymiddleName == nullbefore computingmiddleName.trim().substring(0, 1), so an empty-string middle name throwsStringIndexOutOfBoundsException: begin 0, end 1, length 0. Identities withmiddleName: ""exist in DynamoDB (e.g.mah4006, Manuel Hidalgo Medina). In ReCiter's retrieval engine this crash kills the retrieval worker silently (ForkJoinPool.executestores the exception and never rethrows), leaving the person permanently unretrievable: 353 accepted PMIDs, zero-feature HTTP 200 responses in 0.16s on every attempt, andESearchResultnever populated. Reproduced against the released 3.0.2 jar:new AuthorName("Manuel", "", "Hidalgo")→ crash; null and normal middle names are fine.Fix
null(middleName/middleInitialboth"").setMiddleName: derive the initial from the trimmed value so a whitespace-only input yields""rather than a blank-character initial. Deliberately still stores""(not null) to preserve DynamoDB unconvert semantics for existing identities.Verification
The repo has no test harness, so verified with a compiled check harness against the built 3.0.3 jar: 21/21 assertions pass — blank and whitespace middle names no longer throw and match the null path; null/non-blank behavior is byte-identical;
variants()counts unchanged. Control run against the 3.0.2 jar reproduces the exact production exception. Audited everysubstringinitial-derivation site in the class; the two changed above are the only hazards for middle names.Rollout
Publish 3.0.3 (central-publishing plugin already configured), then bump the dependency in ReCiter's Corretto17 line. A companion ReCiter PR guards the retrieval engine independently, so this release is defense-in-depth for other
AuthorNameconstruction sites rather than a deploy blocker.