http://ofbiz.116.s1.nabble.com/Re-svn-commit-r1518777-in-ofbiz-trunk-applications-accounting-webapp-accounting-WEB-INF-applications-tp4643588p4643868.html
I think you are right. I will revert and test again with the suggestion you made for the generation of help links
> Jacques,
>
> I have spent a lot of time reviewing the whole history behind these changes and also all the comments in the Jira ticket
> OFBIZ-5267.
> Now I think that you should revert this change.
>
> Here is a summary of what happened:
>
> 1) in rev. 1361130 I have moved the "birt" component to specialpurpose; I have also moved all the Birt reports from the
> applications to the webapp I have defined in the specialpurpose/birt component, leveraging the ability of the framework to
> override existing applications: in this way the birt component was adding to the applications the birt reports; when you remove
> the birt component then the reports disappear nicely; this design is based on best practices and I spent a lot of time to follow
> it properly when I have separated the birt component 2) then it was reported that in the overriden webapps the contextual help
> links didn't work any more; this issue is not limited to the applications overriden by the specialpurpose/birt component but it
> is a general issue caused by a wrong design in the generation of help links; specifically, the issue is that in
>
> <set field="helpTopic" value="${groovy: parameters.componentName.toUpperCase() + '_' + requestAttributes._CURRENT_VIEW_}"/>
>
> the name of the help file is composed using the component name; this is a wrong approach and could be solved, for instance, by
> using the webapp name instead of the component name;
>
> 3) instead of fixing the help link creation (e.g., as suggested above) you simply, in rev. 1400463, commented out the webapp
> declarations in specialpurpose/birt (WRONG), canceling a relevant part of my initial work (#1); with this change you have
> disabled all Birt reports
>
> 4) then it was reported (in OFBIZ-5267) that a Birt report was not available (this because of your change in #3): after a lot of
> confusion (see history of comments) you wrote it was caused by my commit at #1 and you implemented in r1518777 an ugly mechanism
> where birt webapp controllers are included in the applications controllers but with a new attribute (requiring framework
> modifications) to avoid errors if the birt component is missing (WRONG)
>
> I am now asking you to revert all the work you did in #3 and #4; we will then focus on improving the help system, whose design is
> ugly, to make it work properly with the overriden webapp mechanism that OFBiz supports.
>
> In the future, please pay more attention in making changes that increase entropy in the project.
>
> Regards,
>
> Jacopo
>
> On Aug 30, 2013, at 8:17 AM, Jacopo Cappellato <
[hidden email]> wrote:
>
>> It is annoying to see that we are still adding these references (or loose dependencies) from applications to specialpurpose...
>> it is ugly. I will try to find some time to think to a better solution.
>>
>> Jacopo
>>
>> On Aug 29, 2013, at 11:39 PM, Jacques Le Roux <
[hidden email]> wrote:
>>
>>> No problems with me, you can change if you want. Note though that, for now, it's not "the missing configuration file" which
>>> "will not prevent the webapp from loading" but only if the whole component is absent.
>>>
>>> Jacques
>>>
>>> Adrian Crum wrote:
>>>> I would prefer an attribute name like "optional" - indicating the
>>>> include is optional and the missing configuration file will not prevent
>>>> the webapp from loading.
>>>>
>>>> -Adrian
>>>>
>>>> On 8/29/2013 12:37 PM,
[hidden email] wrote:
>>>>> Author: jleroux
>>>>> Date: Thu Aug 29 19:37:44 2013
>>>>> New Revision: 1518777
>>>>>
>>>>> URL:
http://svn.apache.org/r1518777>>>>> Log:
>>>>> This fixes "Net before overhead report generates an error"
https://issues.apache.org/jira/browse/OFBIZ-5267>>>>>
>>>>> This is related with r1361130, where the birt component was moved from framework to specialpurpose.
>>>>>
>>>>> The fix is to create the possibility of a loose coupling from applications controllers to specialpurpose controllers. For that
>>>>> an if-present attribute is added to the include element. When used, if the included controller does not exist, instead of an
>>>>> error and a stack trace, a warning is thrown (only in English, for developers) suggesting to checkout the component from
>>>>> trunk. Indeed, in the current case, this situation should only arise in release after 12.04 where the birt component, like
>>>>> all others but the ecommerce component, had been removed. This can be used in other cases, but for now only the component
>>>>> level is covered.
>>>>>
>>>>> Modified:
>>>>> ofbiz/trunk/applications/accounting/webapp/accounting/WEB-INF/controller.xml
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/controller.xml
>>>>> ofbiz/trunk/applications/product/webapp/facility/WEB-INF/controller.xml
>>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/location/ComponentLocationResolver.java
>>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java
>>>>> ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>>>>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>>>>> ofbiz/trunk/specialpurpose/birt/ofbiz-component.xml
>>>>> ofbiz/trunk/specialpurpose/birt/webapp/accounting/WEB-INF/controller.xml
>>>>> ofbiz/trunk/specialpurpose/birt/webapp/facility/WEB-INF/controller.xml
>>>>> ofbiz/trunk/specialpurpose/birt/webapp/ordermgr/WEB-INF/controller.xml
>>>>>
>>>>> Modified: ofbiz/trunk/applications/accounting/webapp/accounting/WEB-INF/controller.xml
>>>>> URL:
>>>>>
http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/webapp/accounting/WEB-INF/controller.xml?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ============================================================================== ---
>>>>> ofbiz/trunk/applications/accounting/webapp/accounting/WEB-INF/controller.xml (original) +++
>>>>> ofbiz/trunk/applications/accounting/webapp/accounting/WEB-INF/controller.xml Thu Aug 29 19:37:44 2013 @@ -22,6 +22,7 @@ under
>>>>> the License. xsi:noNamespaceSchemaLocation="
http://ofbiz.apache.org/dtds/site-conf.xsd">
>>>>> <include location="component://common/webcommon/WEB-INF/common-controller.xml"/>
>>>>> <include location="component://commonext/webapp/WEB-INF/controller.xml"/>
>>>>> + <include location="component://birt/webapp/accounting/WEB-INF/controller.xml" if-present="true"/>
>>>>> <description>Accounting Manager Module Site Configuration File</description>
>>>>>
>>>>> <!-- Events to run on every request before security (chains exempt) -->
>>>>>
>>>>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/controller.xml
>>>>> URL:
>>>>>
http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/controller.xml?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ============================================================================== ---
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/controller.xml (original) +++
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/controller.xml Thu Aug 29 19:37:44 2013 @@ -23,6 +23,7 @@ under the
>>>>> License. <include location="component://common/webcommon/WEB-INF/common-controller.xml"/>
>>>>> <include location="component://commonext/webapp/WEB-INF/controller.xml"/>
>>>>> <include location="component://content/webapp/content/WEB-INF/controller.xml"/>
>>>>> + <include location="component://birt/webapp/ordermgr/WEB-INF/controller.xml" if-present="true"/>
>>>>> <description>Order Manager Module Site Configuration File</description>
>>>>>
>>>>> <!-- event handlers -->
>>>>>
>>>>> Modified: ofbiz/trunk/applications/product/webapp/facility/WEB-INF/controller.xml
>>>>> URL:
>>>>>
http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/webapp/facility/WEB-INF/controller.xml?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ============================================================================== ---
>>>>> ofbiz/trunk/applications/product/webapp/facility/WEB-INF/controller.xml (original) +++
>>>>> ofbiz/trunk/applications/product/webapp/facility/WEB-INF/controller.xml Thu Aug 29 19:37:44 2013 @@ -22,6 +22,8 @@ under the
>>>>> License. xsi:noNamespaceSchemaLocation="
http://ofbiz.apache.org/dtds/site-conf.xsd">
>>>>> <include location="component://common/webcommon/WEB-INF/common-controller.xml"/>
>>>>> <include location="component://commonext/webapp/WEB-INF/controller.xml"/>
>>>>> + <include location="component://birt/webapp/accounting/WEB-INF/controller.xml" if-present="true"/>
>>>>> +
>>>>> <description>Facility Manager Module Site Configuration File</description>
>>>>>
>>>>> <handler name="service-multi" type="request" class="org.ofbiz.webapp.event.ServiceMultiEventHandler"/>
>>>>>
>>>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/location/ComponentLocationResolver.java
>>>>> URL:
>>>>>
http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/location/ComponentLocationResolver.java?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ============================================================================== ---
>>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/location/ComponentLocationResolver.java (original) +++
>>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/location/ComponentLocationResolver.java Thu Aug 29 19:37:44 2013 @@ -38,7 +38,7
>>>>> @@ public class ComponentLocationResolver i public static final String module = ComponentLocationResolver.class.getName();
>>>>>
>>>>> public URL resolveLocation(String location) throws MalformedURLException {
>>>>> - String baseLocation = getBaseLocation(location).toString();
>>>>> + String baseLocation = getBaseLocation(location, false).toString();
>>>>> if (File.separatorChar != '/') {
>>>>> baseLocation = baseLocation.replace(File.separatorChar, '/');
>>>>> }
>>>>> @@ -52,7 +52,7 @@ public class ComponentLocationResolver i
>>>>> }
>>>>> }
>>>>>
>>>>> - public static StringBuilder getBaseLocation(String location) throws MalformedURLException {
>>>>> + public static StringBuilder getBaseLocation(String location, boolean ifPresent) throws MalformedURLException {
>>>>> StringBuilder baseLocation = new StringBuilder(FlexibleLocation.stripLocationType(location));
>>>>> // componentName is between the first slash and the second
>>>>> int firstSlash = baseLocation.indexOf("/");
>>>>> @@ -73,9 +73,13 @@ public class ComponentLocationResolver i
>>>>> baseLocation.insert(0, rootLocation);
>>>>> return baseLocation;
>>>>> } catch (ComponentException e) {
>>>>> - String errMsg = "Could not get root location for component with name [" + componentName + "], error was: " +
>>>>> e.toString();
>>>>> - Debug.logError(e, errMsg, module);
>>>>> - throw new MalformedURLException(errMsg);
>>>>> + if (!ifPresent) {
>>>>> + String errMsg = "Could not get root location for component with name [" + componentName + "], error was: " +
>>>>> e.toString(); + Debug.logError(e, errMsg, module);
>>>>> + throw new MalformedURLException(errMsg);
>>>>> + } else {
>>>>> + return null;
>>>>> + }
>>>>> }
>>>>> - }
>>>>> + }
>>>>> }
>>>>>
>>>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java
>>>>> URL:
>>>>>
http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ============================================================================== ---
>>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java (original) +++
>>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java Thu Aug 29 19:37:44 2013 @@ -38,9 +38,8 @@ import
>>>>> java.util.Set; import javolution.util.FastList;
>>>>> import javolution.util.FastSet;
>>>>>
>>>>> -import org.ofbiz.base.location.ComponentLocationResolver;
>>>>> -
>>>>> import org.apache.commons.io.FileUtils;
>>>>> +import org.ofbiz.base.location.ComponentLocationResolver;
>>>>>
>>>>> /**
>>>>> * File Utilities
>>>>> @@ -57,7 +56,7 @@ public class FileUtil {
>>>>> public static File getFile(File root, String path) {
>>>>> if (path.startsWith("component://")) {
>>>>> try {
>>>>> - path = ComponentLocationResolver.getBaseLocation(path).toString();
>>>>> + path = ComponentLocationResolver.getBaseLocation(path, false).toString();
>>>>> } catch (MalformedURLException e) {
>>>>> Debug.logError(e, module);
>>>>> return null;
>>>>>
>>>>> Modified: ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>>>>> URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/dtd/site-conf.xsd?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/framework/webapp/dtd/site-conf.xsd (original)
>>>>> +++ ofbiz/trunk/framework/webapp/dtd/site-conf.xsd Thu Aug 29 19:37:44 2013
>>>>> @@ -55,6 +55,17 @@ under the License.
>>>>> </xs:element>
>>>>> <xs:attributeGroup name="attlist.include">
>>>>> <xs:attribute type="xs:string" name="location" use="required"/>
>>>>> + <xs:attribute type="xs:boolean" name="if-present" use="optional" default="true">
>>>>> + <xs:annotation>
>>>>> + <xs:documentation>
>>>>> + Only used with component type location (component://). This allows to introduce a loose coupling to
>>>>> another component. +
>>>>> + If true, if the component is absent it will be ignored.
>>>>> + A warning will be logged, to let know the component can possibly be checked out from trunk HEAD
>>>>> + (since R13.07, in releases specialpurpose components are removed but ecommerce).
>>>>> + </xs:documentation>
>>>>> + </xs:annotation>
>>>>> + </xs:attribute>
>>>>> </xs:attributeGroup>
>>>>> <xs:element name="description" type="xs:string"/>
>>>>> <xs:element name="owner" type="xs:string"/>
>>>>>
>>>>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>>>>> URL:
>>>>>
http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ============================================================================== ---
>>>>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java (original) +++
>>>>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java Thu Aug 29 19:37:44 2013 @@ -32,6 +32,7 @@
>>>>> import javolution.util.FastList; import javolution.util.FastMap;
>>>>> import javolution.util.FastSet;
>>>>>
>>>>> +import org.ofbiz.base.location.ComponentLocationResolver;
>>>>> import org.ofbiz.base.location.FlexibleLocation;
>>>>> import org.ofbiz.base.metrics.Metrics;
>>>>> import org.ofbiz.base.metrics.MetricsFactory;
>>>>> @@ -295,7 +296,24 @@ public class ConfigXMLReader {
>>>>> protected void loadIncludes(Element rootElement) {
>>>>> for (Element includeElement: UtilXml.childElementList(rootElement, "include")) {
>>>>> String includeLocation = includeElement.getAttribute("location");
>>>>> + boolean testIfPresent = "true".equals(includeElement.getAttribute("if-present"));
>>>>> if (UtilValidate.isNotEmpty(includeLocation)) {
>>>>> + if (includeLocation.startsWith("component://") && testIfPresent) {
>>>>> + try {
>>>>> + if (null == ComponentLocationResolver.getBaseLocation(includeLocation, true)) {
>>>>> + Debug.logWarning(includeLocation + " does not exist." + " Since R13.07, in releases,
>>>>> specialpurpose components were removed but ecommerce." + + " If you need this
>>>>> component, you might check it out from Apache OFBiz trunk HEAD (
http://svn.apache.org/repos/asf/ofbiz/trunk)." +
>>>>> + " Else, you can simply neglect this warning.", module); + continue;
>>>>> + }
>>>>> + } catch (MalformedURLException mue) {
>>>>> + Debug.logWarning("While trying to retrieve " + includeLocation + " an error occured (but
>>>>> if-present was used, so it's maybe only a typo)." + + " Also note that since R13.07, in
>>>>> releases, specialpurpose components were removed but ecommerce." + + " If you need this
>>>>> component, you might check it out from Apache OFBiz trunk HEAD (
http://svn.apache.org/repos/asf/ofbiz/trunk)." +
>>>>> + " Else, you can simply neglect this warning.", module); + continue;
>>>>> + }
>>>>> + }
>>>>> try {
>>>>> URL urlLocation = FlexibleLocation.resolveLocation(includeLocation);
>>>>> includes.add(urlLocation);
>>>>>
>>>>> Modified: ofbiz/trunk/specialpurpose/birt/ofbiz-component.xml
>>>>> URL:
>>>>>
http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/birt/ofbiz-component.xml?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ============================================================================== ---
>>>>> ofbiz/trunk/specialpurpose/birt/ofbiz-component.xml (original) +++ ofbiz/trunk/specialpurpose/birt/ofbiz-component.xml Thu
>>>>> Aug 29 19:37:44 2013 @@ -29,28 +29,6 @@ under the License.
>>>>> <entity-resource type="data" reader-name="seed" loader="main" location="data/OrderPortletData.xml"/>
>>>>> <service-resource type="model" loader="main" location="servicedef/services.xml"/>
>>>>>
>>>>> - <!-- use when reports need to be injected into applications Note: this will break contextual help for those applications.
>>>>> - <webapp name="accounting"
>>>>> - title="Accounting"
>>>>> - server="default-server"
>>>>> - location="webapp/accounting"
>>>>> - base-permission="OFBTOOLS,ACCOUNTING"
>>>>> - mount-point="/accounting"/>
>>>>> - <webapp name="facility"
>>>>> - title="Facility"
>>>>> - description="FacilityComponentDescription"
>>>>> - server="default-server"
>>>>> - location="webapp/facility"
>>>>> - base-permission="OFBTOOLS,FACILITY"
>>>>> - mount-point="/facility"/>
>>>>> - <webapp name="order"
>>>>> - title="Order"
>>>>> - description="OrderComponentDescription"
>>>>> - server="default-server"
>>>>> - location="webapp/ordermgr"
>>>>> - base-permission="OFBTOOLS,ORDERMGR"
>>>>> - mount-point="/ordermgr"/>
>>>>> - -->
>>>>> <webapp name="birt"
>>>>> title="BIRT"
>>>>> server="default-server"
>>>>>
>>>>> Modified: ofbiz/trunk/specialpurpose/birt/webapp/accounting/WEB-INF/controller.xml
>>>>> URL:
>>>>>
http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/birt/webapp/accounting/WEB-INF/controller.xml?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ============================================================================== ---
>>>>> ofbiz/trunk/specialpurpose/birt/webapp/accounting/WEB-INF/controller.xml (original) +++
>>>>> ofbiz/trunk/specialpurpose/birt/webapp/accounting/WEB-INF/controller.xml Thu Aug 29 19:37:44 2013 @@ -20,7 +20,6 @@ under the
>>>>> License.
>>>>>
>>>>> <site-conf xmlns:xsi="
http://www.w3.org/2001/XMLSchema-instance"
>>>>> xsi:noNamespaceSchemaLocation="
http://ofbiz.apache.org/dtds/site-conf.xsd">
>>>>> - <include location="component://accounting/webapp/accounting/WEB-INF/controller.xml"/>
>>>>>
>>>>> <description>Extended Accounting Controller Configuration File</description>
>>>>>
>>>>>
>>>>> Modified: ofbiz/trunk/specialpurpose/birt/webapp/facility/WEB-INF/controller.xml
>>>>> URL:
>>>>>
http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/birt/webapp/facility/WEB-INF/controller.xml?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ============================================================================== ---
>>>>> ofbiz/trunk/specialpurpose/birt/webapp/facility/WEB-INF/controller.xml (original) +++
>>>>> ofbiz/trunk/specialpurpose/birt/webapp/facility/WEB-INF/controller.xml Thu Aug 29 19:37:44 2013 @@ -20,7 +20,6 @@ under the
>>>>> License.
>>>>>
>>>>> <site-conf xmlns:xsi="
http://www.w3.org/2001/XMLSchema-instance"
>>>>> xsi:noNamespaceSchemaLocation="
http://ofbiz.apache.org/dtds/site-conf.xsd">
>>>>> - <include location="component://product/webapp/facility/WEB-INF/controller.xml"/>
>>>>>
>>>>> <description>Extended Facility Manager Controller Configuration File</description>
>>>>>
>>>>>
>>>>> Modified: ofbiz/trunk/specialpurpose/birt/webapp/ordermgr/WEB-INF/controller.xml
>>>>> URL:
>>>>>
http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/birt/webapp/ordermgr/WEB-INF/controller.xml?rev=1518777&r1=1518776&r2=1518777&view=diff>>>>> ============================================================================== ---
>>>>> ofbiz/trunk/specialpurpose/birt/webapp/ordermgr/WEB-INF/controller.xml (original) +++
>>>>> ofbiz/trunk/specialpurpose/birt/webapp/ordermgr/WEB-INF/controller.xml Thu Aug 29 19:37:44 2013 @@ -20,7 +20,6 @@ under the
>>>>> License.
>>>>>
>>>>> <site-conf xmlns:xsi="
http://www.w3.org/2001/XMLSchema-instance"
>>>>> xsi:noNamespaceSchemaLocation="
http://ofbiz.apache.org/dtds/site-conf.xsd">
>>>>> - <include location="component://order/webapp/ordermgr/WEB-INF/controller.xml"/>
>>>>>
>>>>> <description>Extended Order Manager Controller Configuration File</description>