Update faraday to 2.12, remove deprecated faraday_middleware gem

[eikes]

No description provided.

[eikes]

This error can not be raised in this spec anymore. We are stubbing the response which Faraday returns after the middleware is applied. So no JSON parsing takes place anymore.

[eikes]

When a response is an empty string, it will not be cast to nil anymore, so we check for empty? or nil?

[phylor]

@eikes I quickly tested this in the two projects I'm working on and I do have failing tests. So there seems to be a behavioral change. Let me know if this PR is review ready, I'll then check what's going on!

[eikes]

@phylor Yes, it's ready for review, using an outdated Faraday version was a problem for me. Please let me know how or what is failing. I can help debug if you want me to.

[phylor]

@phylor Yes, it's ready for review, using an outdated Faraday version was a problem for me. Please let me know how or what is failing. I can help debug if you want me to.

It is a testing issue. I'm not so sure if this will ever happen in production code.

The behavioral change is that we now return nil from create/update/delete API methods, when the HTTP response is {}. Before, we initialized an empty model (e.g. we called Ioki::Model::Operator::Vehicle.new({})).

If we were defensive, we could change the condition to something like return if parsed_response.nil? || (parsed_response.is_a?(String) && parsed_response.empty?).

I'm not sure how often that happens in real code. It happens in our test code as we return an empty hash in mocked API calls when we're not interested in the response. We used to get an empty model back, i.e. we could still call all methods on it (they do return nil then). The new behavior is that the response is nil and we can't call methods on it.

I'm also not sure if it's more logical/correct to return nil instead of an empty model. Maybe it is?

I thought about what happens when the API returns []. But I believe the relevant endpoints do not support arrays anyways.

I don't have a strong opinion - I'm slightly leaning towards your proposed solution, as returning nil instead of an empty model seems to be more correct.

@eikes What do you think?

[eikes]

@phylor Two things come to mind:

1. We should define the behaviour and create a test for it in this repository which makes sure that it works as defined.
2. I think that an empty JSON object {} is still a valid response and should in fact be turned into an instance of the ruby class the user expects, even if all attributes are empty.

[phylor]

@phylor Two things come to mind:

1. We should define the behaviour and create a test for it in this repository which makes sure that it works as defined.
2. I think that an empty JSON object {} is still a valid response and should in fact be turned into an instance of the ruby class the user expects, even if all attributes are empty.

Okay, then let's agree that we don't change the existing behavior. At least not for the empty hash.

[phylor]

@eikes I changed a couple of things to make the update work without breaking existing behavior.

Our behavior is inconsistent though. It depends on what the response is. If the response is invalid JSON, we return nil, otherwise an empty model. We should probably fix that at some point, but it would be a breaking change.

[eikes]

@phylor Thank you for working on this! I am fine with the changes as they are now. Please feel free to merge this.