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

Verify if EN_RTE_NODE_T.CONTENT_FRAGMENT is used anywhere and remove it if it is not

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.0
    • Component/s: Database, Development
    • Labels:
      None
    • Rice Module:
      KEW

      Description

      I think this must have been replaced by EN_RTE_NODE_CFG_PARM_T. However, please verify in the code whether or not this is the case and remove it if it's no longer being used. The BO represented by this table is org.kuali.rice.kew.engine.node.RouteNode.

      Removing this column would constitute:

      1) Add an ALTER_TABLE to 0.9.3->0.9.4 upgrade scripts
      2) Remove column from EN_RTE_NODE_T.table.ddl file
      3) Remove all references in code and OJB mappings
      4) Regenerate rice_db_bootstrap.sql and/or dataset generated by db import tool

        Attachments

          Issue Links

            Activity

            Hide
            ahamid Aaron Hamid (Inactive) added a comment -

            FYI, I added EN_RTE_NODE_CFG_PARM_T but kept the CONTENT_FRAGMENT column for backwards compatibility. IIRC the content fragment is also added to the EN_RTE_NODE_CFG_PARM_T as a node config param. RouteNode will set both but return the value from the config param table.

            Show
            ahamid Aaron Hamid (Inactive) added a comment - FYI, I added EN_RTE_NODE_CFG_PARM_T but kept the CONTENT_FRAGMENT column for backwards compatibility. IIRC the content fragment is also added to the EN_RTE_NODE_CFG_PARM_T as a node config param. RouteNode will set both but return the value from the config param table.
            Hide
            ewestfal Eric Westfall added a comment -

            Thanks Aaron. It seemed when I was looking in there that CONTENT_FRAGMENT was never read into or used anywhere in the code. Instead it fetches that value from EN_RTE_NODE_CFG_PARM_T using the "contentFragment" key. So there I don't think it would hurt to remove it unless it's discovered that the code is falling back onto EN_RTE_NODE_T.CONTENT_FRAGMENT if no entry is found in EN_RTE_NODE_CFG_PARM_T.

            Show
            ewestfal Eric Westfall added a comment - Thanks Aaron. It seemed when I was looking in there that CONTENT_FRAGMENT was never read into or used anywhere in the code. Instead it fetches that value from EN_RTE_NODE_CFG_PARM_T using the "contentFragment" key. So there I don't think it would hurt to remove it unless it's discovered that the code is falling back onto EN_RTE_NODE_T.CONTENT_FRAGMENT if no entry is found in EN_RTE_NODE_CFG_PARM_T.
            Hide
            stimble Shubhangi Timble (Inactive) added a comment -

            Eric, I found that the following classes were using contentFragment of RouteNode , updated their code to read this value from RouteNodeConfigParam.

            1 RouteNodeConfigParamTest
            2 org.kuali.rice.kew.mail.EmailNode
            3 org.kuali.rice.kew.engine.node.var.SetVarNode
            4 org.kuali.rice.kew.engine.node.LogNode
            5 org.kuali.rice.kew.engine.transition.LoopTransitionEngine
            6 org.kuali.rice.kew.xml.export.DocumentTypeXmlExporter
            7 org.kuali.rice.kew.routemodule.InlineRequestsRouteModule

            Show
            stimble Shubhangi Timble (Inactive) added a comment - Eric, I found that the following classes were using contentFragment of RouteNode , updated their code to read this value from RouteNodeConfigParam. 1 RouteNodeConfigParamTest 2 org.kuali.rice.kew.mail.EmailNode 3 org.kuali.rice.kew.engine.node.var.SetVarNode 4 org.kuali.rice.kew.engine.node.LogNode 5 org.kuali.rice.kew.engine.transition.LoopTransitionEngine 6 org.kuali.rice.kew.xml.export.DocumentTypeXmlExporter 7 org.kuali.rice.kew.routemodule.InlineRequestsRouteModule
            Hide
            ahamid Aaron Hamid (Inactive) added a comment -

            I don't think I created SQL to convert data in the existing column to the new table in the upgrade script when this was introduced - do we need to be concerned about historical docs that may not have been updated? The relationship is marked auto-update...so I think a load/save could possibly handle it.

            Show
            ahamid Aaron Hamid (Inactive) added a comment - I don't think I created SQL to convert data in the existing column to the new table in the upgrade script when this was introduced - do we need to be concerned about historical docs that may not have been updated? The relationship is marked auto-update...so I think a load/save could possibly handle it.
            Hide
            stimble Shubhangi Timble (Inactive) added a comment -

            org.kuali.rice.kew.xml.DocumentTypeXmlParser is setting the contentFragment in RouteNode, I have changed the code as given below, is it correct?

            private RouteNode makeRouteNodePrototype(String nodeTypeName, String nodeName, String nodeExpression, Node routeNodesNode, DocumentType documentType, RoutePathContext context) throws XPathExpressionException, InvalidWorkgroupException, InvalidXmlException, TransformerException {
            NodeList nodeList;
            try

            { nodeList = (NodeList) xpath.evaluate(nodeExpression, routeNodesNode, XPathConstants.NODESET); }

            catch (XPathExpressionException xpee)

            { LOG.error("Error evaluating node expression: '" + nodeExpression + "'"); throw xpee; }

            if (nodeList.getLength() > 1)

            { throw new InvalidXmlException("More than one node under routeNodes has the same name of '" + nodeName + "'"); }

            if (nodeList.getLength() == 0)

            { throw new InvalidXmlException("No node definition was found with the name '" + nodeName + "'"); }

            Node node = nodeList.item(0);

            RouteNode routeNode = new RouteNode();
            // set fields that all route nodes of all types should have defined
            routeNode.setDocumentType(documentType);
            routeNode.setRouteNodeName((String) xpath.evaluate("./@name", node, XPathConstants.STRING));
            //routeNode.setContentFragment(XmlHelper.writeNode(node));
            routeNode.getConfigParams().add(new RouteNodeConfigParam(routeNode,"contentFragment",node.getTextContent()));

            Show
            stimble Shubhangi Timble (Inactive) added a comment - org.kuali.rice.kew.xml.DocumentTypeXmlParser is setting the contentFragment in RouteNode, I have changed the code as given below, is it correct? private RouteNode makeRouteNodePrototype(String nodeTypeName, String nodeName, String nodeExpression, Node routeNodesNode, DocumentType documentType, RoutePathContext context) throws XPathExpressionException, InvalidWorkgroupException, InvalidXmlException, TransformerException { NodeList nodeList; try { nodeList = (NodeList) xpath.evaluate(nodeExpression, routeNodesNode, XPathConstants.NODESET); } catch (XPathExpressionException xpee) { LOG.error("Error evaluating node expression: '" + nodeExpression + "'"); throw xpee; } if (nodeList.getLength() > 1) { throw new InvalidXmlException("More than one node under routeNodes has the same name of '" + nodeName + "'"); } if (nodeList.getLength() == 0) { throw new InvalidXmlException("No node definition was found with the name '" + nodeName + "'"); } Node node = nodeList.item(0); RouteNode routeNode = new RouteNode(); // set fields that all route nodes of all types should have defined routeNode.setDocumentType(documentType); routeNode.setRouteNodeName((String) xpath.evaluate("./@name", node, XPathConstants.STRING)); //routeNode.setContentFragment(XmlHelper.writeNode(node)); routeNode.getConfigParams().add(new RouteNodeConfigParam(routeNode,"contentFragment",node.getTextContent()));
            Hide
            ewestfal Eric Westfall added a comment -

            Shubhangi, please go ahead and leave the getContentFragment and setContentFragments on RouteNode as is. Currently they are simply convenience methods which make it easier to fetch and set the content fragment. They should be implemented as follows:

            public String getContentFragment()

            { RouteNodeConfigParam cfCfgParam = getConfigParam(CONTENT_FRAGMENT_CFG_KEY); if (cfCfgParam == null) return null; return cfCfgParam.getValue(); }

            /**

            • @param contentFragment the content fragment of the node, which will be set as a RouteNodeConfigParam under the 'contentFragment' key
              */
              public void setContentFragment(String contentFragment) { setConfigParam(CONTENT_FRAGMENT_CFG_KEY, contentFragment); }

            Otherwise you have to end up manually executing these pieces of code anywhere you want to fetch the content fragment. For this issue I was mostly concerned with removing the column from the database.

            Show
            ewestfal Eric Westfall added a comment - Shubhangi, please go ahead and leave the getContentFragment and setContentFragments on RouteNode as is. Currently they are simply convenience methods which make it easier to fetch and set the content fragment. They should be implemented as follows: public String getContentFragment() { RouteNodeConfigParam cfCfgParam = getConfigParam(CONTENT_FRAGMENT_CFG_KEY); if (cfCfgParam == null) return null; return cfCfgParam.getValue(); } /** @param contentFragment the content fragment of the node, which will be set as a RouteNodeConfigParam under the 'contentFragment' key */ public void setContentFragment(String contentFragment) { setConfigParam(CONTENT_FRAGMENT_CFG_KEY, contentFragment); } Otherwise you have to end up manually executing these pieces of code anywhere you want to fetch the content fragment. For this issue I was mostly concerned with removing the column from the database.
            Hide
            ewestfal Eric Westfall added a comment -

            Regarding Aaron's question, I don't believe there was code anywhere which would have read the original value of CONTENT_FRAGMENT on EN_RTE_NODE_T so I'm not sure how it could have been using that column for backward compatibility (unless I missed it).

            However, for the sake of completeness:

            Shubhangi, please implement a script which copies from EN_RTE_NODE_CFG_PARM_T the appropriate "contentFragment" parameter from EN_RTE_NODE_T.CONTENT_FRAGMENT. However, if possible be sure to only do this if there is not already a contentFragment parameter in EN_RTE_NODE_CFG_PARM_T. The reason I say this is because it appears that this change was implemented way back on 0.9.2 but no upgrade script was created during that release to do the copy from one table to another.

            Show
            ewestfal Eric Westfall added a comment - Regarding Aaron's question, I don't believe there was code anywhere which would have read the original value of CONTENT_FRAGMENT on EN_RTE_NODE_T so I'm not sure how it could have been using that column for backward compatibility (unless I missed it). However, for the sake of completeness: Shubhangi, please implement a script which copies from EN_RTE_NODE_CFG_PARM_T the appropriate "contentFragment" parameter from EN_RTE_NODE_T.CONTENT_FRAGMENT. However, if possible be sure to only do this if there is not already a contentFragment parameter in EN_RTE_NODE_CFG_PARM_T . The reason I say this is because it appears that this change was implemented way back on 0.9.2 but no upgrade script was created during that release to do the copy from one table to another.
            Hide
            ewestfal Eric Westfall added a comment -

            I went ahead and added back the RouteNode.getContentFragment and setContentFragment methods. Their absence was causing some code to not compile and for this particular issue I was really only wanting to remove the column from the database

            Show
            ewestfal Eric Westfall added a comment - I went ahead and added back the RouteNode.getContentFragment and setContentFragment methods. Their absence was causing some code to not compile and for this particular issue I was really only wanting to remove the column from the database
            Hide
            ewestfal Eric Westfall added a comment -

            Bulk change of all Rice 1.0 issues to closed after public release.

            Show
            ewestfal Eric Westfall added a comment - Bulk change of all Rice 1.0 issues to closed after public release.

              People

              • Assignee:
                stimble Shubhangi Timble (Inactive)
                Reporter:
                ewestfal Eric Westfall
              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - 2 hours
                  2h
                  Remaining:
                  Remaining Estimate - 2 hours
                  2h
                  Logged:
                  Time Spent - Not Specified
                  Not Specified