AbstractMicroserviceHttpRepository::delete status code wrongly handled

[mtarld]

The AbstractMicroserviceHttpRepository::delete is dealing with the status code in the wrong way. < 500, < 400, [...] is in the wrong order IMHO. We'll always have a server exception no matter the status code value.

We must update that and add related tests.

[ismail1432]

👋 Sorry I don't get what do you mean by "wrong order" and "We'll always have a server exception"?

[mtarld]

Sorry if I wasn't clear enough ^^ Here is a part of the delete method:

$response = $this->request('DELETE', $this->buildUri($iri, $additionalQueryParams));
$statusCode = $response->getStatusCode();

if (500 <= $statusCode) {
    throw new ServerException($response);
}

if (400 <= $statusCode) {
    if (null !== $violations = $this->createConstraintViolationListFromResponse($response)) {
        throw new ResourceValidationException($resource, $violations);
    }

    throw new ClientException($response);
}

if (300 <= $statusCode) {
    throw new RedirectionException($response);
}

~As you can see, we always enter in the first if, which will result in ServerException~

EDIT: Wow I read it the wrong way... We may make it more readable and moreover add more tests then

[ismail1432]

I'm not convinced for "We may make it more readable"

The same logic is used here => https://github.com/symfony/http-client/blob/5.3/Response/CommonResponseTrait.php#L172

[mtarld]

Closed thanks to https://github.com/mtarld/api-platform-ms-bundle/pull/69