Kuali Rice Development
  1. Kuali Rice Development
  2. KULRICE-13948

Please make composite keys instead of adding multiple ID keys to JPA entity

    Details

    • Type: Bug Fix Bug Fix
    • Status: Open Open
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 2.5
    • Fix Version/s: 2.6
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
    • Similar issues:
      KULRICE-4026Composite Primary Key class enforcement
      KULRICE-3858Create base abstract class for JPA composite primary key classes
      KULRICE-4052Conversion Script: composite key objects should have better imports
      KULRICE-5771Change @Cacheable annotation keys to use parameter numbers instead of parameter names
      KULRICE-3685Kim entity tables missing foreign-key relationships
      KULRICE-9242Modify jpa metadata provider that gets primary key fields to use a combination of getDeclaredFields and JPA metamodel
      KULRICE-6948KRMS is treating RemotableAttributeErrors as message keys instead of messages
      KULRICE-2948Rename TARGET_PRIMARY_KEY column on certain KIM tables
      KULRICE-3853JPA - Proof of concept with sequence / identity/ prePersist
      KULRICE-14086Improvements for Key Personnel Page Rendering
    • Rice Team:
      Middleware
    • Rice Module:
      KRAD
    • Application Requirement:
      KC
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes

      Description

      We keep getting tons of these errors in our logs

      [10/16/14, 2:02:47 PM] Geo Thomas: [EL Warning]: 2014-10-16 14:07:26.899--ServerSession(1992696841)--You have specified multiple ids for the entity class [org.kuali.rice.krad.bo.AdHocRouteWorkgroup] without specifying an @IdClass. By doing this you may lose the ability to find by identity, distributed cache support etc. Note: You may however use entity manager find operations by passing a list of primary key fields. Else, you will have to use JPQL queries to read your entities. For other id options see @PrimaryKey.
      [EL Warning]: 2014-10-16 14:07:26.9--ServerSession(1992696841)--You have specified multiple ids for the entity class [org.kuali.rice.krad.bo.AdHocRoutePerson] without specifying an @IdClass. By doing this you may lose the ability to find by identity, distributed cache support etc. Note: You may however use entity manager find operations by passing a list of primary key fields. Else, you will have to use JPQL queries to read your entities. For other id options see @PrimaryKey. 
      
      

      If you look at AdhocRouteRecipient.java, it has multiple Ids defined. Can you create a composite key for this instead?

        Activity

        Hide
        Peter Giles (Inactive) added a comment -

        Strictly speaking, these are not errors, they are just warnings. We are not using the distributed cache presently, so we don't need to worry on that count at this point in time. Also, our criteria objects get translated down to JPQL, so we should be alright in that respect as well as long as we are not using DataObjectService.find(...) or something like that to retrieve these. Is there a technical issue we need to solve here, or is the motivation for refactoring these classes (and there are many of them) simply making the warnings go away?

        Here are the entities that are giving us that same EL Warning at startup:

        org.kuali.rice.krad.bo.AdHocRoutePerson
        org.kuali.rice.krad.bo.AdHocRouteWorkgroup
        org.kuali.rice.krad.messages.Message
        org.kuali.rice.kim.bo.ui.KimDocumentRoleQualifier
        org.kuali.rice.krad.messages.Message
        org.kuali.rice.kim.bo.ui.PersonDocumentPhone
        org.kuali.rice.kim.bo.ui.KimDocumentRoleResponsibilityAction
        org.kuali.rice.kim.bo.ui.PersonDocumentEmploymentInfo
        org.kuali.rice.kim.bo.ui.GroupDocumentQualifier
        org.kuali.rice.kim.bo.ui.KimDocumentRoleMember
        org.kuali.rice.kim.bo.ui.PersonDocumentName
        org.kuali.rice.kim.bo.ui.PersonDocumentRole
        org.kuali.rice.kim.bo.ui.PersonDocumentCitizenship
        org.kuali.rice.kim.bo.ui.PersonDocumentEmail
        org.kuali.rice.kim.bo.ui.KimDocumentRolePermission
        org.kuali.rice.kim.bo.ui.KimDocumentRoleResponsibility
        org.kuali.rice.kim.bo.ui.PersonDocumentGroup
        org.kuali.rice.kim.bo.ui.GroupDocumentMember
        org.kuali.rice.kim.bo.ui.RoleDocumentDelegationMember
        org.kuali.rice.kim.bo.ui.PersonDocumentAffiliation
        org.kuali.rice.kim.bo.ui.RoleDocumentDelegation
        org.kuali.rice.kim.bo.ui.PersonDocumentAddress
        org.kuali.rice.kim.bo.ui.RoleDocumentDelegationMemberQualifier

        If we do need to mute the warnings that are being produced, a lower impact approach to addressing this might be to define @IdClasses for these, since that wouldn't effect all usages of all these types.

        Show
        Peter Giles (Inactive) added a comment - Strictly speaking, these are not errors, they are just warnings. We are not using the distributed cache presently, so we don't need to worry on that count at this point in time. Also, our criteria objects get translated down to JPQL, so we should be alright in that respect as well as long as we are not using DataObjectService.find(...) or something like that to retrieve these. Is there a technical issue we need to solve here, or is the motivation for refactoring these classes (and there are many of them) simply making the warnings go away? Here are the entities that are giving us that same EL Warning at startup: org.kuali.rice.krad.bo.AdHocRoutePerson org.kuali.rice.krad.bo.AdHocRouteWorkgroup org.kuali.rice.krad.messages.Message org.kuali.rice.kim.bo.ui.KimDocumentRoleQualifier org.kuali.rice.krad.messages.Message org.kuali.rice.kim.bo.ui.PersonDocumentPhone org.kuali.rice.kim.bo.ui.KimDocumentRoleResponsibilityAction org.kuali.rice.kim.bo.ui.PersonDocumentEmploymentInfo org.kuali.rice.kim.bo.ui.GroupDocumentQualifier org.kuali.rice.kim.bo.ui.KimDocumentRoleMember org.kuali.rice.kim.bo.ui.PersonDocumentName org.kuali.rice.kim.bo.ui.PersonDocumentRole org.kuali.rice.kim.bo.ui.PersonDocumentCitizenship org.kuali.rice.kim.bo.ui.PersonDocumentEmail org.kuali.rice.kim.bo.ui.KimDocumentRolePermission org.kuali.rice.kim.bo.ui.KimDocumentRoleResponsibility org.kuali.rice.kim.bo.ui.PersonDocumentGroup org.kuali.rice.kim.bo.ui.GroupDocumentMember org.kuali.rice.kim.bo.ui.RoleDocumentDelegationMember org.kuali.rice.kim.bo.ui.PersonDocumentAffiliation org.kuali.rice.kim.bo.ui.RoleDocumentDelegation org.kuali.rice.kim.bo.ui.PersonDocumentAddress org.kuali.rice.kim.bo.ui.RoleDocumentDelegationMemberQualifier If we do need to mute the warnings that are being produced, a lower impact approach to addressing this might be to define @IdClasses for these, since that wouldn't effect all usages of all these types.
        Hide
        Peter Giles (Inactive) added a comment -

        Just to follow up briefly and clarify my opinion, I think the better approach would actually be using an @EmbeddedId and @Embeddable class as Gaythri suggests, it would be a much DRYer way to go. However, I don't think we should be doing that kind of entity refactoring in a patch release. If we decide it's necessary, I would definitely defer that to 2.6.

        Show
        Peter Giles (Inactive) added a comment - Just to follow up briefly and clarify my opinion, I think the better approach would actually be using an @EmbeddedId and @Embeddable class as Gaythri suggests, it would be a much DRYer way to go. However, I don't think we should be doing that kind of entity refactoring in a patch release. If we decide it's necessary, I would definitely defer that to 2.6.
        Hide
        Gayathri Athreya added a comment -

        Thanks Peter. Could we suppress this warning but keep other warnings from JPA that might actually be important.

        Show
        Gayathri Athreya added a comment - Thanks Peter. Could we suppress this warning but keep other warnings from JPA that might actually be important.
        Hide
        Peter Giles (Inactive) added a comment -

        I started implementing a solution, but as I got into it I became concerned about the downside of unintentionally suppressing other messages, e.g. messages about client application entities that may actually need compound keys extracted. I don't see a good way to accomplish this without that possibility. In my opinion we need to live with these until 2.6 at which point we can take another look at refactoring the above classes.

        Show
        Peter Giles (Inactive) added a comment - I started implementing a solution, but as I got into it I became concerned about the downside of unintentionally suppressing other messages, e.g. messages about client application entities that may actually need compound keys extracted. I don't see a good way to accomplish this without that possibility. In my opinion we need to live with these until 2.6 at which point we can take another look at refactoring the above classes.

          People

          • Assignee:
            Unassigned
            Reporter:
            Gayathri Athreya
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Structure Helper Panel