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

Tie in KRAD PropertyEditors to ObjectPropertyUtils

    Details

    • Type: Bug Fix Bug Fix
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Complete
    • Affects Version/s: None
    • Fix Version/s: 2.5
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-5179Convert KNS Formatters to Spring PropertyEditors for use in KRAD
      KULRICE-10677Correct issues with ObjectPropertyUtils
      KULRICE-5600Uif Framework: Change Formatter property on AttributeField to PropertyEditor
      KULRICE-12099Need a propertyEditor equivalent to TimestampAMPMFormatter for KRAD document
      KULRICE-10878Simplify ObjectPropertyUtils
      KULRICE-10063Replace ObjectPropertyUtils BeanWrapper implementation
      KULRICE-10645Roll back recent ObjectPropertyUtils changes in rice-2.3 branch
      KULRICE-12697Property Editors no longer registered
      KULRICE-9473Convert all existing Formatter instances to PropertyEditor
      KULRICE-9212Rework KRAD Property Editors/Formatters
    • Rice Module:
      KRAD
    • KRAD Feature Area:
      UIF Tools
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes

      Description

      Thanks Jerry.

      I’ve moved the custom property editor register code from UifConfigurableWebBindingInitializer to a new method in ObjectPropertyUtils#registerPropertyEditors(PropertyEditorRegistry), then call this method from UifConfigurableWebBindingInitializer to set up the WebDataBinder. None of the calls there deviated from the PropertyEditorRegistry interfaces, so this seemed a good refactoring. In ObjectPropertyUtils, I also called this method on a static PropertyEditorRegistrySupport instance and am getting the right editor. This only catch now is that DateTimeService is not present in the unit test environment – a mock service should be easy enough to provide; I’ll plan to commit before tomorrow morning.

      It seems that the registerPropertyEditors method should pull its custom editors from a Spring context rather than hard-coding, but I haven’t made that migration yet. Do you think moving the PE setup to a Spring context is worthwhile?

      Note the ObjectPropertyUtils only uses PropertyEditors while setting properties, and doesn’t perform type conversion when getting properties. The PE is also only used in the case where a String is provided, but the property type is not String. In this case, there will either be a PropertyEditor or an error. At least for ObjectPropertyUtils, it seems we should need to worry about considering editors used for formatting, as long as they are able to convert from a string where appropriate.

      Best,
      Mark

      From: Jerry Neal jkneal@iu.edu
      Sent: Wednesday, April 02, 2014 9:42 AM
      To: 'Mark Fyffe'
      Subject: RE: property editor

      Hi Mark,

      I have never seen error that before.

      One of the things that would be great, and we have had as a task for quite awhile, is to hook into the property editors that are registered for the web binding. We do this in UifConfigurableWebBindingInitializer currently. Do you see any possibility with hooking into that instead of the data dictionary? In some cases though, you might not want to use the property editor. Sometimes we want to get the value for presentation, other times just the ‘model’ value. The issue is property editors are used both for data type conversion and ‘formatting’.

      Thanks,
      Jerry

      From: Mark Fyffe mark.w.fyffe@gmail.com
      Sent: Wednesday, April 2, 2014 7:17 AM
      To: 'Jerry Neal'
      Subject: RE: property editor

      I have reproduced the conversion error and added a condition to ObjectPropertyUtilsTest. It looks like I should be able to hook in to the DataDictionary’s bean factory to pull property editors from Spring. Once I have the test passing, I’ll add to JIRA and commit.

      I have 2.5 up to date now so can also take care of 11948 this week.

      One issue I’m having today is that I can’t start Tomcat from IntelliJ (krad-sampleapp Tomcat 7) using a fresh checkout of 2.5 – I see the error below in a red popup:

      Error running krad sampleapp (tomcat 7):
      Cannot run program "C:\cygwin\opt\kuali\servers\apache-tomcat-7.0.53\bin\catalina.bat" (in directory "C:\cygwin\opt\kuali\servers\apache-tomcat-7.0.53\bin"): CreateProcess error=5, Access is denied

      The tomcat install works just fine if I drop the war in manually and start up from the command line. Does the IntelliJ error look familiar?

      Best,
      Mark

      From: Mark Fyffe mark.w.fyffe@gmail.com
      Sent: Tuesday, April 01, 2014 7:47 AM
      To: 'Jerry Neal'
      Subject: RE: property editor

      No problem, Jerry.

      I should have time to look into this later today or tomorrow.

      Thanks,
      Mark

      From: Jerry Neal jkneal@iu.edu
      Sent: Monday, March 31, 2014 12:01 PM
      To: 'Mark Fyffe'
      Subject: property editor

      Hi Mark,

      I am working on updating my training project to 2.5.0-M1. I ran into an exception using the new code:

      Caused by: java.lang.IllegalArgumentException: No property editor available for converting '01/03/2013' to class java.util.Date
      at org.kuali.rice.krad.uif.util.ObjectPropertyReference.convertStringToPropertyType(ObjectPropertyReference.java:480)
      at org.kuali.rice.krad.uif.util.ObjectPropertyReference.convertToPropertyType(ObjectPropertyReference.java:546)
      at org.kuali.rice.krad.uif.util.ObjectPropertyReference.set(ObjectPropertyReference.java:897)

      This is being caused by a default value of '01/03/2013' on an input field.

      Its throwing an exception because it cannot find an editor. But Spring should be able to handle this conversion, and it was working before. Would you be able to take a look at this next time you work?

      Thanks!
      Jerry

        Issue Links

          Activity

          Hide
          Mark Fyffe (Inactive) added a comment -

          Also look into this request on this JIRA:

          Mark,

          Can you take a look at UifViewBeanWrapper# getPropertyNameTokens (and PropertyTokenHolder) and move it to ObjectPropertyUtils? I suspect it might duplicate some logic already there.

          Thanks,
          Jerry

          Show
          Mark Fyffe (Inactive) added a comment - Also look into this request on this JIRA: Mark, Can you take a look at UifViewBeanWrapper# getPropertyNameTokens (and PropertyTokenHolder) and move it to ObjectPropertyUtils? I suspect it might duplicate some logic already there. Thanks, Jerry
          Hide
          Mark Fyffe (Inactive) added a comment -

          Another item for this request:

          • Convert property editor setup to configure editors from Spring
          • Provide formatted property value references, with view awareness

          Super! This is really good. Couple of comments. One, moving editor registration to the context would be great. That should allow applications to add editors without having to override the binding class. I am wondering about org.kuali.rice.krad.web.bind.UifViewBeanWrapper#registerEditorFromView as well. This registers editors for particular paths (not by type), but were specified on the data/input field. If this were available in ObjectPropertyUtils that would be great as well. But here is a timing issue. UifViewBeanWrapper doesn’t get called until there is a get/set for the view, and ObjectPropertyUtils might get invoked before that. It also needs to be view aware.

          Could we have a method on ObjectPropertyUtils, getFormattedPropertyValue that would use the property editor and return a String? There are cases where we need to get the value of a property in code for display in the view (this has been one of the gaps).
          ...
          Thanks,
          Jerry

          Updated estimate.

          Show
          Mark Fyffe (Inactive) added a comment - Another item for this request: Convert property editor setup to configure editors from Spring Provide formatted property value references, with view awareness Super! This is really good. Couple of comments. One, moving editor registration to the context would be great. That should allow applications to add editors without having to override the binding class. I am wondering about org.kuali.rice.krad.web.bind.UifViewBeanWrapper#registerEditorFromView as well. This registers editors for particular paths (not by type), but were specified on the data/input field. If this were available in ObjectPropertyUtils that would be great as well. But here is a timing issue. UifViewBeanWrapper doesn’t get called until there is a get/set for the view, and ObjectPropertyUtils might get invoked before that. It also needs to be view aware. Could we have a method on ObjectPropertyUtils, getFormattedPropertyValue that would use the property editor and return a String? There are cases where we need to get the value of a property in code for display in the view (this has been one of the gaps). ... Thanks, Jerry Updated estimate.
          Hide
          Mark Fyffe (Inactive) added a comment -

          Completed implementation for this issue, but have a little more testing and analysis to work through.

          The code to initialize PropertyEditorRegistry via ObjectPropertyUtils now loads a "propertyEditorMap" from DataDictionaryService, which is configured using editors defined by /org/kuail/rice/krad/uif/PropertyEditors.xml. When setting up any PropertyEditorRegistry instance, the map's entries are iterated, and each editor is registered. This first implementation aims to duplicate the previously hard-coded behavior.

          In addition, I have added a servlet request attribute for holding a request the property editor registry in support of tying in property editors that are view aware - view awareness comes from UifServletRequestDataBinder, which is responsible for loading the view onto the form as well as binding properties to the form based on user input. Once the form has been bound, the same databinder becomes the PropertyEditorRegistry instance for ObjectPropertyUtils for the life of the request.

          I've created a code review for the changes: https://fisheye.kuali.org/cru/rice-390

          Still TODO:

          • Identify unit tests for proving view-bound property editor behavior
          • Provide ObjectPropertyUtils#getPropertyValueAsText()
          • Evaluate the need for the behavior evident in getPropertyNameTokens()
          Show
          Mark Fyffe (Inactive) added a comment - Completed implementation for this issue, but have a little more testing and analysis to work through. The code to initialize PropertyEditorRegistry via ObjectPropertyUtils now loads a "propertyEditorMap" from DataDictionaryService, which is configured using editors defined by /org/kuail/rice/krad/uif/PropertyEditors.xml. When setting up any PropertyEditorRegistry instance, the map's entries are iterated, and each editor is registered. This first implementation aims to duplicate the previously hard-coded behavior. In addition, I have added a servlet request attribute for holding a request the property editor registry in support of tying in property editors that are view aware - view awareness comes from UifServletRequestDataBinder, which is responsible for loading the view onto the form as well as binding properties to the form based on user input. Once the form has been bound, the same databinder becomes the PropertyEditorRegistry instance for ObjectPropertyUtils for the life of the request. I've created a code review for the changes: https://fisheye.kuali.org/cru/rice-390 Still TODO: Identify unit tests for proving view-bound property editor behavior Provide ObjectPropertyUtils#getPropertyValueAsText() Evaluate the need for the behavior evident in getPropertyNameTokens()
          Hide
          Mark Fyffe (Inactive) added a comment -

          Added #getPropertyValueAsText(), and set up a unit test for testing a view-bound property editor.

          However, the test was skipped due to a missing UIF component required for UifUnitTestUtils to initialize. Correcting this issue led to ViewLifecycleTest running, but several of those tests are now failing. Will continue working through those failures before commit in the morning.

          Show
          Mark Fyffe (Inactive) added a comment - Added #getPropertyValueAsText(), and set up a unit test for testing a view-bound property editor. However, the test was skipped due to a missing UIF component required for UifUnitTestUtils to initialize. Correcting this issue led to ViewLifecycleTest running, but several of those tests are now failing. Will continue working through those failures before commit in the morning.
          Hide
          Mark Fyffe (Inactive) added a comment -

          The remaining updates turned out to be a bit more involved that originally thought. I worked through test failures after correcting setup issues with ViewLifecycleTest and ObjectPropertyUtilsTest, however this yielded a significant issue with the original implementation in that a global PropertyEditorRegistry is not a stable configuration since Spring expects one registry per scenario (per bean factory, per bean wrapper, etc). Running multiple unit tests in varying order led to failures due to the global configuration. For example, if ViewLifecycleTest ran before ObjectPropertyUtilsTest, then ObjectPropertyUtilsTest would fail due to PropertyEditor-related expectations. However, run alone ObjectPropertyUtilsTest would succeed.

          To address this issue, I updated krad-web-framework as follows:

          • In order to restore ViewLifecycleTest operation, as well as restore service for in-lifecycle rendering, I added MockHttpServletResonse to LifecycleRenderingContext and uncommented constructor calls for ViewLifecycleProcessor.
          • To prevent strict mode failures related to duplicate lifecycle phase processing, I reduced the related illegal state check to a warning.
          • Converted propertyEditorMap to a mapping from property type to prototype bean name, referring to the a bean name in the data dictionary.
          • Added getDictionaryPrototype() method to DataDictionary. This method is identical to getDictionaryBean(), except when attempting to access a singleton an exception is thrown.
          • Converted ObjectPropertyUtils to use DD bean prototypes directly when PropertyEditorRegistry is not available on the current request, instead of using a global registry. A new prototype instance is created for each call.
          • Refactored code related to PropertyEditor in UifViewBeanWrapperImpl and ObjectPropertyReference to eliminate duplicate functionality.

          These updates have been committed on trunk. I'll review CI and update the Crucible review in the morning and resolve the issue is all appears to be in working order.

          Show
          Mark Fyffe (Inactive) added a comment - The remaining updates turned out to be a bit more involved that originally thought. I worked through test failures after correcting setup issues with ViewLifecycleTest and ObjectPropertyUtilsTest, however this yielded a significant issue with the original implementation in that a global PropertyEditorRegistry is not a stable configuration since Spring expects one registry per scenario (per bean factory, per bean wrapper, etc). Running multiple unit tests in varying order led to failures due to the global configuration. For example, if ViewLifecycleTest ran before ObjectPropertyUtilsTest, then ObjectPropertyUtilsTest would fail due to PropertyEditor-related expectations. However, run alone ObjectPropertyUtilsTest would succeed. To address this issue, I updated krad-web-framework as follows: In order to restore ViewLifecycleTest operation, as well as restore service for in-lifecycle rendering, I added MockHttpServletResonse to LifecycleRenderingContext and uncommented constructor calls for ViewLifecycleProcessor. To prevent strict mode failures related to duplicate lifecycle phase processing, I reduced the related illegal state check to a warning. Converted propertyEditorMap to a mapping from property type to prototype bean name, referring to the a bean name in the data dictionary. Added getDictionaryPrototype() method to DataDictionary. This method is identical to getDictionaryBean(), except when attempting to access a singleton an exception is thrown. Converted ObjectPropertyUtils to use DD bean prototypes directly when PropertyEditorRegistry is not available on the current request, instead of using a global registry. A new prototype instance is created for each call. Refactored code related to PropertyEditor in UifViewBeanWrapperImpl and ObjectPropertyReference to eliminate duplicate functionality. These updates have been committed on trunk. I'll review CI and update the Crucible review in the morning and resolve the issue is all appears to be in working order.

            People

            • Assignee:
              Mark Fyffe (Inactive)
              Reporter:
              Jerry Neal (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 6 hours Original Estimate - 6 hours
                6h
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 1 day, 6 hours
                1d 6h

                  Structure Helper Panel