FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: Dataloader helper Try classcast exception by ameerabdul · Pull Request #177 · graphql-java/java-dataloader · GitHub

/ java-dataloader Public

fix: Dataloader helper Try classcast exception - #177

Open
ameerabdul wants to merge 1 commit into
graphql-java:masterfrom
ameerabdul:ameerabdul-try-classcast
ConversationCommits (1)ChecksFiles changed
Open

fix: Dataloader helper Try classcast exception#177
ameerabdul wants to merge 1 commit into
graphql-java:masterfrom
ameerabdul:ameerabdul-try-classcast

Conversation

ameerabdul commented Mar 10, 2025 •
edited
Loading

Copy link
Copy Markdown

Summary

  • Creating a new DataLoader with newMappedDataLoaderWithTry and batchingEnabled as false and cachingEnabled as true.
  • This will cause the value to which is already a Try instance to be wrapped again in another Try.succeed causing class cast exception

ameerabdul marked this pull request as ready for review March 10, 2025 17:28
ameerabdul commented Mar 10, 2025
View reviewed changes
Comment thread src/main/java/org/dataloader/DataLoaderHelper.java
valuesInKeyOrder.set(listIndex, Try.succeeded(v));
}
}
List<V> assembledValues = valuesInKeyOrder.stream().map(Try::get).collect(toList());

ameerabdul Mar 10, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

this might be a bigger change as I can't go a Try.get on failed instances

bbakerman commented Mar 11, 2025

Copy link
Copy Markdown
Member

Can we please have a unit test showing this working.

Its not enough to fix the code, we need to lock in that behavior

bbakerman commented Apr 2, 2025

Copy link
Copy Markdown
Member

I managed to do a reproducton of this. I can confirm its a genuine problem but this is not the fix for it

github-actions Bot commented Jun 2, 2025

Copy link
Copy Markdown

Hello, this pull request has been inactive for 60 days, so we're marking it as stale. If you would like to continue working on this pull request, please make an update within the next 30 days, or we'll close the pull request.

github-actions Bot added the Stale label Jun 2, 2025

github-actions Bot commented Jul 3, 2025

Copy link
Copy Markdown

Hello, as this pull request has been inactive for 90 days, we're closing this pull request. We always welcome contributions, and if you would like to continue, please open a new pull request.

github-actions Bot closed this Jul 3, 2025
bbakerman reopened this Jul 3, 2025
github-actions Bot removed the Stale label Jul 4, 2025
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

keep-open

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL