Fixed exception in SharedDriverLogicHandler::exchangeRate()

[neochief]

Hi!

I'm getting "Return value must be of type array|float, string returned" from SharedDriverLogicHandler::exchangeRate()

This is the fix for this problem.

[ash-jc-allen]

Hey @neochief! Thanks for picking up on this and submitting a fix for it 😄

Please could you let me know which driver you were using and any steps to reproduce the error? I just want to double-check I've not missed this off anywhere else too.

[neochief]

Hi!

Sure, I've used the 'exchange-rates-data-api' driver. I use laravel 9.52. I simply call this two times, and get the error:

ExchangeRate::convert(10, 'EUR', 'USD', \Carbon\Carbon::now())

The issue is related to this problem: laravel/framework#38933

I use redis cache store. I checked that your code indeed tries to save the float to cache, but then retrieves string.

[ash-jc-allen]

Hey @neochief! Sorry for the delay in getting this sorted. I've been super busy over the past week, but I'm catching up on things now 🙂

Thanks for the steps to reproduce this bug. I managed to recreate the issue straight away with them!

The package also supports the functionality for you to get the exchange rates and conversions for multiple currency pairs at once like so:

ExchangeRate::convert(10, 'EUR', ['USD', 'GBP'], \Carbon\Carbon::now());

Running that would return something like:

[
    'USD' => 12.24,
    'EUR' => 11.54661,
]

I think your proposed (float) would break that part of the feature. It seemed to cast the array to 1.00 as a float. So I pushed a quick change to your fix that should hopefully only cast to a float if we're retrieving a single exchange rate. If we're fetching multiple rates, it should leave the array as-is.

If possible, would you be able to have a quick check of this and make sure my proposed change still solves your issue? I'd really appreciate it, but it's totally cool if not 😄

If everything looks good, I'll get it merged and released ASAP!

---

P.S. - I'm a huge fan of your work! I use Refactoring Guru all the time and I've referenced it in a few of my blog posts before too! 😄

[neochief]

Yeah, that makes sense. Sorry, I didn't have the time to debug the whole thing, I thought you cache everything on the currency level, so that you can construct such an array from the currencies you already have in cache. In any case, I tested your solution and can confirm that it works for both single currency and the array.

And by the way, I'm flattered that you know about my project. I'm glad that my work was helpful to you ☺️ Kudos and good luck!

[ash-jc-allen]

Hey @neochief! Apologies for taking so long to get this merged in. This week has been crazy busy so I've not had much time to get stuck in. I'm getting a release built now, so I should have this available for installing tonight 😄

[ash-jc-allen]

This should be available in v7.0.1 of the package now (https://github.com/ash-jc-allen/laravel-exchange-rates/releases/tag/v7.0.1).

Thanks again for the help on this, I appreciate it! 🚀