Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

class_serializers supports closures #1739

Closed
wants to merge 3 commits into from
Closed

Conversation

tw2066
Copy link

@tw2066 tw2066 commented May 14, 2024

class_serializers supports closures

eg.

    'class_serializers' => [
        'callable' => static function (object $object) {
            if(\Hyperf\Stringable\Str::startsWith($object::class,'HyperfExample')){
                return json_decode(json_encode($object),true);
            }
            return $object;
        }
    ],

@stayallive
Copy link
Collaborator

stayallive commented May 14, 2024

This doesn't do what you think it does, have you tested your change? Your change currently will run the serializer for any type and is a big breaking change and also not how class_serializers is documented to work.

The serializer already can be a closure just like we do in our test cases:

$serializer = $this->createSerializer(new Options([
'class_serializers' => [
$objectClass => static function ($object): array {
return [
'purpose' => $object->getPurpose(),
];
},
],
]));

The key in class_serializers must match the class your callback is serializing, so "function" in your example probably won't work but needs to be some (base) class available in your application.

@tw2066
Copy link
Author

tw2066 commented May 14, 2024

Consider making a special judgment on type

Copy link
Member

@cleptric cleptric left a comment

Choose a reason for hiding this comment

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

This needs tests and I'm currently not convenient why this change is needed in the first place.

@tw2066
Copy link
Author

tw2066 commented May 14, 2024

Some classes are not inherited,unified processing can be achieved through namespaces or other judgments

@stayallive
Copy link
Collaborator

When using our ClientBuilder directly you can set your own serializer (that extends the SDK one potentially) in which you can do whatever you want:

public function setRepresentationSerializer(RepresentationSerializerInterface $representationSerializer): self
{
$this->representationSerializer = $representationSerializer;
return $this;
}

This current approach doesn't make sense to me, class_serializers with a magic key where the key is not a class name is going to cause much confusion. So either a custom serializer is the way to go here or creating a whole new option (object_serializer maybe) to support a catch-all serializer for objects could make sense but then we might want to create a more generic option to serialize more types.

Anyway, using the client builder seems like the best bet here for your specific needs?

@cleptric
Copy link
Member

I agree with @stayallive on this.

@cleptric cleptric closed this May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants