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
<jacopo.cappell...@hotwaxmedia.com> 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 <jacques.le.r...@les7arts.com>
> 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, jler...@apache.org 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>
>
