Spring and/or Hibernate: Saving many-to-many relations from one side after form submission

Question

The context

I have a simple association between two entities - Category and Email (NtoM). I'm trying to create web interface for browsing and managing them. I have a simple e-mail subscription edit form with list of checkboxes that represents categories, to which given e-mail belongs (I registered property editor for Set<Category> type).

The problem

Form displaying works well, including marking currently assigned categories (for existing e-mails). But no changes are saved to EmailsCategories table (NtoM mapping table, the one defined with @JoinTable - neither newly checked categories are added, nor unchecked categories are removed.

The code

Email entity:

@Entity
@Table(name = "Emails")
public class Email
{
    @Id
    @GeneratedValue(generator = "system-uuid")
    @GenericGenerator(name = "system-uuid", strategy = "uuid2")
    @Column(length = User.UUID_LENGTH)
    protected UUID id;

    @NaturalId
    @Column(nullable = false)
    @NotEmpty
    @org.hibernate.validator.constraints.Email
    protected String name;

    @Column(nullable = false)
    @Temporal(TemporalType.TIMESTAMP)
    protected Date createdAt;

    @Column
    protected String realName;

    @Column(nullable = false)
    protected boolean isActive = true;

    @ManyToMany(mappedBy = "emails", fetch = FetchType.EAGER)
    protected Set<Category> categories = new HashSet<Category>();

    public UUID getId()
    {
        return this.id;
    }

    public Email setId(UUID value)
    {
        this.id = value;

        return this;
    }

    public String getName()
    {
        return this.name;
    }

    public Email setName(String value)
    {
        this.name = value;

        return this;
    }

    public Date getCreatedAt()
    {
        return this.createdAt;
    }

    public String getRealName()
    {
        return this.realName;
    }

    public Email setRealName(String value)
    {
        this.realName = value;

        return this;
    }

    public boolean isActive()
    {
        return this.isActive;
    }

    public Email setActive(boolean value)
    {
        this.isActive = value;

        return this;
    }

    public Set<Category> getCategories()
    {
        return this.categories;
    }

    public Email setCategories(Set<Category> value)
    {
        this.categories = value;

        return this;
    }

    @PrePersist
    protected void onCreate()
    {
        this.createdAt = new Date();
    }
}

Category entity:

@Entity
@Table(name = "Categories")
public class Category
{
    @Id
    @GeneratedValue(generator = "system-uuid")
    @GenericGenerator(name = "system-uuid", strategy = "uuid2")
    @Column(length = User.UUID_LENGTH)
    protected UUID id;

    @NaturalId(mutable = true)
    @Column(nullable = false)
    @NotEmpty
    protected String name;

    @ManyToMany
    @JoinTable(
        name = "EmailsCategories",
        joinColumns = {
            @JoinColumn(name = "idCategory", nullable = false, updatable = false)
        },
        inverseJoinColumns = {
            @JoinColumn(name = "idEmail", nullable = false, updatable = false)
        }
    )
    protected Set<Email> emails = new HashSet<Email>();

    public UUID getId()
    {
        return this.id;
    }

    public Category setId(UUID value)
    {
        this.id = value;

        return this;
    }

    public String getName()
    {
        return this.name;
    }

    public Category setName(String value)
    {
        this.name = value;

        return this;
    }

    public Set<Email> getEmails()
    {
        return this.emails;
    }

    public Category setEmails(Set<Email> value)
    {
        this.emails = value;

        return this;
    }

    @Override
    public boolean equals(Object object)
    {
        return object != null
            && object.getClass().equals(this.getClass())
            && ((Category) object).getId().equals(this.id);
    }

    @Override
    public int hashCode()
    {
        return this.id.hashCode();
    }
}

Controller:

@Controller
@RequestMapping("/emails/{categoryId}")
public class EmailsController
{
    @Autowired
    protected CategoryService categoryService;

    @Autowired
    protected EmailService emailService;

    @ModelAttribute
    public Email addEmail(@RequestParam(required = false) UUID id)
    {
        Email email = null;

        if (id != null) {
            email = this.emailService.getEmail(id);
        }
        return email == null ? new Email() : email;
    }

    @InitBinder
    public void initBinder(WebDataBinder binder)
    {
        binder.registerCustomEditor(Set.class, "categories", new CategoriesSetEditor(this.categoryService));
    }

    @RequestMapping(value = "/edit/{id}", method = RequestMethod.GET)
    public String editForm(Model model, @PathVariable UUID id)
    {
        model.addAttribute("email", this.emailService.getEmail(id));

        model.addAttribute("categories", this.categoryService.getCategoriesList());

        return "emails/form";
    }

    @RequestMapping(value = "/save", method = RequestMethod.POST)
    public String save(@PathVariable UUID categoryId, @ModelAttribute @Valid Email email, BindingResult result, Model model)
    {
        if (result.hasErrors()) {
            model.addAttribute("categories", this.categoryService.getCategoriesList());
            return "emails/form";
        }

        this.emailService.save(email);

        return String.format("redirect:/emails/%s/", categoryId.toString());
    }
}

Form view:

<form:form action="${pageContext.request.contextPath}/emails/${category.id}/save" method="post" modelAttribute="email">
    <form:hidden path="id"/>
    <fieldset>
        <label for="emailName"><spring:message code="email.form.label.Name" text="E-mail address"/>:</label>
        <form:input path="name" id="emailName" required="required"/>
        <form:errors path="name" cssClass="error"/>

        <label for="emailRealName"><spring:message code="email.form.label.RealName" text="Recipient display name"/>:</label>
        <form:input path="realName" id="emailRealName"/>
        <form:errors path="realName" cssClass="error"/>

        <label for="emailIsActive"><spring:message code="email.form.label.IsActive" text="Activation status"/>:</label>
        <form:checkbox path="active" id="emailIsActive"/>
        <form:errors path="active" cssClass="error"/>

        <form:checkboxes path="categories" element="div" items="${categories}" itemValue="id" itemLabel="name"/>
        <form:errors path="categories" cssClass="error"/>

        <button type="submit"><spring:message code="_common.form.Submit" text="Save"/></button>
    </fieldset>
</form:form>

Edit - added DAO code

(emailService.save() is just a proxy call to emailDao.save())

public void save(Email email)
{
    this.getSession().saveOrUpdate(email);
}

Edit 2 - little more debug/logs

A simple test snippet:

public void test()
{
    Category category = new Category();
    category.setName("New category");
    this.categoryDao.save(category);

    Email email = new Email();
    email.setName("test@me")
        .setRealName("Test <at> me")
        .getCategories().add(category);
    this.emailDao.save(email);

}

And these are logs:

12:05:34.173 [http-bio-8080-exec-23] DEBUG org.hibernate.SQL - insert into Emails (createdAt, isActive, name, realName, id) values (?, ?, ?, ?, ?)
12:05:34.177 [http-bio-8080-exec-23] DEBUG org.hibernate.persister.collection.AbstractCollectionPersister - Inserting collection: [pl.chilldev.mailer.web.entity.Category.emails#24d190e3-99db-4792-93ea-78c294297d2d]
12:05:34.177 [http-bio-8080-exec-23] DEBUG org.hibernate.persister.collection.AbstractCollectionPersister - Collection was empty

Even with this logs it seems a little strage - it tels that it's inserting collection with one element, but then it tells it was empty...

add what's the code of EmailService.save() ? Did you put a breakpoint at `this.emailService.save(email);` to be sure it is called ? — ben75, Oct 09 '13 at 18:52
For sure it's called, because the e-mail record itself is saved. I also checked all properties - categories set of entity contains two elements with correct category IDs. — Rafał Wrzeszcz, Oct 09 '13 at 19:08
Have you tried using the cascade = CascadeType.ALL with @JoinColumn — yousafsajjad, Oct 09 '13 at 20:56
I guess you mean cascade = CascadeType.ALL on @ManyToMany? Yes, tried - didn't work. — Rafał Wrzeszcz, Oct 09 '13 at 21:34

score 45 · Accepted Answer · answered Oct 10 '13 at 09:08

45

Here we go again.

A bidirectional association has two sides: an owner side, and an insverse side. The owner side is the one without the mappedBy attribute. To know which association exists between entities, JPA/Hibernate only cares about the owner side. Your code only modifies the inverse side, and not the owner side.

It's YOUR job to maintain the coherence of the object graph. It's sometimes acceptable to have an incoherent object graph, but not modifying the owner side won't make the changes persistent.

So you need to add

category.getEmails().add(email);

or to choose Email as the owner side rather than Category.

answered Oct 10 '13 at 09:08

JB Nizet

678,734
91
1,224
1,255

As Nizet rightly pointed, you can sometime live with incoherent graph if you do not intend to use that object graph beyond the persistence layer. But it's a good habit to set both sides and this will save you from unwanted surprised ORM can throw if not set. – Shailendra Oct 10 '13 at 10:00
I know this is old and @JB Nizet may not watch this, but would this throw an error if category.getEmails() is blank ?? – HopeKing May 07 '17 at 07:09
1

@CaptainHackSparrow if "blank" means "null", then yes. But if it's null, then it just means *you* didn't correctly initialize it to an empty collection. Hibernate will never set a persistent collection to null. If there is no email, it will be an empty set. If "blank" means "empty", then no, it won't throw an exception (unless *you* chose to initialize the field to an unmodifiable collection, but then it's another bug in your code. Hibernate won't initialize it with an unmodifiable collection). – JB Nizet May 07 '17 at 07:15
1

Just do what the OP of this question did: always initialize your fields to correct values, that respect the invariants of the class: `private Set emails = new HashSet<>();`. Don't leave the field as null. – JB Nizet May 07 '17 at 07:42
One more question, since you mention that we don't leave the field as null, does it mean in our Bean definitions all fields should be initialised ? or does it hold in the case of sets within a Bean only like in this example ?? – HopeKing May 07 '17 at 09:54
3

It's always useful to initialize fields to valid values when possible. The natural default value for a set of emails is an empty set. Initializing to valid values allows preventing exceptions like the NPE you had. Not all fields can be initialized this way, but when they can, then they should be. In particulars, leaving collections as null is always a bad idea. Returning a null collection from a method is always a bad idea. – JB Nizet May 07 '17 at 09:58
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/143596/discussion-between-captain-hacksparrow-and-jb-nizet). – HopeKing May 07 '17 at 10:00

Spring and/or Hibernate: Saving many-to-many relations from one side after form submission

The context

The problem

The code

Edit - added DAO code

Edit 2 - little more debug/logs

1 Answers1

Linked

Related