Edujugon / PushNotification

PHP and Laravel Package to send push notifications to Android and IOS devices.
MIT License
478 stars 159 forks source link

bug in getFeedback() #63

Closed mohammad-mazraeshahi closed 5 years ago

mohammad-mazraeshahi commented 5 years ago

hi @Edujugon

In getFeedback (), the return value of the multicast_id is double, for example, 7.20839961657612e + 18, which is the wrong value.

Please change multicast_id to string:

class Gcm extends PushService implements PushServiceInterface
{
public function send(array $deviceTokens,array $message)
    {
        $fields = $this->addRequestFields($deviceTokens,$message);
        $headers = $this->addRequestHeaders();
        try
        {
            $result = $this->client->post(
                $this->url,
                [
                    'headers' => $headers,
                    'json' => $fields,
                ]
            );

            $json = $result->getBody();

          //here changed
          //old code $this->setFeedback(json_decode($json));
             $this->setFeedback(json_decode($json , false , 512 , JSON_BIGINT_AS_STRING));

            return $this->feedback;

        }catch (\Exception $e)
        {
            $response = ['success' => false, 'error' => $e->getMessage()];

            $this->setFeedback(json_decode(json_encode($response), FALSE));

            return $this->feedback;
        }
    }
}
Edujugon commented 5 years ago

Hi @mohammad-mazraeshahi ,

I'm not sure if I'm getting your point.

Are you suggesting to replace this:

$this->setFeedback(json_decode($json));

to this:

$this->setFeedback(json_decode($json , false , 512 , JSON_BIGINT_AS_STRING));
mohammad-mazraeshahi commented 5 years ago

I will record all FCM returns. I'm sure there is a need for this change. Please apply this change:

$this->setFeedback(json_decode($json , false , 512 , JSON_BIGINT_AS_STRING));

Edujugon commented 5 years ago

Could you please elaborate a bit more the issue you're facing?

The code change you're suggesting seems to use the default parameters in json_decode function. Isn't that exactly the same as the current code?

Could you provide an example of the output with both codes?

Thank you.

mohammad-mazraeshahi commented 5 years ago

tempsnip png

As shown in the above image, the multicast_id return value is a very large number The json_decode () function converts this value to a number such as the following number 6.26174813181673e+48

By inserting the following code, the amount is converted to String and prevents this from happening $this->setFeedback(json_decode($json , false , 512 , JSON_BIGINT_AS_STRING));

output:

multicast_id: "6261748131816737148"

Edujugon commented 5 years ago

Could you dd that $json before calling the json_decode ? I think that "number" (6.26174813181673e+48) is already there. So there is nothing about the json_decode function.

Let me know the result

mohammad-mazraeshahi commented 5 years ago

Show your code output using by Xdebug: tempsnip1 png

Output with the following changes: $this->setFeedback(json_decode($json , false , 512 , JSON_BIGINT_AS_STRING));

tempsnip2 png

Edujugon commented 5 years ago

Hi @mohammad-mazraeshahi ,

Thank you for the provided information. I'm gonna create a new package version fixing this issue.

Shall I update you when released.

Edujugon commented 5 years ago

Hi @mohammad-mazraeshahi ,

I've just released a new package version(v3.0.5) which should fix this issue.

Thanks again for reporting it and providing detailed information.

mohammad-mazraeshahi commented 5 years ago

There is also a problem posting to the topic. Please change the file below:

class Fcm extends Gcm
{

    public function sendByTopic($topic, $message, $isCondition = false)
    {
        $headers = $this->addRequestHeaders();
        $data = $this->buildData($topic, $message, $isCondition);

        try {
            $result = $this->client->post(
                $this->url,
                [
                    'headers' => $headers,
                    'json' => $data,
                ]
            );

            $json = $result->getBody();

// chenge it
// old            $this->setFeedback(json_decode($json));
// new
            $this->setFeedback(json_decode($json , false , 512 , JSON_BIGINT_AS_STRING));

        } catch (\Exception $e) {
            $response = ['success' => false, 'error' => $e->getMessage()];

            $this->setFeedback(json_decode(json_encode($response)));

        } finally {
            return $this->feedback;
        }
    }
}
Edujugon commented 5 years ago

Hey @mohammad-mazraeshahi ,

Thanks again for noticing that. I've just released a new package version that fixes the issue.