0

I have a Spring Boot application using JPA and Hibernate to keep track of a list of Recipe objects. I have a small handful of successful JUnit tests, and I want to add a new test around renaming a recipe.

There's a business requirement that each Recipe must have a unique name.

Relevant code snippets:

Controller:

@RequestMapping(value = "/recipe/{recipeId}", method = RequestMethod.PATCH)
@Transactional
public ResponseEntity<String> renameRecipe(@PathVariable Long recipeId,
        @RequestParam(name = "name") String newRecipeName) {
    Recipe recipe = recipeManager.get(recipeId);
    recipe.setName(newRecipeName);
    recipeManager.save(recipe);
    return new ResponseEntity<>(recipe.toJSONString(), HttpStatus.OK);
}

Manager:

@Repository
public class RecipeManager {
    ....
    public Recipe save(Recipe recipe) {
        System.out.println("save(): All recipes: " + getAll());
        if (lookupByName(recipe.getName()) != null) {
            throw new IllegalArgumentException("A recipe with this name already exists");
        }
        em.persist(recipe);
        return recipe;
    }

Test:

@Test
public void testRecipeCanBeRenamed() throws UnirestException {
    HttpResponse<JsonNode> jsonResp = Unirest.post("http://localhost:" + port + "/recipe")
            .field("name", "Spaghetti").asJson();
    long recipeId = jsonResp.getBody().getObject().getLong("id");
    jsonResp = Unirest.patch("http://localhost:" + port + "/recipe/" + recipeId).field("name", "Pesto Pasta")
            .asJson();
    jsonResp = Unirest.get("http://localhost:" + port + "/recipe/" + recipeId).asJson();
    JSONObject recipeObj = jsonResp.getBody().getObject();
    assertEquals(recipeId, recipeObj.getLong("id"));
    assertEquals("PestoPasta", recipeObj.getString("name"));
    assertNull(recipeManager.lookupByName("Spaghetti"));
    assertNotNull(recipeManager.lookupByName("Pesto Pasta"));
}

The issue with the test is that the IllegalArgumentException is thrown, as a result of what I believe to be Hibernate's local caching mechanism. When I call setName() on the retrieved Recipe instance in the renameRecipe() method, it's actually altering this property on the locally cached instance. So then when the manager attempts to save() the Recipe, it confers with its local cache, and sees the property change that has not yet been persisted to the database.

When running my unit test, in the save() method, immediately prior to the dupe check, I see:

save(): All recipes: [{"name":"Pesto Pasta","id":1}]

This also occurs when running live and testing through Postman.

How can I work around this issue? Do I need to introduce a business layer that checks for the proposed name prior to performing any modifications/persistence?

Craig Otis
  • 27,979
  • 23
  • 117
  • 216
  • This won't solve your current problem but will prevent a new one: note that your *save()* method is calling [EntityManager.persist()](http://docs.oracle.com/javaee/7/api/javax/persistence/EntityManager.html#persist-java.lang.Object-) which will try to create a new persistent entity and almost sure will throw an `EntityExistsException`. Given you actualy want to *update* an existent entity -> [What is the best way to update the entity in JPA](http://stackoverflow.com/questions/8307578/what-is-the-best-way-to-update-the-entity-in-jpa) – dic19 Apr 30 '16 at 16:14

1 Answers1

1

Yes, you should check before modifying, instead of modifying then check. It's much more logical this way anyway, isn't it?

That said, you should also have a unique constraint in the database, because that's the only way to actually prevent duplicate names. With a unique constraint, you would get an exception (a different one, though), even with your way of doing.

JB Nizet
  • 633,450
  • 80
  • 1,108
  • 1,174