@Transactional in REST layer or in Service layer? Which is better?

Question

In my case, I want to create multiple tags by invoking the method "createTags()". For testing convenience, I just use @GET and no params in this method, please don't mind it. And I want all the operations rollback if one of them throw an exception, e.g. if t3's tagValue is empty, t1, t2 shouldn't exist in my database. Obviously, I should use @Transactional here, so my question is that can I use @Transactional this way? I mean to use it in my REST layer. Here is the code:

TestREST.class

@Path("tag/create")
@GET
@Transactional(rollbackOn = {NoTagException.class, TagAlreadyExistedException.class}))
public void createTags() throws NoTagException, TagAlreadyExistedException {
    // create all categories
    Tag t1 = new Tag(TagType.CATEGORY);
    t1.setTagValue("Edu");
    tagService.createTag(t1);
    Tag t2 = new Tag(TagType.CATEGORY);
    t2.setTagValue("ZJNU");
    tagService.createTag(t2);
    Tag t3 = new Tag(TagType.CATEGORY);
    // the value is empty here so I hope all previous operations rollback
    t3.setTagValue("");
    tagService.createTag(t3);
}

TagService.class

public void createTag(Tag tag) throws NoTagException, TagAlreadyExistedException {
    if (tag.getType() == null || !(tag.getType() instanceof TagType)) {
        throw new NoTagException("tag's type must be set");
    } else if (tag.getTagValue() == null || tag.getTagValue().equals("")) {
        throw new NoTagException("tag's value must be set!");
    } else {
        Optional<Tag> existedTag = retrieveTagByTypeAndValue(tag.getType(), tag.getTagValue());
        if (existedTag.isPresent()) {
            throw new TagAlreadyExistedException("one or more tags are already existed!");
        } else {
            tagDAO.create(tag);
        }
    }
}

Or should I always use @Transactional in my Service layer? I change the above methods to this:

TestREST.class

@Path("tag/create")
@GET
public void createTags() throws NoTagException, TagAlreadyExistedException {
    // create all categories
    Set<Tag> tags = new HashSet<>();
    Tag t1 = new Tag(TagType.CATEGORY);
    t1.setTagValue("Edu");
    Tag t2 = new Tag(TagType.CATEGORY);
    t2.setTagValue("ZJNU");
    Tag t3 = new Tag(TagType.CATEGORY);
    t3.setTagValue("");
    tags.add(t1);
    tags.add(t2);
    tags.add(t3);
    tagService.createTags(tags);
}

TagService.class

@Transactional(rollbackOn = {NoTagException.class, TagAlreadyExistedException.class}))
public void createTag(Tag tag) throws NoTagException, TagAlreadyExistedException {
    // just the same as above
}

public void createTags(Set<Tag> tags) throws NoTagException, TagAlreadyExistedException {
    if (tags.isEmpty()) {
        throw new NoTagException("tag must be set");
    } else {
        for (Tag tag : tags) {
            createTag(tag);
        }
    }
}

They can all achieve what I expected. So, which approach should I choose? Why? And any suggestions that I can improve these methods? BTW, I use CDI @RequestScoped in TestREST.class and TagService.class Thanks for your help!

Answer 1

I feel this question is less about where to use Transactional, and more about how code is structured.

In the example, adding @Transactional to the rest layer will achieve the exact goal you want (save all tags or none based on error thrown). The question becomes, "where should the business logic exist?".

If, the rest endpoint is just a data collection method, and there's a service method that takes the data and attempts to save it, then the Transactional annotation should exist at that level.

There are two concepts I've found people much smarter than me using on a regular basis. Move the Transactional annotation down to the finest layer that it make sense; and, annotate interface methods over concrete methods. The latter doesn't apply here, per se (but I hope it helps you in the future), but the former should be your guide.

To reiterate, the question is less about where @Transactional should be placed.. It should be, "where will the call to save the tags be performed"; and the @Transactional will follow.

(Hope this helps)