Yevgeny Zaspitsky has posted comments on this change. Change subject: core: using JPA for engine backup awareness ......................................................................
Patch Set 4: (2 comments) https://gerrit.ovirt.org/#/c/40091/4/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EngineBackupLog.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EngineBackupLog.java: Line 96: && this.passed == other.passed : && Objects.equals(outputMessage, other.outputMessage) > 1. No problem to use getters instead if tha is the right thing to do 1. I'm not sure if that makes any difference from JPA perspective. 2. Including mutable values in hashCode breaks its contract (see Object.hashCode javadoc). Here I consider mutability of a member as whether it being updated during an entity (not necessarily the object) life cycle from business point of view, in other words do we use the setter after the value was set. Optimally that should be backed by not having a setter for an immutable member, but I'm not sure that JPA allows that. If yes, we should have 2 constructors: a public one with all immutable values, which we'll use in our code, and another empty one for JPA with visibility as low as JPA allows. Line 96: && this.passed == other.passed : && Objects.equals(outputMessage, other.outputMessage) > I don't think anything should be done specifically in regards to 2 within t Please see https://docs.jboss.org/hibernate/stable/core.old/reference/en/html/persistent-classes-equalshashcode.html In our case the business key is the DB id, which isn't a common scenario. Having equals wider business key might cause JPA trying to insert a new row when our intention is updating. We should write a test for every possible scenario. Re: project-wide change: we migrate to JPA gradually entity-by-entity, so if we find proof for my thoughts, that should be a part of "JPA migration guide". -- To view, visit https://gerrit.ovirt.org/40091 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia0aa9c4d0c8c0a5f7b40a022d7670f30c5fd5b77 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liran Zelkha <liran.zel...@gmail.com> Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
