Grails

Domain hasMany association using a List is adding duplicate items when the parent is saved

Details

  • Testcase included:
    yes

Description

If you have an Author that has a List of Book objects:

doublesave.Author:


package doublesave

class Author {
    String firstName
    String lastName

    // Hi, see here! 
    List books
    
    static hasMany = [books: Book]
}

doublesave.Book:

package doublesave

class Book {
    String title
    Date published
    BigDecimal price

    static belongsTo = [author: Author]
}

It will re-add a book to the list whenever the Author is saved:


package doublesave

import grails.test.mixin.*
import org.junit.*

@TestFor(Author)
class AuthorTests {

    void testSomething() {
        def author = new Author(firstName:'foo', lastName: 'bar')
        author.save()
        assert author.id != null
        assert author.books == null

        def book1 = new Book(title: 'grails', price: 43, published: new Date(), author: author)
        assert author.books == null

        // add the book to the author to complete the other side
        author.addToBooks(book1)
        assert author.books.size() == 1

        author.save()
        // WTF!  Books size is 2 after the author save?!?
        assert author.books.size() == 1
   }
}

The same test fails in unit and integration tests in 2.0.1.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Graeme Rocher added a comment -

Well, this is not really a bug, because Grails will automatically link the association, and List allows duplicates.. not sure at this stage

Show
Graeme Rocher added a comment - Well, this is not really a bug, because Grails will automatically link the association, and List allows duplicates.. not sure at this stage
Hide
Permalink
Ted Naleid added a comment -

Graeme, I think I'm probably missing something, but I still don't understand why it's adding 2 copies. I only did addToBooks once. Are you saying it's because I did author:author in the Book constructor that grails should be handling it automatically for us? That's not the behavior I'm seeing.

If I change the test to remove the author from the constructor and only do a single addToBooks, it still fails with 2 books:

    void testSomething() {
        def author = new Author(firstName:'foo', lastName: 'bar')
        author.save()
        assert author.id != null
        assert author.books == null

        // REMOVED AUTHOR FROM CONSTRUCTOR
        def book1 = new Book(title: 'grails', price: 43, published: new Date())
        assert author.books == null

        author.addToBooks(book1)
        assert author.books.size() == 1

        author.save()
        // STILL FAILS HERE AFTER SAVE, IT"S BEEN DOUBLE ADDED
        assert author.books.size() == 1
}

Also, if I remove the addToBooks and only have it in the constructor, it fails:

    void testSomething() {
        def author = new Author(firstName:'foo', lastName: 'bar')
        author.save()
        assert author.id != null
        assert author.books == null

        def book1 = new Book(title: 'grails', price: 43, published: new Date(), author: author)
        assert author.books == null

        // COMMENT OUT EXPLICIT addToBooks
//        author.addToBooks(book1)
//        assert author.books.size() == 1

        author.save()
        // fails here with an NPE as author.books is null 
        assert author.books.size() == 1
}

If I don't do either the author in the constructor or the addToBooks, it obviously doesn't know about the relationship and fails to associate the book and author.

Am I missing something, or doing something wrong with lists here?

Show
Ted Naleid added a comment - Graeme, I think I'm probably missing something, but I still don't understand why it's adding 2 copies. I only did addToBooks once. Are you saying it's because I did author:author in the Book constructor that grails should be handling it automatically for us? That's not the behavior I'm seeing. If I change the test to remove the author from the constructor and only do a single addToBooks, it still fails with 2 books: void testSomething() { def author = new Author(firstName:'foo', lastName: 'bar') author.save() assert author.id != null assert author.books == null // REMOVED AUTHOR FROM CONSTRUCTOR def book1 = new Book(title: 'grails', price: 43, published: new Date()) assert author.books == null author.addToBooks(book1) assert author.books.size() == 1 author.save() // STILL FAILS HERE AFTER SAVE, IT"S BEEN DOUBLE ADDED assert author.books.size() == 1 } Also, if I remove the addToBooks and only have it in the constructor, it fails: void testSomething() { def author = new Author(firstName:'foo', lastName: 'bar') author.save() assert author.id != null assert author.books == null def book1 = new Book(title: 'grails', price: 43, published: new Date(), author: author) assert author.books == null // COMMENT OUT EXPLICIT addToBooks // author.addToBooks(book1) // assert author.books.size() == 1 author.save() // fails here with an NPE as author.books is null assert author.books.size() == 1 } If I don't do either the author in the constructor or the addToBooks, it obviously doesn't know about the relationship and fails to associate the book and author. Am I missing something, or doing something wrong with lists here?
Hide
Permalink
Alexandre Masselot added a comment - - edited

Hi
I have had the same problems, killing my afternoon, saving twice
mac os 10.7, grails 2.0.0 and 2.0.1

Taking your project, I've added two tests:
First one save multiple time the same book multiple time, and we see size going 2^n

  void test_one_book_save_multiple_times() {
    def author = new Author(firstName:'foo', lastName: 'bar')
    author.save()

    def book1 = new Book(title: 'grails', price: 43, published: new Date())
    author.addToBooks(book1)

    println "before any author.save: author.book.size():\t"+author.books.size()
    author.save()
    println "after first author.save: author.book.size():\t"+author.books.size()
    author.save()
    println "after second author.save: author.book.size():\t"+author.books.size()
    author.save()
    println "after third author.save: author.book.size():\t"+author.books.size()
  }

System output
before any author.save: author.book.size(): 1
after first author.save: author.book.size(): 2
after second author.save: author.book.size(): 4
after third author.save: author.book.size(): 8

The second one add 3 books and save => there are counted twice

  void test_3_books_save() {
    def author = new Author(firstName:'foo', lastName: 'bar')
    author.save()

    author.addToBooks(new Book(title: 'grails', price: 43, published: new Date()))
    author.addToBooks(new Book(title: 'grails aa', price: 43, published: new Date()))
    author.addToBooks(new Book(title: 'grails bb', price: 43, published: new Date()))

    println "before author.save: author.book.size():\t"+author.books.size()
    author.save()
    println "after author.save: author.book.size():\t"+author.books.size()
  }

System output
before author.save: author.book.size(): 3
after author.save: author.book.size(): 6

There are different variations of that, but as the list can contain duplicate, each save just duplicate the data.
I cannot find a way to handle the situation, event if an author is instanciated and populated at once.

Does is mean there is no way to have hasMany with List?

Show
Alexandre Masselot added a comment - - edited Hi I have had the same problems, killing my afternoon, saving twice mac os 10.7, grails 2.0.0 and 2.0.1 Taking your project, I've added two tests: First one save multiple time the same book multiple time, and we see size going 2^n void test_one_book_save_multiple_times() { def author = new Author(firstName:'foo', lastName: 'bar') author.save() def book1 = new Book(title: 'grails', price: 43, published: new Date()) author.addToBooks(book1) println "before any author.save: author.book.size():\t" +author.books.size() author.save() println "after first author.save: author.book.size():\t" +author.books.size() author.save() println "after second author.save: author.book.size():\t" +author.books.size() author.save() println "after third author.save: author.book.size():\t" +author.books.size() } System output before any author.save: author.book.size(): 1 after first author.save: author.book.size(): 2 after second author.save: author.book.size(): 4 after third author.save: author.book.size(): 8 The second one add 3 books and save => there are counted twice void test_3_books_save() { def author = new Author(firstName:'foo', lastName: 'bar') author.save() author.addToBooks( new Book(title: 'grails', price: 43, published: new Date())) author.addToBooks( new Book(title: 'grails aa', price: 43, published: new Date())) author.addToBooks( new Book(title: 'grails bb', price: 43, published: new Date())) println "before author.save: author.book.size():\t" +author.books.size() author.save() println "after author.save: author.book.size():\t" +author.books.size() } System output before author.save: author.book.size(): 3 after author.save: author.book.size(): 6 There are different variations of that, but as the list can contain duplicate, each save just duplicate the data. I cannot find a way to handle the situation, event if an author is instanciated and populated at once. Does is mean there is no way to have hasMany with List?
Hide
Permalink
Alexandre Masselot added a comment -

BTW Ted, didn't you forget to add @Mock(Book) in your Author test?

Moving on with the problem, I've used a work around my collection are small and don't evolve, but order is important.

I made your Book extend


abstract class SortedCollectionBean implements Comparable{
	int idxList

	int compareTo(other) {
		return idxList.compareTo(other.idxList)
	}
}

add in Author

	void leftShift(Book b){
		b.idxList=books?.size()?:0
		addToBooks(b)
	}

and instead of author.addToBooks

    author << new Book(title: 'grails', price: 43, published: new Date())

Ok, that's not the cleanest way of programming, but tests produce coherent results...

There is certainly some more elegant solution, where the indexing is taken into account only from the Book side with a beforeInsert statement.

alex

Show
Alexandre Masselot added a comment - BTW Ted, didn't you forget to add @Mock(Book) in your Author test? Moving on with the problem, I've used a work around my collection are small and don't evolve, but order is important. I made your Book extend abstract class SortedCollectionBean implements Comparable{ int idxList int compareTo(other) { return idxList.compareTo(other.idxList) } } add in Author void leftShift(Book b){ b.idxList=books?.size()?:0 addToBooks(b) } and instead of author.addToBooks author << new Book(title: 'grails', price: 43, published: new Date()) Ok, that's not the cleanest way of programming, but tests produce coherent results... There is certainly some more elegant solution, where the indexing is taken into account only from the Book side with a beforeInsert statement. alex
Hide
Permalink
Graeme Rocher added a comment -

fixed by https://github.com/SpringSource/grails-data-mapping/commit/48b9f6d27d0f38ce303fcd3ccd77eac23cea68ed

Show
Graeme Rocher added a comment - fixed by https://github.com/SpringSource/grails-data-mapping/commit/48b9f6d27d0f38ce303fcd3ccd77eac23cea68ed

People

  • Assignee:
    Graeme Rocher
    Reporter:
    Ted Naleid
Vote (4)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: