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

Another problem with EBOs - PBO.refresh() not handling them properly

    Details

    • Type: Bug Fix Bug Fix
    • Status: Closed Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.2
    • Fix Version/s: 2.1.1
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-7513YAEBOP - Yet Another EBO Problem...Persistence Service not guarded properly
      KULRICE-3796JPA - Verify that the way we are handling sequences is the proper way to handle them moving forward
      KULRICE-7170PropertyUtils doesn't handle introspection on interfaces well
      KULRICE-4446KualiLookupableHelperServiceImpl does not handle missing EBO modules
      KULRICE-4442Person service does not handle extension objects properly
      KULRICE-4403YAPWKSBENH - Yet Another Place Where KSB Exceptions are Not Handled
      KULRICE-4607DocumentHeaderDaoProxy doesn't check module properly
      KULRICE-7481Failing to load the EBO based on the stateCode and zipCode
      KULRICE-3947Determine how to handle collections in JPA for existing applications
    • Rice Module:
      KNS, KRAD
    • KRAD Feature Area:
      Persistence Framework
    • Application Requirement:
      KFS
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required

      Description

      It looks like we have another problem with EBOs. In this case, it's coming through the maintainable framework. But, the core cause is that, since StateEbo is also now implementing the PersistableBusinessObject interface, it matches this test in KualiMaintainableImpl:

      if (possibleRelationship instanceof PersistableBusinessObject) {
      	((PersistableBusinessObject) possibleRelationship).refresh();
      }
      

      I'm not sure that is the place which needs to be fixed however. It's the base implementation of the refresh() method on PersistableBusinessObjectBase. It's assuming that the object can be retrieved from the ORM tool by calling:

      getPersistenceService().retrieveNonKeyFields(this);

      Really, it would be best if the PersistenceService would handle this without blowing up, utilizing the EBO framework if applicable. (Or, at least do nothing...)

      The method below (from PersistableBusinessObjectBase) performs the needed check to avoid blowing up:

      public void refreshReferenceObject(String referenceObjectName) {
      	if ( StringUtils.isNotBlank(referenceObjectName) && !StringUtils.equals(referenceObjectName, "extension")) {
      		final PersistenceStructureService pss = getPersistenceStructureService();
      		if ( pss.hasReference(this.getClass(), referenceObjectName) || pss.hasCollection(this.getClass(), referenceObjectName)) {
                  	    getPersistenceService().retrieveReferenceObject( this, referenceObjectName);
      		} else {
                          LOG.warn( "refreshReferenceObject() called with non-reference property: " + referenceObjectName );
      		}
      	}
      }
      

      See the linked issue for the errors we are getting.

        Issue Links

          Activity

          Hide
          Peter Giles (Inactive) added a comment -

          Worked fine in embedded mode, blows up in standalone. I have the issue repro'd locally, looking into the fix now.

          Show
          Peter Giles (Inactive) added a comment - Worked fine in embedded mode, blows up in standalone. I have the issue repro'd locally, looking into the fix now.
          Hide
          Peter Giles (Inactive) added a comment -

          I have a local fix at the PersistenceServiceImpl level where if I detect the EBO I take a no-op. However, if the more desirable behavior is to actually refresh the EBO, it looks like that would need to be done from KualiMaintainableImpl since there isn't a method on the ModuleService to load an EBO based on an instance of itself. In other words, we don't have enough information to do it from within the PersistenceService. We need metadata about the keys for the EBO, or we need the parent object and the relationship IIRC.

          Show
          Peter Giles (Inactive) added a comment - I have a local fix at the PersistenceServiceImpl level where if I detect the EBO I take a no-op. However, if the more desirable behavior is to actually refresh the EBO, it looks like that would need to be done from KualiMaintainableImpl since there isn't a method on the ModuleService to load an EBO based on an instance of itself. In other words, we don't have enough information to do it from within the PersistenceService. We need metadata about the keys for the EBO, or we need the parent object and the relationship IIRC.
          Hide
          Jonathan Keller added a comment -

          So - is there something which can be done in PersistableBusinessObjectBase to avoid the call?

          But I like the idea of the PersistenceService not blowing up. That way an "innocent" call to refresh it (e.g., by an implementor who does not know that it's really an EBO) does not blow the application.

          Show
          Jonathan Keller added a comment - So - is there something which can be done in PersistableBusinessObjectBase to avoid the call? But I like the idea of the PersistenceService not blowing up. That way an "innocent" call to refresh it (e.g., by an implementor who does not know that it's really an EBO) does not blow the application.
          Hide
          Peter Giles (Inactive) added a comment -

          Okay, the no-op in PersistenceServiceImpl.retrieveNonKeyFields for EBOs seems like a good way to go then. Resolving.

          Show
          Peter Giles (Inactive) added a comment - Okay, the no-op in PersistenceServiceImpl.retrieveNonKeyFields for EBOs seems like a good way to go then. Resolving.
          Hide
          Peter Giles (Inactive) added a comment - - edited

          This broke an integration test, and I ended up going back and changing PersistenceServiceImpl to actually refresh the EBO. I didn't realize that PersistenceService has a method to get a Map of primary key values in it, which makes this practical to do.

          I haven't looked at EBOs much in the past, and as I'm getting a bit more familiar, I do have some concern – we have the confusing situation where when running locally, EBOs may be retrieved with reference objects intact (as ModuleServiceBase would likely do, depending on the OJB mapping), and when running remotely, they may or may not be there (likely not, looking at our implementations) as that is a decision for the remote ModuleService impl. I guess this is a general hazard of working with EBOs?

          Show
          Peter Giles (Inactive) added a comment - - edited This broke an integration test, and I ended up going back and changing PersistenceServiceImpl to actually refresh the EBO. I didn't realize that PersistenceService has a method to get a Map of primary key values in it, which makes this practical to do. I haven't looked at EBOs much in the past, and as I'm getting a bit more familiar, I do have some concern – we have the confusing situation where when running locally, EBOs may be retrieved with reference objects intact (as ModuleServiceBase would likely do, depending on the OJB mapping), and when running remotely, they may or may not be there (likely not, looking at our implementations) as that is a decision for the remote ModuleService impl. I guess this is a general hazard of working with EBOs?

            People

            • Assignee:
              Peter Giles (Inactive)
              Reporter:
              Jonathan Keller
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Structure Helper Panel