Skip to content

Commit

Permalink
Return 204 No Content on successful DELETE operations
Browse files Browse the repository at this point in the history
Signed-off-by: alexmerlin <[email protected]>
  • Loading branch information
alexmerlin committed Jun 1, 2024
1 parent e78a790 commit c61816e
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/Admin/src/Handler/AdminHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function delete(ServerRequestInterface $request): ResponseInterface

$this->adminService->deleteAdmin($admin);

return $this->infoResponse(Message::ADMIN_DELETED);
return $this->noContentResponse();
}

/**
Expand Down
28 changes: 19 additions & 9 deletions src/App/src/Handler/ResponseTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ trait ResponseTrait
protected HalResponseFactory $responseFactory;

Check warning on line 19 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined class

Undefined class 'HalResponseFactory'
protected ResourceGenerator $resourceGenerator;

Check warning on line 20 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined class

Undefined class 'ResourceGenerator'

public function emptyResponse(int $status = StatusCodeInterface::STATUS_NO_CONTENT): ResponseInterface

Check warning on line 22 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined class

Undefined class 'StatusCodeInterface'
{
return new EmptyResponse($status, ['Content-Type' => 'text/plain']);

Check warning on line 24 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Incompatible return type

Return value type is not compatible with declared

Check warning on line 24 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined class

Undefined class 'EmptyResponse'
}

public function jsonResponse(
array|string $messages = [],
int $status = StatusCodeInterface::STATUS_OK

Check warning on line 29 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined class

Undefined class 'StatusCodeInterface'
): ResponseInterface {

Check warning on line 30 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined class

Undefined class 'ResponseInterface'
return new JsonResponse($messages, $status);
}

public function createResponse(ServerRequestInterface $request, mixed $instance): ResponseInterface
{
return $this->responseFactory->createResponse(
Expand All @@ -34,9 +46,14 @@ public function createdResponse(ServerRequestInterface $request, mixed $instance
return $response->withStatus(StatusCodeInterface::STATUS_CREATED);

Check warning on line 46 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined class

Undefined class 'StatusCodeInterface'
}

public function noContentResponse(): ResponseInterface

Check warning on line 49 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined class

Undefined class 'ResponseInterface'
{
return $this->emptyResponse();
}

public function notFoundResponse(): ResponseInterface
{
return new EmptyResponse(StatusCodeInterface::STATUS_NOT_FOUND, ['Content-Type' => 'text/plain']);
return $this->emptyResponse(StatusCodeInterface::STATUS_NOT_FOUND);

Check warning on line 56 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined class

Undefined class 'StatusCodeInterface'
}

public function errorResponse(
Expand All @@ -61,14 +78,7 @@ public function infoResponse(
], $status);
}

public function jsonResponse(
array|string $messages = [],
int $status = StatusCodeInterface::STATUS_OK
): ResponseInterface {
return new JsonResponse($messages, $status);
}

public function notAcceptedResponse(string $message): ResponseInterface
public function notAcceptableResponse(string $message): ResponseInterface
{
return $this->errorResponse($message, StatusCodeInterface::STATUS_NOT_ACCEPTABLE);

Check warning on line 83 in src/App/src/Handler/ResponseTrait.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined class

Undefined class 'StatusCodeInterface'
}
Expand Down
10 changes: 0 additions & 10 deletions src/App/src/Message.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,8 @@
class Message
{
public const ADMIN_CREATED = 'Admin account has been created.';
public const ADMIN_DELETED = 'Admin account has been deleted.';
public const ADMIN_NOT_ACTIVATED = 'This account is deactivated.';
public const ADMIN_ROLE_MISSING = 'Admin role \'%s\' is missing.';
public const AVATAR_CREATED = 'Avatar has been successfully created.';
public const AVATAR_DELETED = 'Avatar has been successfully deleted.';
public const AVATAR_MISSING = 'This user account has no avatar associated with it.';
public const DUPLICATE_EMAIL = 'An account with this email address already exists.';
public const DUPLICATE_IDENTITY = 'An account with this identity already exists.';
Expand All @@ -21,18 +18,14 @@ class Message
public const INVALID_ACTIVATION_CODE = 'Invalid activation code.';
public const INVALID_CLIENT_ID = 'Invalid client_id.';
public const INVALID_CONFIG = 'Invalid configuration value: \'%s\'';
public const INVALID_EMAIL = 'Invalid email.';
public const INVALID_VALUE = 'The value specified for \'%s\' is invalid.';
public const INVALID_IDENTIFIER = 'User cannot be found by supplied identifier.';
public const MAIL_SENT_RECOVER_IDENTITY = 'If the provided email identifies an account in our system, '
. 'you will receive an email with your account\'s identity.';
public const MAIL_SENT_RESET_PASSWORD = 'If the provided email identifies an account in our system, '
. 'you will receive an email with further instructions on resetting your account\'s password.';
public const MAIL_SENT_USER_ACTIVATION = 'User activation mail has been successfully sent to \'%s\'';
public const MAIL_NOT_SENT = 'Could not send mail.';
public const MAIL_NOT_SENT_TO = 'Could not send mail to \'%s\'.';
public const MISSING_CONFIG = 'Missing configuration value: \'%s\'.';
public const MISSING_PARAMETER = 'Missing parameter: \'%s\'';
public const NOT_FOUND_BY_NAME = 'Unable to find %s identified by name: %s';
public const NOT_FOUND_BY_UUID = 'Unable to find %s identified by UUID: %s';
public const RESET_PASSWORD_EXPIRED = 'Password reset request for hash: \'%s\' is invalid (expired).';
Expand All @@ -50,11 +43,8 @@ class Message
public const USER_NOT_FOUND_BY_EMAIL = 'Could not find account identified by email \'%s\'';
public const USER_NOT_FOUND_BY_IDENTITY = 'Could not find account by identity \'%s\'';
public const VALIDATOR_MIN_LENGTH = '%s must be at least %d characters long.';
public const VALIDATOR_MAX_LENGTH = '%s cannot be longer than %d characters.';
public const VALIDATOR_MIN_MAX_LENGTH = '%s length must be between %d and %d characters.';
public const VALIDATOR_PASSWORD_MISMATCH = 'Password confirmation does not match the provided password.';
public const VALIDATOR_REQUIRED_FIELD = 'This field is required and cannot be empty.';
public const VALIDATOR_REQUIRED_FIELD_BY_NAME = '%s is required and cannot be empty.';
public const VALIDATOR_SKIP_OR_FILL = 'If this field is specified, then it must be filled in.';
public const VALIDATOR_REQUIRED_UPLOAD = 'A file must be uploaded first.';
}
8 changes: 4 additions & 4 deletions src/App/src/Middleware/ContentNegotiationMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public function process(

$accept = $this->formatAcceptRequest($request->getHeaderLine('Accept'));
if (! $this->checkAccept($routeName, $accept)) {
return $this->notAcceptedResponse('Not Acceptable');
return $this->notAcceptableResponse('Not Acceptable');
}

$contentType = $request->getHeaderLine('Content-Type');
Expand All @@ -58,7 +58,7 @@ public function process(

$responseContentType = $response->getHeaderLine('Content-Type');
if (! $this->validateResponseContentType($responseContentType, $accept)) {
return $this->notAcceptedResponse('Unable to resolve Accept header to a representation');
return $this->notAcceptableResponse('Unable to resolve Accept header to a representation');
}

return $response;
Expand All @@ -80,7 +80,7 @@ public function checkAccept(string $routeName, array $accept): bool
}

$acceptList = $this->config['default']['Accept'] ?? [];
if (isset($this->config[$routeName])) {
if (! empty($this->config[$routeName]['Accept'])) {
$acceptList = $this->config[$routeName]['Accept'] ?? [];
}

Expand All @@ -98,7 +98,7 @@ public function checkContentType(string $routeName, string $contentType): bool
}

$acceptList = $this->config['default']['Content-Type'] ?? [];
if (isset($this->config[$routeName])) {
if (! empty($this->config[$routeName]['Content-Type'])) {
$acceptList = $this->config[$routeName]['Content-Type'] ?? [];
}

Expand Down
2 changes: 1 addition & 1 deletion src/User/src/Handler/AccountAvatarHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function delete(ServerRequestInterface $request): ResponseInterface

$this->userAvatarService->removeAvatar($user);

return $this->infoResponse(Message::AVATAR_DELETED);
return $this->noContentResponse();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/User/src/Handler/AccountHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ public function __construct(
*/
public function delete(ServerRequestInterface $request): ResponseInterface
{
$user = $this->userService->deleteUser($request->getAttribute(User::class));
$this->userService->deleteUser($request->getAttribute(User::class));

return $this->createResponse($request, $user);
return $this->noContentResponse();
}

public function get(ServerRequestInterface $request): ResponseInterface
Expand Down
2 changes: 1 addition & 1 deletion src/User/src/Handler/UserAvatarHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function delete(ServerRequestInterface $request): ResponseInterface

$this->userAvatarService->removeAvatar($user);

return $this->infoResponse(Message::AVATAR_DELETED);
return $this->noContentResponse();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/User/src/Handler/UserHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ public function delete(ServerRequestInterface $request): ResponseInterface
throw new NotFoundException(sprintf(Message::NOT_FOUND_BY_UUID, 'user', $uuid));
}

$user = $this->userService->deleteUser($user);
$this->userService->deleteUser($user);

return $this->createResponse($request, $user);
return $this->noContentResponse();
}

/**
Expand Down
7 changes: 6 additions & 1 deletion test/Functional/AbstractFunctionalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ protected function put(
protected function delete(
string $uri,
array $queryParams = [],
array $headers = ['Accept' => 'application/json'],
array $headers = ['Accept' => 'application/json, text/plain'],
array $cookies = []
): ResponseInterface {
$request = $this->createRequest(
Expand Down Expand Up @@ -344,6 +344,11 @@ protected function assertResponseGone(ResponseInterface $response): void
$this->assertSame(StatusCodeInterface::STATUS_GONE, $response->getStatusCode());

Check warning on line 344 in test/Functional/AbstractFunctionalTest.php

View workflow job for this annotation

GitHub Actions / Qodana for PHP

Undefined method

Method 'assertSame' is undefined
}

protected function assertResponseNoContent(ResponseInterface $response): void
{
$this->assertSame(StatusCodeInterface::STATUS_NO_CONTENT, $response->getStatusCode());
}

protected function assertBetween(int $value, int $from, int $to): void
{
$this->assertThat(
Expand Down
4 changes: 2 additions & 2 deletions test/Functional/AdminTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ public function testAdminCanDeleteAdminAccount(): void

$response = $this->delete('/admin/' . $admin->getUuid()->toString());

$this->assertResponseOk($response);
$this->assertResponseNoContent($response);

$adminRepository = $this->getEntityManager()->getRepository(Admin::class);
$admin = $adminRepository->find($admin->getUuid()->toString());
Expand Down Expand Up @@ -444,7 +444,7 @@ public function testAdminCanDeleteUserAccount(): void

$response = $this->delete('/user/' . $user->getUuid()->toString());

$this->assertResponseOk($response);
$this->assertResponseNoContent($response);

$userRepository = $this->getEntityManager()->getRepository(User::class);
$user = $userRepository->find($user->getUuid()->toString());
Expand Down
4 changes: 2 additions & 2 deletions test/Functional/UserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public function testDeleteMyAvatar(): void
$this->loginAs($user->getIdentity(), self::DEFAULT_PASSWORD);

$response = $this->delete('/user/my-avatar');
$this->assertResponseOk($response);
$this->assertResponseNoContent($response);
}

public function testActivateMyAccountInvalidCode(): void
Expand Down Expand Up @@ -289,7 +289,7 @@ public function testDeleteMyAccount(): void
$this->loginAs($user->getIdentity(), self::DEFAULT_PASSWORD);

$response = $this->delete('/user/my-account');
$this->assertResponseOk($response);
$this->assertResponseNoContent($response);

$userRepository = $this->getEntityManager()->getRepository(User::class);
$deletedUser = $userRepository->find($user->getUuid()->toString());
Expand Down

0 comments on commit c61816e

Please sign in to comment.