Uploaded image for project: 'Kuali Rice Development'
  1. Kuali Rice Development
  2. KULRICE-8958

ScriptUtils convertToJsValue Performance Improvement

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.3.1
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Epic Link:
    • Rice Module:
      KRAD
    • KRAD Feature Area:
      Utility
    • Sprint:
      2.3.1 Sprint 2
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required

      Description

      ScriptUtils' convertToJsValue method is rather organically developed and can be modified slightly to minimize its impact on performance.

      Catching exceptions and parsing doubles are rather expensive operations. By utilizing precompiled regex patterns, a good deal of the performance hit is alleviated.

      The below is a different implementation for that method which can minimize this impact.

      static final Pattern firstChar = Pattern.compile("^\\{\\[FfTt");
      static final Pattern isDouble = Pattern.compile("^[-+]?\\d
      .?\\d
      $");
      public static String convertToJsValue(String value) {

      // save input value to preserve any whitespace formatting
      String originalValue = value;

      // remove whitespace for correct string matching
      value = StringUtils.strip(value);

      if (StringUtils.isNotBlank(value) && (isDouble.matcher(value).find()))
      return originalValue;

      if(!firstChar.matcher(value).find())
      return "'" + originalValue + "'";

      // If an option value starts with { or [, it would be a nested value
      // and it should not use quotes around it
      if (StringUtils.startsWith(value, "

      {") || StringUtils.startsWith(value, "[")) return originalValue; // need to be the base boolean value "false" is true in js - a non // empty string if (value.equalsIgnoreCase("false") || value.equalsIgnoreCase("true")) return originalValue; // if it is a call back function, do not add the quotes if (StringUtils.startsWith(value, "function") && StringUtils.endsWith(value, "}

      "))
      return originalValue;

      // use single quotes since hidden scripts are placed in the value attribute which surrounds the script with double quotes
      return "'" + originalValue + "'";
      }

        Attachments

          Issue Links

            Activity

            Hide
            jcoltrin Jessica Coltrin (Inactive) added a comment -

            moving out of scope for 2.3 as we narrow down to what's critical for release.

            Show
            jcoltrin Jessica Coltrin (Inactive) added a comment - moving out of scope for 2.3 as we narrow down to what's critical for release.
            Hide
            bsmith Brian Smith (Inactive) added a comment -

            These changes wont provide much gain because we already eliminated the exception catch bad code. The early return for string values may enhance it but I dont believe it is needed, now instead in our version I delay the isNumber check until it is deemed necessary, which might give small insignificant gains similar to the early return.

            Show
            bsmith Brian Smith (Inactive) added a comment - These changes wont provide much gain because we already eliminated the exception catch bad code. The early return for string values may enhance it but I dont believe it is needed, now instead in our version I delay the isNumber check until it is deemed necessary, which might give small insignificant gains similar to the early return.

              People

              • Assignee:
                bsmith Brian Smith (Inactive)
                Reporter:
                jdomeyer Jeff Domeyer (Inactive)
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - 30 minutes
                  30m
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 30 minutes
                  30m