1 Delete
djmil edited this page 2023-07-25 13:58:50 +02:00

To completely cover upcoming Delete endpoint with all the possible test cases, we need to create another user:

UserDetails kumar = users
		.username("kumar2")
		.password(passwordEncoder.encode("xyz789"))
		.roles("CARD-OWNER")
		.build();

Tests

The Happy Path

@Test
@DirtiesContext
void shouldDeleteAnExistingCashCard() {
    ResponseEntity<Void> response = restTemplate
            .withBasicAuth("sarah1", "abc123")
            .exchange("/cashcards/99", HttpMethod.DELETE, null, Void.class);
    assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NO_CONTENT);

	// Ensure that the resource was actually deleted:
    ResponseEntity<String> getResponse = restTemplate
            .withBasicAuth("sarah1", "abc123")
            .getForEntity("/cashcards/99", String.class);
    assertThat(getResponse.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
}

Notice that we've added the @DirtiesContext annotation. We'll add this annotation to all tests which change the data. If we don't, then these tests could affect the result of other tests in the file.

Why not use RestTemplate.delete()?

Notice that we're using RestTemplate.exchange() even though RestTemplate supplies a method that looks like we could use: RestTemplate.delete(). However, let's look at the signature:

public class RestTemplate ... {
    public void delete(String url, Object... uriVariables)

The other methods we've been using (such as getForEntity() and exchange()) return a ResponseEntity, but delete() does not. Instead, it's a void method. Why is this?

The Spring Web framework supplies the delete() method as a convenience, but it comes with some assumptions:

  1. A response to a DELETE request will have no body.
  2. The client shouldn't care what the response code is unless it's an error, in which case, it'll throw an exception.

Given those assumptions, no return value is needed from delete().

But the second assumption makes delete() unsuitable for us: We need the ResponseEntity in order to assert on the status code! So we won't use the convenience method, but rather let's use the more general method: exchange().

Test case: The Cash Card doesn't exist

Our contract states that we should return 404 NOT FOUND if we try to delete a card that doesn't exist.

@Test
void shouldNotDeleteACashCardThatDoesNotExist() {
    ResponseEntity<Void> deleteResponse = restTemplate
            .withBasicAuth("sarah1", "abc123")
            .exchange("/cashcards/99999", HttpMethod.DELETE, null, Void.class);
    assertThat(deleteResponse.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
}

Enforce ownership

We need to check whether the record exists. If not, we should not delete the Cash Card, and return 404 NOT FOUND. Add the existsByIdAndOwner() method to the Repository.

public interface CashCardRepository extends CrudRepository<CashCard, Long>, PagingAndSortingRepository<CashCard, Long> {
	...
	boolean existsByIdAndOwner(Long id, String owner);
	...
}

This is another case where Spring Data will generate the implementation of this method as long as we add it to the Repository.

So why not just use the findByIdAndOwner() method and check whether it returns null? We could absolutely do that! But, such a call would return extra information (the content of the Cash Card retrieved), so we'd like to avoid it as to not introduce extra complexity.

If you'd rather not use the existsByIdAndOwner() method, that's ok! You may choose to use findByIdAndOwner(). The final result will be the same!

Endpoint API

@DeleteMapping("/{id}")
private ResponseEntity<Void> deleteCashCard(@PathVariable Long id, Principal principal) {
    if (cashCardRepository.existsByIdAndOwner(id, principal.getName())) {
	    cashCardRepository.deleteById(id);
	    return ResponseEntity.noContent().build();
    }
    
    return ResponseEntity.notFound().build();
}

We use the @DeleteMapping with the "{id}" parameter, which Spring Web matches to the id method parameter. CashCardRepository already has the method we need: deleteById() (it's inherited from CrudRepository).

Hide Unauthorized Records

At this point, you may ask yourself, "Are we done?" You are the best person to answer that question! There is one more case that we have yet to test: What if the user attempts to delete a Cash Card owned by someone else? We decided in the associated lesson that the response should be 404 NOT FOUND in this case. Thats enough information for us to write a test for that case:

@Test
void shouldNotAllowDeletionOfCashCardsTheyDoNotOwn() {
	ResponseEntity<Void> deleteResponse = restTemplate
			.withBasicAuth("sarah1", "abc123")
			.exchange("/cashcards/102", HttpMethod.DELETE, null, Void.class);
	assertThat(deleteResponse.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
}

Do you think the test will pass? Take a moment to predict the outcome, then run the test.

It passed! That's great news.

  • We have written a test for a specific case in our API.
  • The test passed without any code changes!

Now let's consider a question which may have occurred to you: Why do I need that test, since it passes without having to make any code changes? Isn't the purpose of TDD to use tests to guide the implementation of the application? If that's the case, why did I bother to write that test?

Answers:

  • Yes, that is one of many benefits that TDD provides: A process to guide the creation of code in order to arrive at a desired outcome.
  • Tests themselves, though, have another purpose, separate from TDD: Tests are a powerful safety net to enforce correctness. Since the test you just wrote tests a different case than those already written, it provides value. If someone were to make a code change which caused this new test to fail, then you will have caught the error before it became an issue! Yay for Tests.

But wait, you say. Shouldn't I test that the record that I tried to delete still exists in the database - that it didn't get deleted? Yes! That is a valid test. Thanks for mentioning it! Add the following code to the test method, to verify that the record you tried unsuccessfully to delete is still there:

void shouldNotAllowDeletionOfCashCardsTheyDoNotOwn() {
...
	// Ensure that the record still exsists:
	ResponseEntity<String> getResponse = restTemplate
			.withBasicAuth("kumar2", "xyz789")
			.getForEntity("/cashcards/102", String.class);
	assertThat(getResponse.getStatusCode()).isEqualTo(HttpStatus.OK);
}

Do you think the test will pass? Of course it will! (right?) Run the test. Just to make sure... please run all the tests and ensure that they pass.