Grails
  1. Grails
  2. GRAILS-8815

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

        Hide
        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
        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
        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
        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
        Show
        Graeme Rocher added a comment - fixed by https://github.com/SpringSource/grails-data-mapping/commit/48b9f6d27d0f38ce303fcd3ccd77eac23cea68ed

          People

          • Assignee:
            Graeme Rocher
            Reporter:
            Ted Naleid
          • Votes:
            4 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development