[ebean] insertable/updatable revisited

70 views
Skip to first unread message

Rien Nentjes

unread,
May 11, 2010, 7:41:12 AM5/11/10
to eb...@googlegroups.com
We where wondering...

In the class BeanPropertyAssoc there is the following piece of code:

    /**
     * Return true if this association is updateable.
     */
    public boolean isUpdateable() {
        if (tableJoin.columns().length > 0) {
            return tableJoin.columns()[0].isUpdateable();
        }

        return true;
    }

So the determination if something is updateable or not (or insertable) depends on the first joincolumn annotation we mapped in the entity. This assumption is probably used every where within Ebean.

But is there any reason why the following shouldn't be possible?

  @ManyToOne
  @JoinColumn(name = "companyId")
  private Company company;
 
  @ManyToOne
  @JoinColumns({
    @JoinColumn(name = "companyId", referencedColumnName = "companyId", insertable = false, updatable = false),
    @JoinColumn(name = "customerId", referencedColumnName = "id", insertable = true, updatable = true)
    })
  private Customer customer;

Which would only set the customerId column in the database when the customer property is set. Of course there should be a check to make sure a database column can't be set by two different properties in one entity class.

Rien

Rob Bygrave

unread,
May 11, 2010, 7:55:04 AM5/11/10
to eb...@googlegroups.com
> depends on the first joincolumn annotation

That is almost certainly a bug. It should check all the properties. (this assumption is probably why ImportedIdEmbedded doesn't do the checking... which is probably what should happen).



> This assumption is probably used every where within Ebean.

According to me it is used once - in FactoryAssocOnes which constructs a BindableAssocOne.


So then if some properties are updatable and some not ... then this should probably move the issue to the ImportedId object.... and a quick look at ImportedIdEmbedded ... the dmlAppend and bind methods are not checking for isUpdateable() isInsertable() on each property (so I'm thinking that is where a fix will end up).

... that's from my quick look anyway.

Rien Nentjes

unread,
May 11, 2010, 8:04:44 AM5/11/10
to eb...@googlegroups.com
Yeah, we also had a problem with deletes on an entity that mapped some db columns multiple times and we fixed it by adding a check in "ImportedIdMultiple" and "ImportedIdEmbedded" (see the patch). This seems to be related to this.

Btw, this is a delete on an entity without an @Version (so all columns are included in the where) and with the @ManyToOne + insertable/updatable = false. The preparedstatement gave an exception that it couldn't bind a property with value <x>.

Rien
ebean.patch

Rob Bygrave

unread,
May 12, 2010, 4:11:05 AM5/12/10
to eb...@googlegroups.com
Thanks for that Rien.

Logged as BUG 293 : Issue with Embedded Id's with some properties updatable
http://www.avaje.org/bugdetail-293.html

Fixed in HEAD.

(Patch applied as is)

Rob Bygrave

unread,
May 12, 2010, 4:13:14 AM5/12/10
to eb...@googlegroups.com
The patch didn't have any changes to BeanPropertyAssoc.isUpdateable()

Did I miss something or are your tests passing with BeanPropertyAssoc as it was?

Rien Nentjes

unread,
May 12, 2010, 4:28:22 AM5/12/10
to eb...@googlegroups.com
Rob,

We didn't change BeanPropertyAssoc, of course it is very likely we didn't fix it in the location you would expect. But yes, Ebean is now functioning as we would expect.

Rien

Rob Bygrave

unread,
May 12, 2010, 6:20:43 AM5/12/10
to eb...@googlegroups.com
Ok,

It looks to me like there should still be a bug at FactoryAssocOnes line 57: ...  !ones[i].isUpdateable()

... so I'm not sure why we aren't hitting an issue there. I'll flag this and try to create a test case in a few days (unless you happen to have one I can use as a starting point).


Thanks, Rob.
Reply all
Reply to author
Forward
0 new messages