Fix ActiveJob arguments serialization to correctly serialize String subclasses having custom serializers #50090
Conversation
This file contains 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
byroot
reviewed
Nov 17, 2023
| @@ -72,8 +72,14 @@ def deserialize(arguments) | |||
|
|
|||
| def serialize_argument(argument) | |||
| case argument | |||
| when *PERMITTED_TYPES | |||
There was a problem hiding this comment.
PERMITTED_TYPES is now only used in deserialize_argument, perhaps we can get rid of it.
byroot
approved these changes
Nov 17, 2023
…ubclasses having custom serializers Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
fatkodima
force-pushed
the
fix-active_job-serialization-string-subclasses
branch
from
59aa462
to
14578ea
Compare
4 tasks
octokatherine
added a commit
to octokatherine/rails
that referenced
this pull request
Nov 20, 2023
rails#50090 broke serialization of String subclasses that don't have serializers, like ActiveSupport::SafeBuffer. Co-authored-by: John Hawthorn <john@hawthorn.email>
jarenas9539
added a commit
to jarenas9539/rails
that referenced
this pull request
Nov 21, 2023
… of String After changes from rails#50090 and rails#50122, a redundant if statement remains on Arguments#serialize on String related classes serialization. When adding the begin/rescue statement on the else clause the if clause began to be scoped on the rescue clause as StringSerializer is not defined and will throw SerializationError. As if clause and rescue clause share same result, the if clause started to be redundant.
4 tasks
byroot
added a commit
that referenced
this pull request
Dec 2, 2023
…-string-subclasses Fix ActiveJob arguments serialization to correctly serialize String subclasses having custom serializers
|
Backported to 7-1-stable as 49ac7e5 |
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 #50076 (look there for a lengthy discussion).
Should we also raise a deprecation warning for String subclasses without custom serializers? It worked before this change.
cc @byroot