You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adrian Crum <ad...@yahoo.com> on 2010/07/31 16:17:36 UTC

Re: svn commit: r981018 - in /ofbiz/trunk/framework/service: dtd/services.xsd src/org/ofbiz/service/ModelParam.java src/org/ofbiz/service/ModelServiceReader.java

Scott,

Are you sure this does what you intended? The parameters are being assigned the service's description, not the parameter's description.

-Adrian

--- On Sat, 7/31/10, lektran@apache.org <le...@apache.org> wrote:

> From: lektran@apache.org <le...@apache.org>
> Subject: svn commit: r981018 - in /ofbiz/trunk/framework/service: dtd/services.xsd src/org/ofbiz/service/ModelParam.java src/org/ofbiz/service/ModelServiceReader.java
> To: commits@ofbiz.apache.org
> Date: Saturday, July 31, 2010, 1:26 AM
> Author: lektran
> Date: Sat Jul 31 08:26:42 2010
> New Revision: 981018
> 
> URL: http://svn.apache.org/viewvc?rev=981018&view=rev
> Log:
> Add a description element for service attributes, should
> make documenting some services a little easier.  Still
> need to display it in webtools somewhere.
> 
> Modified:
>    
> ofbiz/trunk/framework/service/dtd/services.xsd
>    
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>    
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> 
> Modified: ofbiz/trunk/framework/service/dtd/services.xsd
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/dtd/services.xsd?rev=981018&r1=981017&r2=981018&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/service/dtd/services.xsd
> (original)
> +++ ofbiz/trunk/framework/service/dtd/services.xsd Sat Jul
> 31 08:26:42 2010
> @@ -314,6 +314,7 @@ under the License.
>      
>    <xs:complexType>
>          
>    <xs:sequence>
>              
>    <xs:element minOccurs="0"
> maxOccurs="unbounded" ref="type-validate"/>
> +               
> <xs:element minOccurs="0" ref="description" />
>          
>    </xs:sequence>
>          
>    <xs:attributeGroup
> ref="attlist.attribute"/>
>      
>    </xs:complexType>
> 
> Modified:
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java?rev=981018&r1=981017&r2=981018&view=diff
> ==============================================================================
> ---
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> (original)
> +++
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> Sat Jul 31 08:26:42 2010
> @@ -43,6 +43,9 @@ public class ModelParam implements Seria
>      /** Parameter name */
>      public String name;
>  
> +    /** The description of this parameter */
> +    public String description;
> +
>      /** Paramater type */
>      public String type;
>  
> @@ -88,6 +91,7 @@ public class ModelParam implements Seria
>  
>      public ModelParam(ModelParam
> param) {
>          this.name =
> param.name;
> +        this.description =
> param.description;
>          this.type =
> param.type;
>          this.mode =
> param.mode;
>          this.formLabel =
> param.formLabel;
> 
> Modified:
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java?rev=981018&r1=981017&r2=981018&view=diff
> ==============================================================================
> ---
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> (original)
> +++
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> Sat Jul 31 08:26:42 2010
> @@ -518,6 +518,7 @@ public class ModelServiceReader
> implemen
>          
>    ModelParam param = new ModelParam();
>  
>          
>    param.name =
> UtilXml.checkEmpty(attribute.getAttribute("name")).intern();
> +           
> param.description = getCDATADef(baseElement,
> "description");
>          
>    param.type =
> UtilXml.checkEmpty(attribute.getAttribute("type")).intern();
>          
>    param.mode =
> UtilXml.checkEmpty(attribute.getAttribute("mode")).intern();
>          
>    param.entityName =
> UtilXml.checkEmpty(attribute.getAttribute("entity-name")).intern();
> 
> 
> 


      

Re: svn commit: r981018 - in /ofbiz/trunk/framework/service: dtd/services.xsd src/org/ofbiz/service/ModelParam.java src/org/ofbiz/service/ModelServiceReader.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Inline...

Scott Gray wrote:
> On 1/08/2010, at 11:24 AM, Adrian Crum wrote:
>
>> --- On Sat, 7/31/10, Scott Gray <sc...@hotwaxmedia.com> wrote:
>>> Yeah whoops, thanks for fixing that
>>> up.
>>>
>>> Inline
>>>
>>> Regards
>>> Scott
>>>
>>> On 1/08/2010, at 3:55 AM, Adrian Crum wrote:
>>>
>>>> Scott,
>>>>
>>>> I fixed the code and built it out a little.
>>>>
>>>> This is a good idea, but I have a couple of problems
>>> with it:
>>>>
>>>> 1. User-oriented meta-data should be kept in
>>> properties files - so it can be internationalized. The
>>> drawback to that is, developers will be less likely to
>>> include descriptions if they have to do it in a separate
>>> file.
>>>
>>> It wasn't my intention for the meta-data to be oriented
>>> towards the user, I see it as developer documentation
>>> only.  A few times in the past I've noticed it can be
>>> difficult to describe an attributes purpose from the service
>>> description and inlining them was intended to make it a
>>> little more like a javadoc.
>>
>> By user I meant developer. If it's developer information only, then why store it in the model? A developer could just look at
>> the service definition file.
>
> I agree it doesn't necessarily need to be in the model, webtools could just as easily go and look at the service def itself.

+1

>>>> 2. Description text should not be stored in the model
>>> - it increases the model's memory footprint greatly.
>>> Instead, the descriptions should be retrieved from
>>> properties files when needed (in AvailableServices.groovy,
>>> for example).
>>>
>>> Well I agree with you except for the properties file part,
>>> but I guess that comes back to #1 above.  We could
>>> always look at doing some sort of lazy loading, where the
>>> descriptions are only added to the model if they're
>>> requested.  Considering that viewing the service in
>>> webtools is the only way to expose the descriptions, you'd
>>> probably never end up with more than a few dozen actually
>>> loaded into memory.
>>
>> The reason I suggested the properties file is because this issue came up before with the entity field descriptions. At the time
>> those were being added, there was interest in internationalizing them.
>
> Maybe we should internationalize java comments as well?  Just because people are interested in something doesn't make it a good
> idea.

I agree, we have enough work already...

>> From my perspective, using a properties file is easier than the existing implementation. In the AvailableServices.groovy script,
>> concatenate the service name and attribute name, then use that as a key to look up the description in a properties file. No need
>> to add anything to the model.
>
> I don't think we are so good at documentation that we should be okay with making it any harder :-)

Yep!

Jacques

>>>>
>>>> -Adrian
>>>>
>>>>
>>>> --- On Sat, 7/31/10, Adrian Crum <ad...@yahoo.com>
>>> wrote:
>>>>> Scott,
>>>>>
>>>>> Are you sure this does what you intended? The
>>> parameters
>>>>> are being assigned the service's description, not
>>> the
>>>>> parameter's description.
>>>>>
>>>>> -Adrian
>>>>>
>>>>> --- On Sat, 7/31/10, lektran@apache.org
>>>>> <le...@apache.org>
>>>>> wrote:
>>>>>
>>>>>> From: lektran@apache.org
>>>>> <le...@apache.org>
>>>>>> Subject: svn commit: r981018 - in
>>>>> /ofbiz/trunk/framework/service: dtd/services.xsd
>>>>> src/org/ofbiz/service/ModelParam.java
>>>>> src/org/ofbiz/service/ModelServiceReader.java
>>>>>> To: commits@ofbiz.apache.org
>>>>>> Date: Saturday, July 31, 2010, 1:26 AM
>>>>>> Author: lektran
>>>>>> Date: Sat Jul 31 08:26:42 2010
>>>>>> New Revision: 981018
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=981018&view=rev
>>>>>> Log:
>>>>>> Add a description element for service attributes, should
>>>>>> make documenting some services a little easier. Still
>>>>>> need to display it in webtools somewhere.
>>>>>>
>>>>>> Modified:
>>>>>>
>>>>>>
>>> ofbiz/trunk/framework/service/dtd/services.xsd
>>>>>>
>>>>>>
>>>>>
>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>>>
>>>>>>
>>>>>
>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>>>
>>>>>> Modified:
>>>>> ofbiz/trunk/framework/service/dtd/services.xsd
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/dtd/services.xsd?rev=981018&r1=981017&r2=981018&view=diff
>>>>>>
>>>>>
>>> ==============================================================================
>>>>>> ---
>>> ofbiz/trunk/framework/service/dtd/services.xsd
>>>>>> (original)
>>>>>> +++ ofbiz/trunk/framework/service/dtd/services.xsd Sat Jul
>>>>>> 31 08:26:42 2010
>>>>>> @@ -314,6 +314,7 @@ under the License.
>>>>>>
>>>>>>     <xs:complexType>
>>>>>>
>>>>>>     <xs:sequence>
>>>>>>
>>>
>>>>>>     <xs:element minOccurs="0"
>>>>>> maxOccurs="unbounded"
>>> ref="type-validate"/>
>>>>>> +
>>>
>>>>>> <xs:element minOccurs="0" ref="description"
>>> />
>>>>>>
>>>>>>     </xs:sequence>
>>>>>>
>>>>>>     <xs:attributeGroup
>>>>>> ref="attlist.attribute"/>
>>
>>>>>>
>>>>>>     </xs:complexType>
>>>>>>
>>>>>> Modified:
>>>>>>
>>>>>
>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>>> URL:
>>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java?rev=981018&r1=981017&r2=981018&view=diff
>>>>>>
>>>>>
>>> ==============================================================================
>>>>>> ---
>>>>>>
>>>>>
>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>>> (original)
>>>>>> +++
>>>>>>
>>>>>
>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>>> Sat Jul 31 08:26:42 2010
>>>>>> @@ -43,6 +43,9 @@ public class ModelParam implements Seria
>>>>>>       /** Parameter name */
>>>>>>       public String name;
>>>>>>
>>>>>> +    /** The description of this
>>> parameter */
>>>>>> +    public String description;
>>>>>> +
>>>>>>       /** Paramater type */
>>>>>>       public String type;
>>>>>>
>>>>>> @@ -88,6 +91,7 @@ public class ModelParam implements Seria
>>>>>>
>>>>>>       public
>>> ModelParam(ModelParam
>>>>>> param) {
>>>>>>           this.name =
>>>>>> param.name;
>>>>>> +        this.description
>>> =
>>>>>> param.description;
>>>>>>           this.type =
>>>>>> param.type;
>>>>>>           this.mode =
>>>>>> param.mode;
>>>>>>
>>> this.formLabel =
>>>>>> param.formLabel;
>>>>>>
>>>>>> Modified:
>>>>>>
>>>>>
>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>>> URL:
>>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java?rev=981018&r1=981017&r2=981018&view=diff
>>>>>>
>>>>>
>>> ==============================================================================
>>>>>> ---
>>>>>>
>>>>>
>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>>> (original)
>>>>>> +++
>>>>>>
>>>>>
>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>>> Sat Jul 31 08:26:42 2010
>>>>>> @@ -518,6 +518,7 @@ public class
>>> ModelServiceReader
>>>>>> implemen
>>>>>>
>>>>>>     ModelParam param = new
>>> ModelParam();
>>>>>>
>>>>>>
>>>>>>     param.name =
>>>>>>
>>>>>
>>> UtilXml.checkEmpty(attribute.getAttribute("name")).intern();
>>>>>> +
>>>
>>>>>> param.description = getCDATADef(baseElement,
>>>>>> "description");
>>>>>>
>>>>>>     param.type =
>>>>>>
>>>>>
>>> UtilXml.checkEmpty(attribute.getAttribute("type")).intern();
>>>>>>
>>>>>>     param.mode =
>>>>>>
>>>>>
>>> UtilXml.checkEmpty(attribute.getAttribute("mode")).intern();
>>>>>>
>>>>>>     param.entityName =
>>>>>>
>>>>>
>>> UtilXml.checkEmpty(attribute.getAttribute("entity-name")).intern();



Re: svn commit: r981018 - in /ofbiz/trunk/framework/service: dtd/services.xsd src/org/ofbiz/service/ModelParam.java src/org/ofbiz/service/ModelServiceReader.java

Posted by Scott Gray <sc...@hotwaxmedia.com>.
On 1/08/2010, at 11:24 AM, Adrian Crum wrote:

> --- On Sat, 7/31/10, Scott Gray <sc...@hotwaxmedia.com> wrote:
>> Yeah whoops, thanks for fixing that
>> up.
>> 
>> Inline
>> 
>> Regards
>> Scott
>> 
>> On 1/08/2010, at 3:55 AM, Adrian Crum wrote:
>> 
>>> Scott,
>>> 
>>> I fixed the code and built it out a little.
>>> 
>>> This is a good idea, but I have a couple of problems
>> with it:
>>> 
>>> 1. User-oriented meta-data should be kept in
>> properties files - so it can be internationalized. The
>> drawback to that is, developers will be less likely to
>> include descriptions if they have to do it in a separate
>> file.
>> 
>> It wasn't my intention for the meta-data to be oriented
>> towards the user, I see it as developer documentation
>> only.  A few times in the past I've noticed it can be
>> difficult to describe an attributes purpose from the service
>> description and inlining them was intended to make it a
>> little more like a javadoc.
> 
> By user I meant developer. If it's developer information only, then why store it in the model? A developer could just look at the service definition file.

I agree it doesn't necessarily need to be in the model, webtools could just as easily go and look at the service def itself.

>>> 2. Description text should not be stored in the model
>> - it increases the model's memory footprint greatly.
>> Instead, the descriptions should be retrieved from
>> properties files when needed (in AvailableServices.groovy,
>> for example).
>> 
>> Well I agree with you except for the properties file part,
>> but I guess that comes back to #1 above.  We could
>> always look at doing some sort of lazy loading, where the
>> descriptions are only added to the model if they're
>> requested.  Considering that viewing the service in
>> webtools is the only way to expose the descriptions, you'd
>> probably never end up with more than a few dozen actually
>> loaded into memory.
> 
> The reason I suggested the properties file is because this issue came up before with the entity field descriptions. At the time those were being added, there was interest in internationalizing them.

Maybe we should internationalize java comments as well?  Just because people are interested in something doesn't make it a good idea.

> From my perspective, using a properties file is easier than the existing implementation. In the AvailableServices.groovy script, concatenate the service name and attribute name, then use that as a key to look up the description in a properties file. No need to add anything to the model.

I don't think we are so good at documentation that we should be okay with making it any harder :-)

>>> 
>>> -Adrian
>>> 
>>> 
>>> --- On Sat, 7/31/10, Adrian Crum <ad...@yahoo.com>
>> wrote:
>>>> Scott,
>>>> 
>>>> Are you sure this does what you intended? The
>> parameters
>>>> are being assigned the service's description, not
>> the
>>>> parameter's description.
>>>> 
>>>> -Adrian
>>>> 
>>>> --- On Sat, 7/31/10, lektran@apache.org
>>>> <le...@apache.org>
>>>> wrote:
>>>> 
>>>>> From: lektran@apache.org
>>>> <le...@apache.org>
>>>>> Subject: svn commit: r981018 - in
>>>> /ofbiz/trunk/framework/service: dtd/services.xsd
>>>> src/org/ofbiz/service/ModelParam.java
>>>> src/org/ofbiz/service/ModelServiceReader.java
>>>>> To: commits@ofbiz.apache.org
>>>>> Date: Saturday, July 31, 2010, 1:26 AM
>>>>> Author: lektran
>>>>> Date: Sat Jul 31 08:26:42 2010
>>>>> New Revision: 981018
>>>>> 
>>>>> URL: http://svn.apache.org/viewvc?rev=981018&view=rev
>>>>> Log:
>>>>> Add a description element for service
>> attributes,
>>>> should
>>>>> make documenting some services a little
>> easier. 
>>>> Still
>>>>> need to display it in webtools somewhere.
>>>>> 
>>>>> Modified:
>>>>>     
>>>>> 
>> ofbiz/trunk/framework/service/dtd/services.xsd
>>>>>     
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>>     
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>> 
>>>>> Modified:
>>>> ofbiz/trunk/framework/service/dtd/services.xsd
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/dtd/services.xsd?rev=981018&r1=981017&r2=981018&view=diff
>>>>> 
>>>> 
>> ==============================================================================
>>>>> ---
>> ofbiz/trunk/framework/service/dtd/services.xsd
>>>>> (original)
>>>>> +++
>> ofbiz/trunk/framework/service/dtd/services.xsd Sat
>>>> Jul
>>>>> 31 08:26:42 2010
>>>>> @@ -314,6 +314,7 @@ under the License.
>>>>>       
>>>>>     <xs:complexType>
>>>>>           
>>>>>     <xs:sequence>
>>>>>            
>>   
>>>>>     <xs:element minOccurs="0"
>>>>> maxOccurs="unbounded"
>> ref="type-validate"/>
>>>>> +           
>>    
>>>>> <xs:element minOccurs="0" ref="description"
>> />
>>>>>           
>>>>>     </xs:sequence>
>>>>>           
>>>>>     <xs:attributeGroup
>>>>> ref="attlist.attribute"/>
> 
>>>>>       
>>>>>     </xs:complexType>
>>>>> 
>>>>> Modified:
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java?rev=981018&r1=981017&r2=981018&view=diff
>>>>> 
>>>> 
>> ==============================================================================
>>>>> ---
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>> (original)
>>>>> +++
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>> Sat Jul 31 08:26:42 2010
>>>>> @@ -43,6 +43,9 @@ public class ModelParam
>> implements
>>>> Seria
>>>>>       /** Parameter name */
>>>>>       public String name;
>>>>>    
>>>>> +    /** The description of this
>> parameter */
>>>>> +    public String description;
>>>>> +
>>>>>       /** Paramater type */
>>>>>       public String type;
>>>>>    
>>>>> @@ -88,6 +91,7 @@ public class ModelParam
>> implements
>>>> Seria
>>>>>    
>>>>>       public
>> ModelParam(ModelParam
>>>>> param) {
>>>>>           this.name =
>>>>> param.name;
>>>>> +        this.description
>> =
>>>>> param.description;
>>>>>           this.type =
>>>>> param.type;
>>>>>           this.mode =
>>>>> param.mode;
>>>>>          
>> this.formLabel =
>>>>> param.formLabel;
>>>>> 
>>>>> Modified:
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java?rev=981018&r1=981017&r2=981018&view=diff
>>>>> 
>>>> 
>> ==============================================================================
>>>>> ---
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>> (original)
>>>>> +++
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>> Sat Jul 31 08:26:42 2010
>>>>> @@ -518,6 +518,7 @@ public class
>> ModelServiceReader
>>>>> implemen
>>>>>           
>>>>>     ModelParam param = new
>> ModelParam();
>>>>>    
>>>>>           
>>>>>     param.name =
>>>>> 
>>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("name")).intern();
>>>>> +       
>>    
>>>>> param.description = getCDATADef(baseElement,
>>>>> "description");
>>>>>           
>>>>>     param.type =
>>>>> 
>>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("type")).intern();
>>>>>           
>>>>>     param.mode =
>>>>> 
>>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("mode")).intern();
>>>>>           
>>>>>     param.entityName =
>>>>> 
>>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("entity-name")).intern();
>>>>> 
>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>> 
>>> 
>>> 
>> 
>> 
> 
> 
> 


Re: svn commit: r981018 - in /ofbiz/trunk/framework/service: dtd/services.xsd src/org/ofbiz/service/ModelParam.java src/org/ofbiz/service/ModelServiceReader.java

Posted by Adrian Crum <ad...@yahoo.com>.
--- On Sat, 7/31/10, Scott Gray <sc...@hotwaxmedia.com> wrote:
> Yeah whoops, thanks for fixing that
> up.
> 
> Inline
> 
> Regards
> Scott
> 
> On 1/08/2010, at 3:55 AM, Adrian Crum wrote:
> 
> > Scott,
> > 
> > I fixed the code and built it out a little.
> > 
> > This is a good idea, but I have a couple of problems
> with it:
> > 
> > 1. User-oriented meta-data should be kept in
> properties files - so it can be internationalized. The
> drawback to that is, developers will be less likely to
> include descriptions if they have to do it in a separate
> file.
> 
> It wasn't my intention for the meta-data to be oriented
> towards the user, I see it as developer documentation
> only.  A few times in the past I've noticed it can be
> difficult to describe an attributes purpose from the service
> description and inlining them was intended to make it a
> little more like a javadoc.

By user I meant developer. If it's developer information only, then why store it in the model? A developer could just look at the service definition file.

> > 2. Description text should not be stored in the model
> - it increases the model's memory footprint greatly.
> Instead, the descriptions should be retrieved from
> properties files when needed (in AvailableServices.groovy,
> for example).
> 
> Well I agree with you except for the properties file part,
> but I guess that comes back to #1 above.  We could
> always look at doing some sort of lazy loading, where the
> descriptions are only added to the model if they're
> requested.  Considering that viewing the service in
> webtools is the only way to expose the descriptions, you'd
> probably never end up with more than a few dozen actually
> loaded into memory.

The reason I suggested the properties file is because this issue came up before with the entity field descriptions. At the time those were being added, there was interest in internationalizing them.

From my perspective, using a properties file is easier than the existing implementation. In the AvailableServices.groovy script, concatenate the service name and attribute name, then use that as a key to look up the description in a properties file. No need to add anything to the model.

> > 
> > -Adrian
> > 
> > 
> > --- On Sat, 7/31/10, Adrian Crum <ad...@yahoo.com>
> wrote:
> >> Scott,
> >> 
> >> Are you sure this does what you intended? The
> parameters
> >> are being assigned the service's description, not
> the
> >> parameter's description.
> >> 
> >> -Adrian
> >> 
> >> --- On Sat, 7/31/10, lektran@apache.org
> >> <le...@apache.org>
> >> wrote:
> >> 
> >>> From: lektran@apache.org
> >> <le...@apache.org>
> >>> Subject: svn commit: r981018 - in
> >> /ofbiz/trunk/framework/service: dtd/services.xsd
> >> src/org/ofbiz/service/ModelParam.java
> >> src/org/ofbiz/service/ModelServiceReader.java
> >>> To: commits@ofbiz.apache.org
> >>> Date: Saturday, July 31, 2010, 1:26 AM
> >>> Author: lektran
> >>> Date: Sat Jul 31 08:26:42 2010
> >>> New Revision: 981018
> >>> 
> >>> URL: http://svn.apache.org/viewvc?rev=981018&view=rev
> >>> Log:
> >>> Add a description element for service
> attributes,
> >> should
> >>> make documenting some services a little
> easier. 
> >> Still
> >>> need to display it in webtools somewhere.
> >>> 
> >>> Modified:
> >>>    
> >>>
> ofbiz/trunk/framework/service/dtd/services.xsd
> >>>    
> >>> 
> >>
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> >>>    
> >>> 
> >>
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> >>> 
> >>> Modified:
> >> ofbiz/trunk/framework/service/dtd/services.xsd
> >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/dtd/services.xsd?rev=981018&r1=981017&r2=981018&view=diff
> >>> 
> >>
> ==============================================================================
> >>> ---
> ofbiz/trunk/framework/service/dtd/services.xsd
> >>> (original)
> >>> +++
> ofbiz/trunk/framework/service/dtd/services.xsd Sat
> >> Jul
> >>> 31 08:26:42 2010
> >>> @@ -314,6 +314,7 @@ under the License.
> >>>      
> >>>    <xs:complexType>
> >>>          
> >>>    <xs:sequence>
> >>>           
>   
> >>>    <xs:element minOccurs="0"
> >>> maxOccurs="unbounded"
> ref="type-validate"/>
> >>> +           
>    
> >>> <xs:element minOccurs="0" ref="description"
> />
> >>>          
> >>>    </xs:sequence>
> >>>          
> >>>    <xs:attributeGroup
> >>> ref="attlist.attribute"/>

> >>>      
> >>>    </xs:complexType>
> >>> 
> >>> Modified:
> >>> 
> >>
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java?rev=981018&r1=981017&r2=981018&view=diff
> >>> 
> >>
> ==============================================================================
> >>> ---
> >>> 
> >>
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> >>> (original)
> >>> +++
> >>> 
> >>
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> >>> Sat Jul 31 08:26:42 2010
> >>> @@ -43,6 +43,9 @@ public class ModelParam
> implements
> >> Seria
> >>>      /** Parameter name */
> >>>      public String name;
> >>>   
> >>> +    /** The description of this
> parameter */
> >>> +    public String description;
> >>> +
> >>>      /** Paramater type */
> >>>      public String type;
> >>>   
> >>> @@ -88,6 +91,7 @@ public class ModelParam
> implements
> >> Seria
> >>>   
> >>>      public
> ModelParam(ModelParam
> >>> param) {
> >>>          this.name =
> >>> param.name;
> >>> +        this.description
> =
> >>> param.description;
> >>>          this.type =
> >>> param.type;
> >>>          this.mode =
> >>> param.mode;
> >>>         
> this.formLabel =
> >>> param.formLabel;
> >>> 
> >>> Modified:
> >>> 
> >>
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java?rev=981018&r1=981017&r2=981018&view=diff
> >>> 
> >>
> ==============================================================================
> >>> ---
> >>> 
> >>
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> >>> (original)
> >>> +++
> >>> 
> >>
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> >>> Sat Jul 31 08:26:42 2010
> >>> @@ -518,6 +518,7 @@ public class
> ModelServiceReader
> >>> implemen
> >>>          
> >>>    ModelParam param = new
> ModelParam();
> >>>   
> >>>          
> >>>    param.name =
> >>> 
> >>
> UtilXml.checkEmpty(attribute.getAttribute("name")).intern();
> >>> +       
>    
> >>> param.description = getCDATADef(baseElement,
> >>> "description");
> >>>          
> >>>    param.type =
> >>> 
> >>
> UtilXml.checkEmpty(attribute.getAttribute("type")).intern();
> >>>          
> >>>    param.mode =
> >>> 
> >>
> UtilXml.checkEmpty(attribute.getAttribute("mode")).intern();
> >>>          
> >>>    param.entityName =
> >>> 
> >>
> UtilXml.checkEmpty(attribute.getAttribute("entity-name")).intern();
> >>> 
> >>> 
> >>> 
> >> 
> >> 
> >> 
> >> 
> > 
> > 
> > 
> 
> 


      

Re: svn commit: r981018 - in /ofbiz/trunk/framework/service: dtd/services.xsd src/org/ofbiz/service/ModelParam.java src/org/ofbiz/service/ModelServiceReader.java

Posted by Scott Gray <sc...@hotwaxmedia.com>.
Yeah whoops, thanks for fixing that up.

Inline

Regards
Scott

On 1/08/2010, at 3:55 AM, Adrian Crum wrote:

> Scott,
> 
> I fixed the code and built it out a little.
> 
> This is a good idea, but I have a couple of problems with it:
> 
> 1. User-oriented meta-data should be kept in properties files - so it can be internationalized. The drawback to that is, developers will be less likely to include descriptions if they have to do it in a separate file.

It wasn't my intention for the meta-data to be oriented towards the user, I see it as developer documentation only.  A few times in the past I've noticed it can be difficult to describe an attributes purpose from the service description and inlining them was intended to make it a little more like a javadoc.

> 2. Description text should not be stored in the model - it increases the model's memory footprint greatly. Instead, the descriptions should be retrieved from properties files when needed (in AvailableServices.groovy, for example).

Well I agree with you except for the properties file part, but I guess that comes back to #1 above.  We could always look at doing some sort of lazy loading, where the descriptions are only added to the model if they're requested.  Considering that viewing the service in webtools is the only way to expose the descriptions, you'd probably never end up with more than a few dozen actually loaded into memory.

> 
> -Adrian
> 
> 
> --- On Sat, 7/31/10, Adrian Crum <ad...@yahoo.com> wrote:
>> Scott,
>> 
>> Are you sure this does what you intended? The parameters
>> are being assigned the service's description, not the
>> parameter's description.
>> 
>> -Adrian
>> 
>> --- On Sat, 7/31/10, lektran@apache.org
>> <le...@apache.org>
>> wrote:
>> 
>>> From: lektran@apache.org
>> <le...@apache.org>
>>> Subject: svn commit: r981018 - in
>> /ofbiz/trunk/framework/service: dtd/services.xsd
>> src/org/ofbiz/service/ModelParam.java
>> src/org/ofbiz/service/ModelServiceReader.java
>>> To: commits@ofbiz.apache.org
>>> Date: Saturday, July 31, 2010, 1:26 AM
>>> Author: lektran
>>> Date: Sat Jul 31 08:26:42 2010
>>> New Revision: 981018
>>> 
>>> URL: http://svn.apache.org/viewvc?rev=981018&view=rev
>>> Log:
>>> Add a description element for service attributes,
>> should
>>> make documenting some services a little easier. 
>> Still
>>> need to display it in webtools somewhere.
>>> 
>>> Modified:
>>>    
>>> ofbiz/trunk/framework/service/dtd/services.xsd
>>>    
>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>    
>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>> 
>>> Modified:
>> ofbiz/trunk/framework/service/dtd/services.xsd
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/dtd/services.xsd?rev=981018&r1=981017&r2=981018&view=diff
>>> 
>> ==============================================================================
>>> --- ofbiz/trunk/framework/service/dtd/services.xsd
>>> (original)
>>> +++ ofbiz/trunk/framework/service/dtd/services.xsd Sat
>> Jul
>>> 31 08:26:42 2010
>>> @@ -314,6 +314,7 @@ under the License.
>>>      
>>>    <xs:complexType>
>>>          
>>>    <xs:sequence>
>>>              
>>>    <xs:element minOccurs="0"
>>> maxOccurs="unbounded" ref="type-validate"/>
>>> +               
>>> <xs:element minOccurs="0" ref="description" />
>>>          
>>>    </xs:sequence>
>>>          
>>>    <xs:attributeGroup
>>> ref="attlist.attribute"/>
>>>      
>>>    </xs:complexType>
>>> 
>>> Modified:
>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java?rev=981018&r1=981017&r2=981018&view=diff
>>> 
>> ==============================================================================
>>> ---
>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>> (original)
>>> +++
>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>> Sat Jul 31 08:26:42 2010
>>> @@ -43,6 +43,9 @@ public class ModelParam implements
>> Seria
>>>      /** Parameter name */
>>>      public String name;
>>>   
>>> +    /** The description of this parameter */
>>> +    public String description;
>>> +
>>>      /** Paramater type */
>>>      public String type;
>>>   
>>> @@ -88,6 +91,7 @@ public class ModelParam implements
>> Seria
>>>   
>>>      public ModelParam(ModelParam
>>> param) {
>>>          this.name =
>>> param.name;
>>> +        this.description =
>>> param.description;
>>>          this.type =
>>> param.type;
>>>          this.mode =
>>> param.mode;
>>>          this.formLabel =
>>> param.formLabel;
>>> 
>>> Modified:
>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java?rev=981018&r1=981017&r2=981018&view=diff
>>> 
>> ==============================================================================
>>> ---
>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>> (original)
>>> +++
>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>> Sat Jul 31 08:26:42 2010
>>> @@ -518,6 +518,7 @@ public class ModelServiceReader
>>> implemen
>>>          
>>>    ModelParam param = new ModelParam();
>>>   
>>>          
>>>    param.name =
>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("name")).intern();
>>> +           
>>> param.description = getCDATADef(baseElement,
>>> "description");
>>>          
>>>    param.type =
>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("type")).intern();
>>>          
>>>    param.mode =
>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("mode")).intern();
>>>          
>>>    param.entityName =
>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("entity-name")).intern();
>>> 
>>> 
>>> 
>> 
>> 
>> 
>> 
> 
> 
> 


Re: svn commit: r981018 - in /ofbiz/trunk/framework/service: dtd/services.xsd src/org/ofbiz/service/ModelParam.java src/org/ofbiz/service/ModelServiceReader.java

Posted by Adrian Crum <ad...@yahoo.com>.
Scott,

I fixed the code and built it out a little.

This is a good idea, but I have a couple of problems with it:

1. User-oriented meta-data should be kept in properties files - so it can be internationalized. The drawback to that is, developers will be less likely to include descriptions if they have to do it in a separate file.

2. Description text should not be stored in the model - it increases the model's memory footprint greatly. Instead, the descriptions should be retrieved from properties files when needed (in AvailableServices.groovy, for example).

-Adrian


--- On Sat, 7/31/10, Adrian Crum <ad...@yahoo.com> wrote:
> Scott,
> 
> Are you sure this does what you intended? The parameters
> are being assigned the service's description, not the
> parameter's description.
> 
> -Adrian
> 
> --- On Sat, 7/31/10, lektran@apache.org
> <le...@apache.org>
> wrote:
> 
> > From: lektran@apache.org
> <le...@apache.org>
> > Subject: svn commit: r981018 - in
> /ofbiz/trunk/framework/service: dtd/services.xsd
> src/org/ofbiz/service/ModelParam.java
> src/org/ofbiz/service/ModelServiceReader.java
> > To: commits@ofbiz.apache.org
> > Date: Saturday, July 31, 2010, 1:26 AM
> > Author: lektran
> > Date: Sat Jul 31 08:26:42 2010
> > New Revision: 981018
> > 
> > URL: http://svn.apache.org/viewvc?rev=981018&view=rev
> > Log:
> > Add a description element for service attributes,
> should
> > make documenting some services a little easier. 
> Still
> > need to display it in webtools somewhere.
> > 
> > Modified:
> >    
> > ofbiz/trunk/framework/service/dtd/services.xsd
> >    
> >
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> >    
> >
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> > 
> > Modified:
> ofbiz/trunk/framework/service/dtd/services.xsd
> > URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/dtd/services.xsd?rev=981018&r1=981017&r2=981018&view=diff
> >
> ==============================================================================
> > --- ofbiz/trunk/framework/service/dtd/services.xsd
> > (original)
> > +++ ofbiz/trunk/framework/service/dtd/services.xsd Sat
> Jul
> > 31 08:26:42 2010
> > @@ -314,6 +314,7 @@ under the License.
> >      
> >    <xs:complexType>
> >          
> >    <xs:sequence>
> >              
> >    <xs:element minOccurs="0"
> > maxOccurs="unbounded" ref="type-validate"/>
> > +               
> > <xs:element minOccurs="0" ref="description" />
> >          
> >    </xs:sequence>
> >          
> >    <xs:attributeGroup
> > ref="attlist.attribute"/>
> >      
> >    </xs:complexType>
> > 
> > Modified:
> >
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> > URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java?rev=981018&r1=981017&r2=981018&view=diff
> >
> ==============================================================================
> > ---
> >
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> > (original)
> > +++
> >
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
> > Sat Jul 31 08:26:42 2010
> > @@ -43,6 +43,9 @@ public class ModelParam implements
> Seria
> >      /** Parameter name */
> >      public String name;
> >  
> > +    /** The description of this parameter */
> > +    public String description;
> > +
> >      /** Paramater type */
> >      public String type;
> >  
> > @@ -88,6 +91,7 @@ public class ModelParam implements
> Seria
> >  
> >      public ModelParam(ModelParam
> > param) {
> >          this.name =
> > param.name;
> > +        this.description =
> > param.description;
> >          this.type =
> > param.type;
> >          this.mode =
> > param.mode;
> >          this.formLabel =
> > param.formLabel;
> > 
> > Modified:
> >
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> > URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java?rev=981018&r1=981017&r2=981018&view=diff
> >
> ==============================================================================
> > ---
> >
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> > (original)
> > +++
> >
> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
> > Sat Jul 31 08:26:42 2010
> > @@ -518,6 +518,7 @@ public class ModelServiceReader
> > implemen
> >          
> >    ModelParam param = new ModelParam();
> >  
> >          
> >    param.name =
> >
> UtilXml.checkEmpty(attribute.getAttribute("name")).intern();
> > +           
> > param.description = getCDATADef(baseElement,
> > "description");
> >          
> >    param.type =
> >
> UtilXml.checkEmpty(attribute.getAttribute("type")).intern();
> >          
> >    param.mode =
> >
> UtilXml.checkEmpty(attribute.getAttribute("mode")).intern();
> >          
> >    param.entityName =
> >
> UtilXml.checkEmpty(attribute.getAttribute("entity-name")).intern();
> > 
> > 
> > 
> 
> 
> 
>