Details

    • Type: Bug Fix
    • Status: Closed
    • Priority: 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
    • 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

        Attachments

          Issue Links

            Activity

            Hide
            jcoltrin 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
            jcoltrin 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
            shahess 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
            shahess 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
            shahess 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
            shahess 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
            gilesp 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
            gilesp 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
            shahess 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
            shahess 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
            shahess Shannon Hess added a comment -
            Show
            shahess Shannon Hess added a comment - I made this fix on the https://test.kuali.org/svn/rice/branches/rice-1.0 branch.
            Hide
            riceci 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
            riceci 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:
                shahess Shannon Hess
                Reporter:
                gtaylor 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