Details

    • Type: Bug Fix Bug Fix
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.3.2
    • Fix Version/s: 1.0.3.3
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-13110lookup searches do not ignore case even though forceUppercase=true
      KULRICE-4796MaintainableFieldDefinition's readOnlyAfterAdd flag is ignored in a specific scenario
      KULRICE-11712Suggest queries should be case insensitive
      KULRICE-4165Search on document description should be case insensitive
      KULRICE-85implement case-insensitive searching
      KULRICE-6687Person lookup isn't case-insensitive in Rice 2.0
      KULRICE-6817Document search by document type doesn't allow wildcards or case insensitive searches
      KULRICE-2996Allow for kim principal names to be case insensitive without requring principal name to be stored all lowercase in KRIM_PRNCPL_T
      KULRICE-13167Person lookups not ignoring case while doing wildcard searches
      KULRICE-7882KNS validationPattern property on AttributeDefinition being ignored
    • Rice Module:
      KNS
    • Application Requirement:
      KS
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required

      Description

      KS is doing a search via the LookupService. One of the fields is supposed to be case insensitive and we have it configured in the DD to be this way. But I've noticed that the lookupServiceImpl has a bug that negates the field.

      If you go to: LookupDaoOjb::getCollectionCriteriaFromMap there's a line:

      caseInsensitive = !KNSServiceLocator.getDataDictionaryService().getAttributeForceUppercase( example.getClass(), propertyName );

      that ! is causing the value to be flipped and make the caseInsensitive check break. People haven't noticed this yet because if you're using KNS screens the value is "toUppered" prior to reaching this method, but because KS is calling a web service the toUpper never happens and the search doesn't work.

      So the fix is... remove the !.

      Please make sure my logic is correct with this one.
      As far as KS1.2 we have a workaround in place so this isn't critical.

      Thanks,

      Garey

        Issue Links

          Activity

          Hide
          Jessica Coltrin (Inactive) added a comment -

          Shannon - please work on this on the 1.0 code line (https://test.kuali.org/svn/rice/branches/rice-1.0/). See Peter with any questions. Thanks!

          Show
          Jessica Coltrin (Inactive) added a comment - Shannon - please work on this on the 1.0 code line ( https://test.kuali.org/svn/rice/branches/rice-1.0/ ). See Peter with any questions. Thanks!
          Hide
          Shannon Hess added a comment -

          Unfortunately, I don't think the fix is simply removing the !. forceUppercase means that the user entry is converted to upper case and the database value is always displayed and stored as upper case. So the code in getCollectionCriteriaFromMap uses a value that doesn't really mean caseInsensitive to derive if the search should be case insensitive. If forceUppercase is true, both the database value and the user entry should be converted to uppercase, so the code changes caseInsensitive to false since we don't need to worry about the values not matching. However, if forceUppercase is set to false, it makes sure to do a caseInsensitive search because the database value and user entry could be mixed case. When I remove the !, it breaks many of the case insenstive searches within the sample application. (Namespace code and affiliation type code are good lookups for testing this code.)

          It sounds like since KS is calling a web service the toUpper on the user entry never happens which is breaking the logic in getCollectionCriteriaFromMap.

          QUESTION – How should the be handled? There is a LookupUtils.forceUppercase() method that can be called which checks the value of forceUppercase and returns an upper cased field value if forceUppercase is set to true. Is this something that should be done for all of the fields on the KS side rather than only calling .toUpperCase() for subjectCode? Or is this a deeper problem with how forceUppercase and caseInsensitive works?

          Thanks,
          Shannon

          Here's my comments to the code:

          in LookupDaoOjb::getCollectionCriteriaFromMap,

          public Criteria getCollectionCriteriaFromMap(BusinessObject example, Map formProps) {
          ...
          ...
          // Always make the search caseInsensitive by default
          Boolean caseInsensitive = Boolean.TRUE;

          // If the attribute is defined, go ahead and check to see if we can get rid of the case insensitive search criteria
          if ( KNSServiceLocator.getDataDictionaryService().isAttributeDefined( example.getClass(), propertyName ))

          { // If forceUppercase is true, both the database value and the user entry will be converted to Uppercase -- so change the caseInsensitive to false since we don't need to // worry about the values not matching. However, if the forceUppercase is false, make sure to do a caseInsensitive search because the database value and user entry // could be mixed case. So caseInsensitive should be the opposite of forceUppercase. caseInsensitive = !KNSServiceLocator.getDataDictionaryService().getAttributeForceUppercase( example.getClass(), propertyName ); }

          // if null is returned from the above getAttributeForceUppercase method, go ahead and set caseInsensitive back to true.
          if ( caseInsensitive == null )

          { caseInsensitive = Boolean.TRUE; }

          ...
          ...

          Show
          Shannon Hess added a comment - Unfortunately, I don't think the fix is simply removing the !. forceUppercase means that the user entry is converted to upper case and the database value is always displayed and stored as upper case. So the code in getCollectionCriteriaFromMap uses a value that doesn't really mean caseInsensitive to derive if the search should be case insensitive. If forceUppercase is true, both the database value and the user entry should be converted to uppercase, so the code changes caseInsensitive to false since we don't need to worry about the values not matching. However, if forceUppercase is set to false, it makes sure to do a caseInsensitive search because the database value and user entry could be mixed case. When I remove the !, it breaks many of the case insenstive searches within the sample application. (Namespace code and affiliation type code are good lookups for testing this code.) It sounds like since KS is calling a web service the toUpper on the user entry never happens which is breaking the logic in getCollectionCriteriaFromMap. QUESTION – How should the be handled? There is a LookupUtils.forceUppercase() method that can be called which checks the value of forceUppercase and returns an upper cased field value if forceUppercase is set to true. Is this something that should be done for all of the fields on the KS side rather than only calling .toUpperCase() for subjectCode? Or is this a deeper problem with how forceUppercase and caseInsensitive works? Thanks, Shannon Here's my comments to the code: in LookupDaoOjb::getCollectionCriteriaFromMap, public Criteria getCollectionCriteriaFromMap(BusinessObject example, Map formProps) { ... ... // Always make the search caseInsensitive by default Boolean caseInsensitive = Boolean.TRUE; // If the attribute is defined, go ahead and check to see if we can get rid of the case insensitive search criteria if ( KNSServiceLocator.getDataDictionaryService().isAttributeDefined( example.getClass(), propertyName )) { // If forceUppercase is true, both the database value and the user entry will be converted to Uppercase -- so change the caseInsensitive to false since we don't need to // worry about the values not matching. However, if the forceUppercase is false, make sure to do a caseInsensitive search because the database value and user entry // could be mixed case. So caseInsensitive should be the opposite of forceUppercase. caseInsensitive = !KNSServiceLocator.getDataDictionaryService().getAttributeForceUppercase( example.getClass(), propertyName ); } // if null is returned from the above getAttributeForceUppercase method, go ahead and set caseInsensitive back to true. if ( caseInsensitive == null ) { caseInsensitive = Boolean.TRUE; } ... ...
          Hide
          Shannon Hess added a comment -

          Hello All,

          Does anyone have any thoughts on the question in my last comment? To me it sounds like KS could call LookupUtils.forceUppercase() for all their fields. OR I could make ALL searches case insensitive since that's the intended behavior - the code causing the problem is used to reduce the number of times that case insensitive must be used. (i.e. If both search criteria and database value are upper case don't do a case insensitive search since it is not needed.)

          Thanks,
          Shannon

          Show
          Shannon Hess added a comment - Hello All, Does anyone have any thoughts on the question in my last comment? To me it sounds like KS could call LookupUtils.forceUppercase() for all their fields. OR I could make ALL searches case insensitive since that's the intended behavior - the code causing the problem is used to reduce the number of times that case insensitive must be used. (i.e. If both search criteria and database value are upper case don't do a case insensitive search since it is not needed.) Thanks, Shannon
          Hide
          Peter Giles (Inactive) added a comment - - edited

          Hi Shannon, so the heart of the issue is that forceUppercase is being mis-used for toggling case sensitivity on lookups. I'll see if any other KNS experts want to weigh in here, but it sounds to me like it shouldn't be used for that purpose, and that this is an optimization that breaks some valid use cases.

          Show
          Peter Giles (Inactive) added a comment - - edited Hi Shannon, so the heart of the issue is that forceUppercase is being mis-used for toggling case sensitivity on lookups. I'll see if any other KNS experts want to weigh in here, but it sounds to me like it shouldn't be used for that purpose, and that this is an optimization that breaks some valid use cases.
          Hide
          Shannon Hess added a comment -

          Since I didn't know the exact impact of always doing a case insensitive search, I made it so the search value is uppercased if the forceUppercase is set to true (case insensitive set to false). This is typically taken care of in LookupForm.java, where LookupUtils.forceUppercase is called. Since the KS calling a web service and the LookupUtils.forceUppercase is never called, I added similar processing in LookupDaoOjb.java when it is needed.

          Thanks to Peter and Jerry for the help!

          Comment from Jerry:
          I agree with Shannon’s last comment that it seems like we should just always do a case-insensitive search. If there is some issue with that (not sure how it is implemented, maybe it is inefficient to do so), then if the field is marked as forceUppercase do an uppercase before putting the value into the criteria. Regardless, I think it should be handled by the service, not requiring another method to be called beforehand in order for the search to work properly.

          Show
          Shannon Hess added a comment - Since I didn't know the exact impact of always doing a case insensitive search, I made it so the search value is uppercased if the forceUppercase is set to true (case insensitive set to false). This is typically taken care of in LookupForm.java, where LookupUtils.forceUppercase is called. Since the KS calling a web service and the LookupUtils.forceUppercase is never called, I added similar processing in LookupDaoOjb.java when it is needed. Thanks to Peter and Jerry for the help! Comment from Jerry: I agree with Shannon’s last comment that it seems like we should just always do a case-insensitive search. If there is some issue with that (not sure how it is implemented, maybe it is inefficient to do so), then if the field is marked as forceUppercase do an uppercase before putting the value into the criteria. Regardless, I think it should be handled by the service, not requiring another method to be called beforehand in order for the search to work properly.
          Hide
          Shannon Hess added a comment -
          Show
          Shannon Hess added a comment - I made this fix on the https://test.kuali.org/svn/rice/branches/rice-1.0 branch.
          Hide
          Rice-CI User (Inactive) added a comment -

          Integrated in rice-1.0-nightly #68 (See http://ci.rice.kuali.org/job/rice-1.0-nightly/68/)
          KULRICE-5249 - Made sure the searchValue was Uppercased when forceUpper is used in the DD so that case insensitive searches will work properly.

          Show
          Rice-CI User (Inactive) added a comment - Integrated in rice-1.0-nightly #68 (See http://ci.rice.kuali.org/job/rice-1.0-nightly/68/ ) KULRICE-5249 - Made sure the searchValue was Uppercased when forceUpper is used in the DD so that case insensitive searches will work properly.

            People

            • Assignee:
              Shannon Hess
              Reporter:
              Garey Taylor
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 1 hour Original Estimate - 1 hour
                1h
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 2 hours
                2h

                  Structure Helper Panel