Repository navigation
Conversation
…omen-Coding-Community#747) findByMemberId/findById caught every DataAccessException and returned Optional.empty(). Two concurrent uploads could therefore leave two member_profile_picture rows; every later read threw IncorrectResultSizeDataAccessException, which was swallowed, so the member's picture silently and permanently vanished from all responses. - Narrow the catch to EmptyResultDataAccessException only, so genuine DB errors (duplicate rows, connection failures) propagate instead of being masked as 'no picture'. - create() now upserts on member_id (ON CONFLICT DO UPDATE), and a new Flyway migration adds a UNIQUE constraint on member_id, so concurrent uploads can no longer leave duplicate rows behind. - Tests: duplicate-row reads now assert the error propagates instead of returning empty; create() asserts the upsert SQL.
p3xz
requested review from
OlenaReukova,
Sowmiya07,
Tooonia,
dricazenck,
lauracabtay,
nora-weisser and
nverbic
October 5, 2026 22:05
|
womencodingcommunity
left a comment
womencodingcommunity
left a comment
Contributor
There was a problem hiding this comment.
Automated review
dricazenck
reviewed
Oct 12, 2026
| return Optional.ofNullable(profilePicture); | ||
| } catch (DataAccessException e) { | ||
| } catch (EmptyResultDataAccessException e) { | ||
| // No row for this resource id: genuinely absent, not a swallowed DB error. |
Collaborator
There was a problem hiding this comment.
this inline comment should be part of the javadoc.
dricazenck
reviewed
Oct 12, 2026
| return Optional.ofNullable(profilePicture); | ||
| } catch (DataAccessException e) { | ||
| } catch (EmptyResultDataAccessException e) { | ||
| // No row for this member: genuinely absent, not a swallowed DB error. |
Collaborator
There was a problem hiding this comment.
same as previous comment, please move to javadoc instead of inline comment.
dricazenck
approved these changes
Oct 12, 2026
dricazenck
left a comment
dricazenck
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the contribution, please consider the small comment to move info to javadoc instead of inline comment. This keeps the code cleaner and easier to maintain and read.
This branch has not been deployed
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.
Fixes #747
Bug:
PostgresMemberProfilePictureRepository.findByMemberId/findByIdcaught everyDataAccessExceptionand returnedOptional.empty(). Two concurrent profile-picture uploads could race between the read and the insert, leaving twomember_profile_picturerows for one member. Every later read then threwIncorrectResultSizeDataAccessException, which was swallowed to empty, so the member's picture silently and permanently vanished from all responses (and the cleanup path inResourceService.uploadProfilenever fired).Fix (3 parts):
findByMemberId/findByIdnow catch onlyEmptyResultDataAccessException(genuinely no row). Real DB errors, including duplicate-row reads, propagate instead of being masked as "no picture".create()is now an upsert:INSERT ... ON CONFLICT (member_id) DO UPDATE SET resource_id = EXCLUDED.resource_id, so a concurrent re-upload replaces the row instead of duplicating it.V40adds aUNIQUEconstraint onmember_idas the schema-level guarantee.Verification:
javac(Gradle daemon can't run in this sandbox); PMD (repo'scustom-ruleset.xml/custom-ruleset-test.xml) reports no violations on the changed files.PostgresMemberProfilePictureRepositoryTest: 10/10 pass. Before/after check: the 3 new tests (duplicate-row error propagates for both reads; upsert SQL containsON CONFLICT (member_id)) fail against the old code (3 failed, 7 passed) and pass with this fix.