kainxspirits / laravel-pubsub-queue

A Laravel queue driver for Google PubSub.
MIT License
45 stars 36 forks source link

remove base64_encode/decode of the payload. #78

Open Sergio-Ben opened 8 months ago

Sergio-Ben commented 8 months ago

Google lib, which is used underneath is already properly encoding/decoding the payload. There is no need to do it twice. Here is some screenshots with the proofs :) Feel free to dive into the google pub/sub lib and check it on your own :)

Screenshot 2024-03-26 at 15 40 07

As it is the non-compatible change, i suggest bumping up major version during releasing

kainxspirits commented 6 months ago

Thank you for the contribution.

Even if it not a huge library, it will break for existing users (there is no major version yet). I feel like it would be better to use the encode property of the Google lib in order to make the encoding optional.

Sergio-Ben commented 6 months ago

@kainxspirits The problem here is the encode property is something you cannot set from the application level.

Screenshot 2024-05-07 at 09 48 21

It's just an internal flag, because Google pub/sub lib could use 2 types of transport, http or grpc. And encode will always be true for the default http transport.

Also with current approach the message size is much bigger than expected. Encoding message with base64 resulting in increasing payload size by ~ 33%. With the current approach it's even bigger $res = ($mesSize 1.33) 1.33 So double encoding will produce ~ 76% bigger payload from original size.

Also, it's worth mentioning the potential release version.

[https://getcomposer.org/doc/articles/versions.md#caret-version-range-] For pre-1.0 versions it also acts with safety in mind and treats ^0.3 as >=0.3.0 <0.4.0 and ^0.0.3 as >=0.0.3 <0.0.4.

So, even if the next version will be 0.10.0 the composer install command won't fetch the latest version.

Obviously, the release is up to you. Just wanted to highlight the problem I found using this lib.