[KULRICE-11182] Clear out all locking keys on Maintenance copy, not just the main object Created: 04/Nov/13  Updated: 30/Mar/16

Status: Open
Project: Kuali Rice Development
Component/s: Development
Affects Version/s: None
Fix Version/s: None
Security Level: Public (Public: Anyone can view)

Type: New Feature Priority: Major
Reporter: James Smith Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: LongTerm
Remaining Estimate: 1 day, 6 hours
Time Spent: 2 hours
Original Estimate: 2 days

Issue Links:
cloned to KULRICE-14154 Analysis for db changes on git Open
discovered KULRICE-14096 Cannot copy TravelAccount when it has... Closed
Epic Link: KFS Backlog
Rice Module:
Application Requirement:
Sprint: Middleware 2.5.2 Sprint 3, Middleware 2.5.2 Sprint 4, Middleware 2.5.2 Sprint 5
KAI Review Status: Not Required
KTI Review Status: Not Required
Code Review Status: Not Required
Include in Release Notes?:
Story Points: 5


Reported via James Smith:

About a month ago, I discovered an issue in TEM where a business object with children was being maintained on a maint doc. The business object itself and more importantly, the collection of children objects, all used synthetic keys based on a sequence as their PK’s, and on copy, I found we had to clear the PK’s manually in the processAfterCopy on the maintainable so that the PK’s were reassigned with the document saved them again; if we didn’t do the key clearing, then the children business objects were “stolen” by the object created by the copy and removed from the original. I talked to Doug Pace to see what KC did about this, and he saw two examples of this: one where they cleared the keys and one where they had forgotten (causing the “stealing” issue). Again: I’m not sure what help KRAD could provide to avoid this situation but it’s so easy to forget and cause problems that if there was a way to fix it, I think it would be very helpful for all of the client applications.

At this time, it looks like we are clearing the basic primary keys but do not have the code to clear all keys associated with the maintenance document, like those on collections or those on child objects. We should be doing this if preserveLockingKeysOnCopy is not set. Verify whether this is still a problem in KRAD and fix it if it is.

Comment by Martin Taylor (Inactive) [ 12/Nov/14 ]

Can you provide a link to the CGB Code Changes related to this ticket?

Comment by James Smith [ 18/Nov/14 ]

An example can be found here: https://github.com/kuali/kfs/blob/master/work/src/org/kuali/kfs/module/tem/document/maintenance/AgencyStagingDataMaintainable.java in the processAfterCopy method. Towards the bottom of the method, the id and parent agency staging data id of the TripAccountingInformation is set to null - those are the synthetic keys which OJB will only repopulate if they were set to null. Does that help, Martin?

Comment by Martin Taylor (Inactive) [ 18/Nov/14 ]
     * @see org.kuali.rice.kns.maintenance.KualiMaintainableImpl#processAfterCopy(org.kuali.rice.kns.document.MaintenanceDocument, java.util.Map)
    public void processAfterCopy(MaintenanceDocument document, Map<String,String[]> parameters) {
        super.processAfterCopy(document, parameters);
        AgencyStagingData agencyData = (AgencyStagingData) getBusinessObject();

        AgencyStagingDataMaintainable oldMaintainable = (AgencyStagingDataMaintainable)document.getOldMaintainableObject();
        //this is not new, so it must be for copy - we will set the Copied From Id

        //set TripAccountingInformation primary key and foreign key to null so the maintainance document can handle setting the appropriate key values
        if (!agencyData.getTripAccountingInformation().isEmpty()) {
            for(TripAccountingInformation account : agencyData.getTripAccountingInformation()) {
Comment by Jeff Ruch [ 12/Jan/15 ]

The basic primary keys are being cleared out in maintenanceDocumentServiceImpl$processMaintenanceObjectForCopy in clearPrimaryKeyFields()

Comment by Jeff Ruch [ 12/Jan/15 ]

Hi James,

Looking at the example above, you have it hard-coded for setting the AgencyStagingDataId to null. I assume this shouldn't be done to all foreign keys. What is the rule, or how do I determine which foreign keys should be set to null?

Comment by James Smith [ 12/Jan/15 ]

Adding the OJB declaration for AgencyStagingData to help explain my answer better:

<class-descriptor class="org.kuali.kfs.module.tem.businessobject.AgencyStagingData" table="TEM_AGENCY_STAGING_T">
		<field-descriptor name="id" primarykey="true" column="ID" jdbc-type="INTEGER"/>
		<field-descriptor name="copiedFromId" column="COPIED_AGENCY_STAGING_ID" jdbc-type="INTEGER"/>
		<field-descriptor name="errorCode" column="ERROR_CD" jdbc-type="VARCHAR"/>
		<field-descriptor name="duplicateRecordId" column="DUP_REC_ID" jdbc-type="INTEGER"/>
		<field-descriptor name="importBy" column="IMPORT_BY" jdbc-type="VARCHAR"/>  	    
		<collection-descriptor name="tripAccountingInformation" element-class-ref="org.kuali.kfs.module.tem.businessobject.TripAccountingInformation" collection-class="org.apache.ojb.broker.util.collections.RemovalAwareList" auto-retrieve="true" auto-update="object" auto-delete="true">
			<inverse-foreignkey field-ref="agencyStagingDataId"/>
        <reference-descriptor name="profile" class-ref="org.kuali.kfs.module.tem.businessobject.TemProfile" auto-retrieve="true" auto-update="none" auto-delete="none">
            <foreignkey field-ref="temProfileId"/>
        <reference-descriptor name="creditCardAgency" class-ref="org.kuali.kfs.module.tem.businessobject.CreditCardAgency" auto-retrieve="true" auto-update="none" auto-delete="none">
            <foreignkey field-ref="creditCardOrAgencyCode"/>
        <reference-descriptor name="searchChart" class-ref="org.kuali.kfs.coa.businessobject.Chart" auto-retrieve="true" auto-update="none" auto-delete="none" proxy="true">
	        <foreignkey field-ref="searchChartOfAccountsCode" />
	    <reference-descriptor name="searchAccount" class-ref="org.kuali.kfs.coa.businessobject.Account" auto-retrieve="true" auto-update="none" auto-delete="none" proxy="true">
	        <foreignkey field-ref="searchChartOfAccountsCode" />
	        <foreignkey field-ref="searchAccountNumber" />
	    <reference-descriptor name="searchSubAccount" class-ref="org.kuali.kfs.coa.businessobject.SubAccount" auto-retrieve="true" auto-update="none" auto-delete="none" proxy="true">
	        <foreignkey field-ref="searchChartOfAccountsCode" />
	        <foreignkey field-ref="searchAccountNumber" />
	        <foreignkey field-ref="searchSubAccountNumber" />
	    <reference-descriptor name="agencyServiceFee" class-ref="org.kuali.kfs.module.tem.businessobject.AgencyServiceFee" auto-retrieve="true" auto-update="none" auto-delete="none">
	    	<foreignkey field-ref="distributionCode"/>

I actually removed several field descriptors because they're kind of irrelevant and there's a ton of them...

And here's TripAccountingInformation:

	<class-descriptor class="org.kuali.kfs.module.tem.businessobject.TripAccountingInformation" table="TEM_TRP_ACCT_INFO_T">
		<field-descriptor name="id" primarykey="true" sequence-name="TEM_TRP_ACCT_INFO_ID_SEQ" autoincrement="true" column="id" jdbc-type="INTEGER"/>
		<field-descriptor name="agencyStagingDataId" column="AGENCY_ID" jdbc-type="INTEGER"/>
		<field-descriptor name="tripChartCode" column="FIN_COA_CD" jdbc-type="VARCHAR"/>
		<field-descriptor name="tripAccountNumber" column="ACCT_NBR" jdbc-type="VARCHAR"/>
		<field-descriptor name="tripSubAccountNumber" column="SUB_ACCT_NBR" jdbc-type="VARCHAR"/>
		<field-descriptor name="objectCode" column="OBJ_CD" jdbc-type="VARCHAR"/>
		<field-descriptor name="subObjectCode" column="SUB_OBJ_CD" jdbc-type="VARCHAR"/>
		<field-descriptor name="projectCode" column="PRJ_CD" jdbc-type="VARCHAR"/>
		<field-descriptor name="organizationReference" column="ORG_REF" jdbc-type="VARCHAR"/>
		<field-descriptor name="amount" column="AMOUNT" jdbc-type="DECIMAL" conversion="org.kuali.rice.core.framework.persistence.ojb.conversion.OjbKualiDecimalFieldConversion"/>
		<field-descriptor name="objectId" column="OBJ_ID" jdbc-type="VARCHAR"/>
		<field-descriptor name="versionNumber" locking="true" column="VER_NBR" jdbc-type="BIGINT"/>
	    <reference-descriptor name="account" class-ref="org.kuali.kfs.coa.businessobject.Account" auto-retrieve="true" auto-update="none" auto-delete="none" proxy="true">
            <foreignkey field-ref="tripChartCode" />
            <foreignkey field-ref="tripAccountNumber" />
        <reference-descriptor name="subAccount" class-ref="org.kuali.kfs.coa.businessobject.SubAccount" auto-retrieve="true" auto-update="none" auto-delete="none" proxy="true">
            <foreignkey field-ref="tripChartCode" />
            <foreignkey field-ref="tripAccountNumber" />
            <foreignkey field-ref="tripSubAccountNumber" />
        <reference-descriptor name="project" class-ref="org.kuali.kfs.coa.businessobject.ProjectCode" auto-retrieve="true" auto-update="none" auto-delete="none" proxy="true">
        	<foreignkey field-ref="projectCode" />
Comment by James Smith [ 12/Jan/15 ]

Okay, so in the AgencyStagingData example, I clear out tripAccountingInformation because a) it has a synthetic key and b) the relationship of AgencyStagingData to TripAccountingInformation is such that when AgencyStagingData is saved, I know that TripAccountingInformation is saved as well. So that's why when I copy AgencyStagingData, I clear out the id's there.

Having said that, I'm beginning to think that clearing out the agencyStagingId is kind of a mistake. Theoretically, TripAccountingInformation should have a relationship back to its parent object so that ojb can set that id. (The agencyStagingId is probably set manually somewhere else in the maintainable...I'd better check that...but if the ORM mapping had been done right, that shouldn't have been an issue.) In that case, ideally, you'd be able to find a relationship on the child object which points back to the parent object which is being copied, and you'd be able to clear out the key there.

Am I making sense? I'm kind of sick today so apologies if I'm unclear.

Comment by James Smith [ 12/Jan/15 ]

Oh - actually OJB probably knows to fill in agency staging data because of the inverse-foreignkey from its mapping for the collection of TripAccountingInformation objects. So yeah...that should be easy to find and clear as well.

Sorry for OJB mappings - KFS hasn't moved to JPA yet - but I am certain there are JPA ways to derive this information as well if need be.

Comment by Jeff Ruch [ 12/Jan/15 ]

Thanks. I think I got it. Let me play around with it. First day back from the flu for me, so I'm a little fuzzy today as well. Thanks again.

Comment by James Smith [ 12/Jan/15 ]

Totally understood re: the flu. Winter sucks. Thank you Jeff!

Comment by Jeff Ruch [ 15/Jan/15 ]

My understanding is that items with: auto-retrieve="true" , auto-update="object" , auto-delete="true" with an inverse foreign key in OJB need to have their pk cleared on copy if preserveLockingKeysOnCopy is not set.

Comment by James Smith [ 15/Jan/15 ]

I'm wondering if just auto-update would be the necessary criteria there. Certainly auto-retrieve is irrelevant. And I can imagine child objects inactivating instead of actually deleting, ie the auto-delete may not always convey child-objectness.

Comment by James Smith [ 15/Jan/15 ]

Added Jonathan as watcher to see if he agrees with my comment just made.

Comment by James Smith [ 15/Jan/15 ]

And added Douglas Pace, too, since he saw the KC versions of these, to see if he agrees with my assessment above. Thank you Jonathan and Doug!

Comment by Jeff Ruch [ 15/Jan/15 ]

I agree that auto-retrieve="true" is irrelevant. If I just check for auto-update, I'm assuming it will cover auto-delete. Is there ever a case where you configure auto-delete, without auto-update? Curious on what others have to say.

Comment by James Smith [ 15/Jan/15 ]

An auto-delete without an auto-update seems like...something would be wrong in the modelling. I can't think of an example off hand.

Comment by Douglas Pace [ 15/Jan/15 ]

I'm not sure if we've seen this in maint docs as we don't have many with child records, but we have frequently seen this in our trans docs when we allow copies or versioning. The new dataObjectService.copyInstance seems to handle this for us thus far.

Comment by Jonathan Keller [ 16/Jan/15 ]

Yes - just checking for auto-update would be correct on this one.

Delete without update - yes, that would be odd. Only situation I can think of that being used (auto-delete=yes,auto-update=no) is if you really needed to control when the child objects were updated. (But that had no purpose without their parent.)

Generated at Tue Sep 29 06:10:20 CDT 2020 using JIRA 7.0.11#70121-sha1:19d24976997c1d95f06f3e327e087be0b71f28d4.