[KULRICE-3801] Fix the way that "empty" KIM objects are handled Created: 07/Jan/10  Updated: 03/Nov/10  Resolved: 15/Jan/10

Status: Closed
Project: Kuali Rice Development
Component/s: Development
Affects Version/s: None
Fix Version/s: 1.0.2

Type: Bug Fix Priority: Critical
Reporter: Eric Westfall Assignee: Peter Giles (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Discovered
discovered by KULRICE-3766 org.kuali.rice.kim.test.service.remot... Closed
Similar issues:
KULRICE-3672Need to improve the way Rice handles remote applications when they are not accessible.
KULRICE-11779Fix DataDictionaryComponentPublisherServiceImpl to work with data objects
KULRICE-4890Direct Inquiry on KIM Objects does not handle invalid inputs
KULRICE-4023Make KualiCodeBase handle null values better.
KULRICE-10937Determine the best way to handle "refresh reference" and leverage EntityManager.getReference for JPA
KULRICE-2390Post-KIM Conversion: fix hard-coding handing of Person objects in core services
KULRICE-7082KIM role document not handling missing qualifiers on role members
KULRICE-3796JPA - Verify that the way we are handling sequences is the proper way to handle them moving forward
KULRICE-4173SectionBridge not checking for empty collections in old maintainables
KULRICE-3558Fix KIM unit tests
Rice Module:
KIM

 Description   

On the 1.0.x KC branch some changes were made to construct empty objects for certain KIM info objects whenever there was no corresponding record in the database.

In particular, we need to look at the places where the following methods are called:

DtoUtils.unNullify(Date)
DtoUtils.unNullify( T obj, Class<? extends T> typ)

We really don't want to be using those last two, instead, if there date does not exist it should be null, same with related objects. However, this was originally done for a reason because of some bugs in Rice code itself that was expecting some of this data to be non-null. So we need to track down that case and figure out which rice code needs to be fixed to do a null check. It looks like the offending method above is only used in this constructor:

KimEntityEntityTypeInfo

So there may be rice code trying to use some of the defaulted data there which wasn't properly doing a null check (assuming there was no default address, default email, etc. for the entity).

Here are the specific notes from the KTI meeting regarding this item:

  1. empty many to one relationships should be an empty list
  2. Null for other objects in one-to-one relationships
    • comes down to if we can make a reasonable assumption about what the values should be
  3. Do some work on the javadocs so they specify what the expected return value is if the data is there or not
    • particularly on kim entity default info and supporting classes
  4. Document what the expected values of the Strings, right now it's blank
  5. Geoff - also at some point we'll want to identify the minimun set of data for KIM for applications like KC, KFS, KS


 Comments   
Comment by Peter Giles (Inactive) [ 07/Jan/10 ]

I'm 99% sure this issue's fix version should be 1.0-kc. I'll change it (it is currently set to 1.0.1.1).

Comment by Peter Giles (Inactive) [ 07/Jan/10 ]

Am I interpreting correctly that getters for empty/null string members should return "", the empty string, not null?

Also, what should getters for Collection types return in the null case? Empty collections, or null?

Comment by Peter Giles (Inactive) [ 11/Jan/10 ]

I consulted Eric on Friday on what the policy on String and Collection members will be. The Collections part is actually redundant to the KTI notes above, but I'll restate it anyway

  • Getters for String types will return null if the member reference is null.
  • Getters for Collection types will assign and return empty Collections if the member reference is null.

I have another question on this, should these policies/behaviors be document at the interface or the Impl level (or both)? Seems like it would be useful to have that specified in the interfaces to make those contracts stronger, and therefore make it easier to craft new implementations that play well, but I'd like to double check that I'm not stepping out of bounds on that.

Comment by Peter Giles (Inactive) [ 11/Jan/10 ]

Doing some additional refactoring to avoid use of setter methods in constructors. For the motivation on this, see Ian Shef's comment here: http://bytes.com/topic/java/answers/473701-using-direct-access-constructor

Comment by Peter Giles (Inactive) [ 13/Jan/10 ]

Emailed Travis about reverting change to KimEntity & KimEntityPrivacyPreferences that made them extend ExternalizableBusinessObject.

Comment by Peter Giles (Inactive) [ 15/Jan/10 ]

Changes committed. Used FindBugs to verify that no null dereferences of KimEntity* are occuring in Rice code.

Comment by Eric Westfall [ 16/Jan/10 ]

Just had a chance to read through all of these comments. This all looks good peter thanks!

Generated at Tue Jun 02 11:35:07 CDT 2020 using JIRA 6.1.5#6160-sha1:a61a0fc278117a0da0ec9b89167b8f29b6afdab2.