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

Fix the way that "empty" KIM objects are handled

    Details

    • Type: Bug Fix Bug Fix
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.0.2
    • Component/s: Development
    • Labels:
      None
    • 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

        Issue Links

          Activity

          Hide
          Eric Westfall added a comment -

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

          Show
          Eric Westfall added a comment - Just had a chance to read through all of these comments. This all looks good peter thanks!
          Hide
          Peter Giles (Inactive) added a comment -

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

          Show
          Peter Giles (Inactive) added a comment - Changes committed. Used FindBugs to verify that no null dereferences of KimEntity* are occuring in Rice code.
          Hide
          Peter Giles (Inactive) added a comment -

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

          Show
          Peter Giles (Inactive) added a comment - Emailed Travis about reverting change to KimEntity & KimEntityPrivacyPreferences that made them extend ExternalizableBusinessObject.
          Hide
          Peter Giles (Inactive) added a comment -

          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

          Show
          Peter Giles (Inactive) added a comment - 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
          Hide
          Peter Giles (Inactive) added a comment -

          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.

          Show
          Peter Giles (Inactive) added a comment - 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.
          Hide
          Peter Giles (Inactive) added a comment - - edited

          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?

          Show
          Peter Giles (Inactive) added a comment - - edited 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?
          Hide
          Peter Giles (Inactive) added a comment -

          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).

          Show
          Peter Giles (Inactive) added a comment - 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).

            People

            • Assignee:
              Peter Giles (Inactive)
              Reporter:
              Eric Westfall
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Structure Helper Panel