You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by "Pierre De Rop (JIRA)" <ji...@apache.org> on 2008/07/21 17:21:31 UTC
[jira] Created: (FELIX-639) Need more logs from SCR
Need more logs from SCR
-----------------------
Key: FELIX-639
URL: https://issues.apache.org/jira/browse/FELIX-639
Project: Felix
Issue Type: Improvement
Components: Declarative Services (SCR)
Affects Versions: scr-1.0.2
Environment: linux
Reporter: Pierre De Rop
Priority: Minor
As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
for example, the following SCR.xml has two errors:
<components>
<component name="Component1">
<implementation class="test.ds.hello.HelloComponent1"/>
<reference name="LOG" interface="org.osgi.service.log.LogService"
bind="setLog"
unbind="unsetLog"
cardinality="1..n"
policy="dynamic"/>
</component>
<component name=""Component2">
<implementation class="test.ds.hello.HelloComponent2"/>
<reference name="LOG" interface="org.osgi.service.log.LogService"
bind="setLog"
unbind="unsetLog"
cardinality="1..n"
policy="dynamic"/>
<reference name="LOG" interface="org.osgi.service.log.LogService"
bind="setLog2"
unbind="unsetLog2"
cardinality="1..n"
policy="dynamic"/>
</component>
</components>
-> the two components are embedded in an invalid <components> xml root element
-> the component "Component2" has two references with the SAME name
I would like the SCR to log some WARNINGs message, when finding these sort of errors:
1/ log a WARNING message when finding an unknown/invalid xml element
2/ log a WARNING message when duplicate reference names are detected for one given component.
Here is a tentative fix which is working for my use cases:
1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
// Check that the references are ok.
// We'll especially Check if there's not duplicate names in the component references.
HashSet refs = new HashSet();
Iterator referenceIterator = m_references.iterator();
while ( referenceIterator.hasNext() )
{
ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
refMeta.validate( this );
if (! refs.add(refMeta.getName())) {
throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
}
}
2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
+++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
@@ -211,14 +211,21 @@
ref.setUnbind( attrib.getProperty( "unbind" ) );
m_currentComponent.addDependency( ref );
- }
+ }
+ else {
+ throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
+ ": \"" + localName + "\"", null);
+ }
}
catch ( Exception ex )
{
ex.printStackTrace();
throw new ParseException( "Exception during parsing", ex );
}
- }
+ } else {
+ throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
+ ": \"" + localName + "\"", null);
+ }
}
-> Here is the complete/modifed method:
public void startElement( String uri, String localName, Properties attrib ) throws ParseException
{
// according to the spec, the elements should have the namespace,
// except when the root element is the "component" element
// So we check this for the first element, we receive.
if ( firstElement )
{
firstElement = false;
if ( localName.equals( "component" ) && "".equals( uri ) )
{
overrideNamespace = NAMESPACE_URI;
}
}
if ( overrideNamespace != null && "".equals( uri ) )
{
uri = overrideNamespace;
}
if ( NAMESPACE_URI.equals( uri ) )
{
try
{
// 112.4.3 Component Element
if ( localName.equals( "component" ) )
{
// Create a new ComponentMetadata
m_currentComponent = new ComponentMetadata();
// name attribute is mandatory
m_currentComponent.setName( attrib.getProperty( "name" ) );
// enabled attribute is optional
if ( attrib.getProperty( "enabled" ) != null )
{
m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
}
// immediate attribute is optional
if ( attrib.getProperty( "immediate" ) != null )
{
m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
}
// factory attribute is optional
if ( attrib.getProperty( "factory" ) != null )
{
m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
}
// Add this component to the list
m_components.add( m_currentComponent );
}
// 112.4.4 Implementation
else if ( localName.equals( "implementation" ) )
{
// Set the implementation class name (mandatory)
m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
}
// 112.4.5 [...] Property Elements
else if ( localName.equals( "property" ) )
{
PropertyMetadata prop = new PropertyMetadata();
// name attribute is mandatory
prop.setName( attrib.getProperty( "name" ) );
// type attribute is optional
if ( attrib.getProperty( "type" ) != null )
{
prop.setType( attrib.getProperty( "type" ) );
}
// 112.4.5: If the value attribute is specified, the body of the element is ignored.
if ( attrib.getProperty( "value" ) != null )
{
prop.setValue( attrib.getProperty( "value" ) );
m_currentComponent.addProperty( prop );
}
else
{
// hold the metadata pending
m_pendingProperty = prop;
}
}
// 112.4.5 Properties [...] Elements
else if ( localName.equals( "properties" ) )
{
readPropertiesEntry( attrib.getProperty( "entry" ) );
}
// 112.4.6 Service Element
else if ( localName.equals( "service" ) )
{
m_currentService = new ServiceMetadata();
// servicefactory attribute is optional
if ( attrib.getProperty( "servicefactory" ) != null )
{
m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
}
m_currentComponent.setService( m_currentService );
}
else if ( localName.equals( "provide" ) )
{
m_currentService.addProvide( attrib.getProperty( "interface" ) );
}
// 112.4.7 Reference element
else if ( localName.equals( "reference" ) )
{
ReferenceMetadata ref = new ReferenceMetadata();
ref.setName( attrib.getProperty( "name" ) );
ref.setInterface( attrib.getProperty( "interface" ) );
// Cardinality
if ( attrib.getProperty( "cardinality" ) != null )
{
ref.setCardinality( attrib.getProperty( "cardinality" ) );
}
if ( attrib.getProperty( "policy" ) != null )
{
ref.setPolicy( attrib.getProperty( "policy" ) );
}
//if
ref.setTarget( attrib.getProperty( "target" ) );
ref.setBind( attrib.getProperty( "bind" ) );
ref.setUnbind( attrib.getProperty( "unbind" ) );
m_currentComponent.addDependency( ref );
}
else {
throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
": \"" + localName + "\"", null);
}
}
catch ( Exception ex )
{
ex.printStackTrace();
throw new ParseException( "Exception during parsing", ex );
}
} else {
throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
": \"" + localName + "\"", null);
}
}
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (FELIX-639) Need more logs from SCR
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616036#action_12616036 ]
Felix Meschberger commented on FELIX-639:
-----------------------------------------
Thanks for submitting this issue. Please allow me to comment on this quickly:
(1) "Unexpected Elements": Such elements are explicitly allowed as per the Declarative Services specification. So your example is perfectly valid, except that you have to use namespaces here (as Carsten Ziegeler already stated). The namespace may only be omitted in case of having a sling <component> element in the XML file. So The ParseException in the XMLHandler will certainly not be done as proposed as this would violate the spec.
What I could imagine is, that we should add a check for this situation: if there is no namespace, only a single <component> element is allowed, else an ERROR is logged. Otherwise multiple <component> elements are allowed.
(2) Duplicate Reference names: I could not find a note in the Declarative Services speicifcation which states that these names must be unique inside a component. The spec only states that they are local to the component and may be used for ComponentContext.locateService. So throwing an exception during validation in case of duplicate names is IMHO not appropriate because validation failure means the component is not activated. Rather I would log a warning (at most, if not just an INFO) message and go on.
> Need more logs from SCR
> -----------------------
>
> Key: FELIX-639
> URL: https://issues.apache.org/jira/browse/FELIX-639
> Project: Felix
> Issue Type: Improvement
> Components: Declarative Services (SCR)
> Affects Versions: scr-1.0.2
> Environment: linux
> Reporter: Pierre De Rop
> Priority: Minor
> Attachments: ComponentMetadata.java, XmlHandler.java
>
>
> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
> for example, the following SCR.xml has two errors:
> <components>
> <component name="Component1">
> <implementation class="test.ds.hello.HelloComponent1"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> <component name=""Component2">
> <implementation class="test.ds.hello.HelloComponent2"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog2"
> unbind="unsetLog2"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> </components>
> -> the two components are embedded in an invalid <components> xml root element
> -> the component "Component2" has two references with the SAME name
> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
> 1/ log a WARNING message when finding an unknown/invalid xml element
> 2/ log a WARNING message when duplicate reference names are detected for one given component.
> Here is a tentative fix which is working for my use cases:
> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
> // Check that the references are ok.
> // We'll especially Check if there's not duplicate names in the component references.
> HashSet refs = new HashSet();
> Iterator referenceIterator = m_references.iterator();
> while ( referenceIterator.hasNext() )
> {
> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
> refMeta.validate( this );
> if (! refs.add(refMeta.getName())) {
> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
> }
> }
> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
> @@ -211,14 +211,21 @@
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> - }
> + }
> + else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> - }
> + } else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> -> Here is the complete/modifed method:
> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
> {
> // according to the spec, the elements should have the namespace,
> // except when the root element is the "component" element
> // So we check this for the first element, we receive.
> if ( firstElement )
> {
> firstElement = false;
> if ( localName.equals( "component" ) && "".equals( uri ) )
> {
> overrideNamespace = NAMESPACE_URI;
> }
> }
> if ( overrideNamespace != null && "".equals( uri ) )
> {
> uri = overrideNamespace;
> }
> if ( NAMESPACE_URI.equals( uri ) )
> {
> try
> {
> // 112.4.3 Component Element
> if ( localName.equals( "component" ) )
> {
> // Create a new ComponentMetadata
> m_currentComponent = new ComponentMetadata();
> // name attribute is mandatory
> m_currentComponent.setName( attrib.getProperty( "name" ) );
> // enabled attribute is optional
> if ( attrib.getProperty( "enabled" ) != null )
> {
> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
> }
> // immediate attribute is optional
> if ( attrib.getProperty( "immediate" ) != null )
> {
> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
> }
> // factory attribute is optional
> if ( attrib.getProperty( "factory" ) != null )
> {
> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
> }
> // Add this component to the list
> m_components.add( m_currentComponent );
> }
> // 112.4.4 Implementation
> else if ( localName.equals( "implementation" ) )
> {
> // Set the implementation class name (mandatory)
> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
> }
> // 112.4.5 [...] Property Elements
> else if ( localName.equals( "property" ) )
> {
> PropertyMetadata prop = new PropertyMetadata();
> // name attribute is mandatory
> prop.setName( attrib.getProperty( "name" ) );
> // type attribute is optional
> if ( attrib.getProperty( "type" ) != null )
> {
> prop.setType( attrib.getProperty( "type" ) );
> }
> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
> if ( attrib.getProperty( "value" ) != null )
> {
> prop.setValue( attrib.getProperty( "value" ) );
> m_currentComponent.addProperty( prop );
> }
> else
> {
> // hold the metadata pending
> m_pendingProperty = prop;
> }
> }
> // 112.4.5 Properties [...] Elements
> else if ( localName.equals( "properties" ) )
> {
> readPropertiesEntry( attrib.getProperty( "entry" ) );
> }
> // 112.4.6 Service Element
> else if ( localName.equals( "service" ) )
> {
> m_currentService = new ServiceMetadata();
> // servicefactory attribute is optional
> if ( attrib.getProperty( "servicefactory" ) != null )
> {
> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
> }
> m_currentComponent.setService( m_currentService );
> }
> else if ( localName.equals( "provide" ) )
> {
> m_currentService.addProvide( attrib.getProperty( "interface" ) );
> }
> // 112.4.7 Reference element
> else if ( localName.equals( "reference" ) )
> {
> ReferenceMetadata ref = new ReferenceMetadata();
> ref.setName( attrib.getProperty( "name" ) );
> ref.setInterface( attrib.getProperty( "interface" ) );
> // Cardinality
> if ( attrib.getProperty( "cardinality" ) != null )
> {
> ref.setCardinality( attrib.getProperty( "cardinality" ) );
> }
> if ( attrib.getProperty( "policy" ) != null )
> {
> ref.setPolicy( attrib.getProperty( "policy" ) );
> }
> //if
> ref.setTarget( attrib.getProperty( "target" ) );
> ref.setBind( attrib.getProperty( "bind" ) );
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> }
> else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> } else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (FELIX-639) Need more logs from SCR
Posted by "Pierre De Rop (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12671198#action_12671198 ]
Pierre De Rop commented on FELIX-639:
-------------------------------------
Ok, thanks for the fix in ComponentMetadata.
For the XmlHandler fix, I'm ok, but I have a last suggestion: may be it would help if you just add a DEBUG log (instead of raising a ParserException) ?
> Need more logs from SCR
> -----------------------
>
> Key: FELIX-639
> URL: https://issues.apache.org/jira/browse/FELIX-639
> Project: Felix
> Issue Type: Improvement
> Components: Declarative Services (SCR)
> Affects Versions: scr-1.0.2
> Environment: linux
> Reporter: Pierre De Rop
> Priority: Minor
> Attachments: ComponentMetadata.java, XmlHandler.java
>
>
> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
> for example, the following SCR.xml has two errors:
> <components>
> <component name="Component1">
> <implementation class="test.ds.hello.HelloComponent1"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> <component name=""Component2">
> <implementation class="test.ds.hello.HelloComponent2"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog2"
> unbind="unsetLog2"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> </components>
> -> the two components are embedded in an invalid <components> xml root element
> -> the component "Component2" has two references with the SAME name
> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
> 1/ log a WARNING message when finding an unknown/invalid xml element
> 2/ log a WARNING message when duplicate reference names are detected for one given component.
> Here is a tentative fix which is working for my use cases:
> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
> // Check that the references are ok.
> // We'll especially Check if there's not duplicate names in the component references.
> HashSet refs = new HashSet();
> Iterator referenceIterator = m_references.iterator();
> while ( referenceIterator.hasNext() )
> {
> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
> refMeta.validate( this );
> if (! refs.add(refMeta.getName())) {
> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
> }
> }
> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
> @@ -211,14 +211,21 @@
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> - }
> + }
> + else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> - }
> + } else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> -> Here is the complete/modifed method:
> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
> {
> // according to the spec, the elements should have the namespace,
> // except when the root element is the "component" element
> // So we check this for the first element, we receive.
> if ( firstElement )
> {
> firstElement = false;
> if ( localName.equals( "component" ) && "".equals( uri ) )
> {
> overrideNamespace = NAMESPACE_URI;
> }
> }
> if ( overrideNamespace != null && "".equals( uri ) )
> {
> uri = overrideNamespace;
> }
> if ( NAMESPACE_URI.equals( uri ) )
> {
> try
> {
> // 112.4.3 Component Element
> if ( localName.equals( "component" ) )
> {
> // Create a new ComponentMetadata
> m_currentComponent = new ComponentMetadata();
> // name attribute is mandatory
> m_currentComponent.setName( attrib.getProperty( "name" ) );
> // enabled attribute is optional
> if ( attrib.getProperty( "enabled" ) != null )
> {
> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
> }
> // immediate attribute is optional
> if ( attrib.getProperty( "immediate" ) != null )
> {
> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
> }
> // factory attribute is optional
> if ( attrib.getProperty( "factory" ) != null )
> {
> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
> }
> // Add this component to the list
> m_components.add( m_currentComponent );
> }
> // 112.4.4 Implementation
> else if ( localName.equals( "implementation" ) )
> {
> // Set the implementation class name (mandatory)
> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
> }
> // 112.4.5 [...] Property Elements
> else if ( localName.equals( "property" ) )
> {
> PropertyMetadata prop = new PropertyMetadata();
> // name attribute is mandatory
> prop.setName( attrib.getProperty( "name" ) );
> // type attribute is optional
> if ( attrib.getProperty( "type" ) != null )
> {
> prop.setType( attrib.getProperty( "type" ) );
> }
> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
> if ( attrib.getProperty( "value" ) != null )
> {
> prop.setValue( attrib.getProperty( "value" ) );
> m_currentComponent.addProperty( prop );
> }
> else
> {
> // hold the metadata pending
> m_pendingProperty = prop;
> }
> }
> // 112.4.5 Properties [...] Elements
> else if ( localName.equals( "properties" ) )
> {
> readPropertiesEntry( attrib.getProperty( "entry" ) );
> }
> // 112.4.6 Service Element
> else if ( localName.equals( "service" ) )
> {
> m_currentService = new ServiceMetadata();
> // servicefactory attribute is optional
> if ( attrib.getProperty( "servicefactory" ) != null )
> {
> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
> }
> m_currentComponent.setService( m_currentService );
> }
> else if ( localName.equals( "provide" ) )
> {
> m_currentService.addProvide( attrib.getProperty( "interface" ) );
> }
> // 112.4.7 Reference element
> else if ( localName.equals( "reference" ) )
> {
> ReferenceMetadata ref = new ReferenceMetadata();
> ref.setName( attrib.getProperty( "name" ) );
> ref.setInterface( attrib.getProperty( "interface" ) );
> // Cardinality
> if ( attrib.getProperty( "cardinality" ) != null )
> {
> ref.setCardinality( attrib.getProperty( "cardinality" ) );
> }
> if ( attrib.getProperty( "policy" ) != null )
> {
> ref.setPolicy( attrib.getProperty( "policy" ) );
> }
> //if
> ref.setTarget( attrib.getProperty( "target" ) );
> ref.setBind( attrib.getProperty( "bind" ) );
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> }
> else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> } else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (FELIX-639) Need more logs from SCR
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12671181#action_12671181 ]
Felix Meschberger commented on FELIX-639:
-----------------------------------------
Applied patch to ComponentMetadata in Rev. 741601 (and modifications to just log the situation instead of throwing in Rev. 741611)
> Need more logs from SCR
> -----------------------
>
> Key: FELIX-639
> URL: https://issues.apache.org/jira/browse/FELIX-639
> Project: Felix
> Issue Type: Improvement
> Components: Declarative Services (SCR)
> Affects Versions: scr-1.0.2
> Environment: linux
> Reporter: Pierre De Rop
> Priority: Minor
> Attachments: ComponentMetadata.java, XmlHandler.java
>
>
> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
> for example, the following SCR.xml has two errors:
> <components>
> <component name="Component1">
> <implementation class="test.ds.hello.HelloComponent1"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> <component name=""Component2">
> <implementation class="test.ds.hello.HelloComponent2"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog2"
> unbind="unsetLog2"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> </components>
> -> the two components are embedded in an invalid <components> xml root element
> -> the component "Component2" has two references with the SAME name
> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
> 1/ log a WARNING message when finding an unknown/invalid xml element
> 2/ log a WARNING message when duplicate reference names are detected for one given component.
> Here is a tentative fix which is working for my use cases:
> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
> // Check that the references are ok.
> // We'll especially Check if there's not duplicate names in the component references.
> HashSet refs = new HashSet();
> Iterator referenceIterator = m_references.iterator();
> while ( referenceIterator.hasNext() )
> {
> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
> refMeta.validate( this );
> if (! refs.add(refMeta.getName())) {
> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
> }
> }
> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
> @@ -211,14 +211,21 @@
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> - }
> + }
> + else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> - }
> + } else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> -> Here is the complete/modifed method:
> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
> {
> // according to the spec, the elements should have the namespace,
> // except when the root element is the "component" element
> // So we check this for the first element, we receive.
> if ( firstElement )
> {
> firstElement = false;
> if ( localName.equals( "component" ) && "".equals( uri ) )
> {
> overrideNamespace = NAMESPACE_URI;
> }
> }
> if ( overrideNamespace != null && "".equals( uri ) )
> {
> uri = overrideNamespace;
> }
> if ( NAMESPACE_URI.equals( uri ) )
> {
> try
> {
> // 112.4.3 Component Element
> if ( localName.equals( "component" ) )
> {
> // Create a new ComponentMetadata
> m_currentComponent = new ComponentMetadata();
> // name attribute is mandatory
> m_currentComponent.setName( attrib.getProperty( "name" ) );
> // enabled attribute is optional
> if ( attrib.getProperty( "enabled" ) != null )
> {
> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
> }
> // immediate attribute is optional
> if ( attrib.getProperty( "immediate" ) != null )
> {
> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
> }
> // factory attribute is optional
> if ( attrib.getProperty( "factory" ) != null )
> {
> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
> }
> // Add this component to the list
> m_components.add( m_currentComponent );
> }
> // 112.4.4 Implementation
> else if ( localName.equals( "implementation" ) )
> {
> // Set the implementation class name (mandatory)
> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
> }
> // 112.4.5 [...] Property Elements
> else if ( localName.equals( "property" ) )
> {
> PropertyMetadata prop = new PropertyMetadata();
> // name attribute is mandatory
> prop.setName( attrib.getProperty( "name" ) );
> // type attribute is optional
> if ( attrib.getProperty( "type" ) != null )
> {
> prop.setType( attrib.getProperty( "type" ) );
> }
> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
> if ( attrib.getProperty( "value" ) != null )
> {
> prop.setValue( attrib.getProperty( "value" ) );
> m_currentComponent.addProperty( prop );
> }
> else
> {
> // hold the metadata pending
> m_pendingProperty = prop;
> }
> }
> // 112.4.5 Properties [...] Elements
> else if ( localName.equals( "properties" ) )
> {
> readPropertiesEntry( attrib.getProperty( "entry" ) );
> }
> // 112.4.6 Service Element
> else if ( localName.equals( "service" ) )
> {
> m_currentService = new ServiceMetadata();
> // servicefactory attribute is optional
> if ( attrib.getProperty( "servicefactory" ) != null )
> {
> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
> }
> m_currentComponent.setService( m_currentService );
> }
> else if ( localName.equals( "provide" ) )
> {
> m_currentService.addProvide( attrib.getProperty( "interface" ) );
> }
> // 112.4.7 Reference element
> else if ( localName.equals( "reference" ) )
> {
> ReferenceMetadata ref = new ReferenceMetadata();
> ref.setName( attrib.getProperty( "name" ) );
> ref.setInterface( attrib.getProperty( "interface" ) );
> // Cardinality
> if ( attrib.getProperty( "cardinality" ) != null )
> {
> ref.setCardinality( attrib.getProperty( "cardinality" ) );
> }
> if ( attrib.getProperty( "policy" ) != null )
> {
> ref.setPolicy( attrib.getProperty( "policy" ) );
> }
> //if
> ref.setTarget( attrib.getProperty( "target" ) );
> ref.setBind( attrib.getProperty( "bind" ) );
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> }
> else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> } else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Issue Comment Edited: (FELIX-639) Need more logs from
SCR
Posted by Felix Meschberger <fm...@gmail.com>.
Hi,
Pierre De Rop schrieb:
> Hi Felix;
>
> Sorry for my wrong suggestion about raising exceptions on invalid
> scr.xml descriptor.
Don't worry ;-)
> But, as you suggested, some logs (warn/info, or whatever level) would be
> just fine and helpful.
Ok. So will add some checks and logs.
Regards
Felix
>
> /Pierre
>
> Felix Meschberger (JIRA) wrote:
>> [
>> https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616036#action_12616036
>> ]
>> fmeschbe edited comment on FELIX-639 at 8/6/08 2:31 AM:
>> -----------------------------------------------------------------
>>
>> Thanks for submitting this issue. Please allow me to comment on this
>> quickly:
>>
>> (1) "Unexpected Elements": Such elements are explicitly allowed as per
>> the Declarative Services specification. So your example is perfectly
>> valid, except that you have to use namespaces here (as Carsten
>> Ziegeler already stated). The namespace may only be omitted in case of
>> having a single <component> element in the XML file. So the
>> ParseException in the XMLHandler is not appropriate as this would
>> violate the spec.
>>
>> What I could imagine is, that we should add a check for this
>> situation: if there is no namespace, only a single <component> element
>> is allowed, else an ERROR is logged. Otherwise multiple <component>
>> elements are allowed.
>>
>> (2) Duplicate Reference names: I could not find a note in the
>> Declarative Services speicifcation which states that these names must
>> be unique inside a component. The spec only states that they are local
>> to the component and may be used for ComponentContext.locateService.
>> So throwing an exception during validation in case of duplicate names
>> is IMHO not appropriate because validation failure means the component
>> is not going to be activated. Rather I would log a warning (at most,
>> if not just an INFO) message and go on.
>>
>> was (Author: fmeschbe):
>> Thanks for submitting this issue. Please allow me to comment on
>> this quickly:
>>
>> (1) "Unexpected Elements": Such elements are explicitly allowed as per
>> the Declarative Services specification. So your example is perfectly
>> valid, except that you have to use namespaces here (as Carsten
>> Ziegeler already stated). The namespace may only be omitted in case of
>> having a sling <component> element in the XML file. So The
>> ParseException in the XMLHandler will certainly not be done as
>> proposed as this would violate the spec.
>>
>> What I could imagine is, that we should add a check for this
>> situation: if there is no namespace, only a single <component> element
>> is allowed, else an ERROR is logged. Otherwise multiple <component>
>> elements are allowed.
>>
>> (2) Duplicate Reference names: I could not find a note in the
>> Declarative Services speicifcation which states that these names must
>> be unique inside a component. The spec only states that they are local
>> to the component and may be used for ComponentContext.locateService.
>> So throwing an exception during validation in case of duplicate names
>> is IMHO not appropriate because validation failure means the component
>> is not activated. Rather I would log a warning (at most, if not just
>> an INFO) message and go on.
>>
>>> Need more logs from SCR
>>> -----------------------
>>>
>>> Key: FELIX-639
>>> URL: https://issues.apache.org/jira/browse/FELIX-639
>>> Project: Felix
>>> Issue Type: Improvement
>>> Components: Declarative Services (SCR)
>>> Affects Versions: scr-1.0.2
>>> Environment: linux
>>> Reporter: Pierre De Rop
>>> Priority: Minor
>>> Attachments: ComponentMetadata.java, XmlHandler.java
>>>
>>>
>>> As explained in the dev post
>>> http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
>>> I would like the SCR to be improved in order to log some WARNINGs,
>>> when a SCR.xml descriptor contains invalid elements, event if it is
>>> well formed.
>>> for example, the following SCR.xml has two errors:
>>> <components>
>>> <component name="Component1">
>>> <implementation class="test.ds.hello.HelloComponent1"/>
>>> <reference name="LOG" interface="org.osgi.service.log.LogService"
>>> bind="setLog" unbind="unsetLog"
>>> cardinality="1..n"
>>> policy="dynamic"/>
>>> </component>
>>> <component name=""Component2">
>>> <implementation class="test.ds.hello.HelloComponent2"/>
>>> <reference name="LOG" interface="org.osgi.service.log.LogService"
>>> bind="setLog" unbind="unsetLog"
>>> cardinality="1..n"
>>> policy="dynamic"/>
>>> <reference name="LOG"
>>> interface="org.osgi.service.log.LogService" bind="setLog2"
>>> unbind="unsetLog2"
>>> cardinality="1..n"
>>> policy="dynamic"/>
>>> </component>
>>> </components>
>>> -> the two components are embedded in an invalid <components> xml
>>> root element
>>> -> the component "Component2" has two references with the SAME name
>>> I would like the SCR to log some WARNINGs message, when finding these
>>> sort of errors:
>>> 1/ log a WARNING message when finding an unknown/invalid xml element
>>> 2/ log a WARNING message when duplicate reference names are detected
>>> for one given component.
>>> Here is a tentative fix which is working for my use cases:
>>> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate()
>>> method (around line 299):
>>> // Check that the references are ok.
>>> // We'll especially Check if there's not duplicate names in the
>>> component references.
>>> HashSet refs = new HashSet();
>>> Iterator referenceIterator = m_references.iterator();
>>> while ( referenceIterator.hasNext() )
>>> {
>>> ReferenceMetadata refMeta = ( ReferenceMetadata )
>>> referenceIterator.next();
>>> refMeta.validate( this );
>>> if (! refs.add(refMeta.getName())) {
>>> throw validationFailure( "Detected duplicate reference
>>> name: \"" + refMeta.getName() + "\"");
>>> }
>>> }
>>> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method
>>> (around line 215, at the end of the method):
>>> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java
>>> (working copy)
>>> @@ -211,14 +211,21 @@
>>> ref.setUnbind( attrib.getProperty( "unbind" ) );
>>> m_currentComponent.addDependency( ref );
>>> - }
>>> + }
>>> + else {
>>> + throw new ParseException("Invalid SCR xml element
>>> found in " + m_bundle.getLocation() +
>>> + ": \"" + localName +
>>> "\"", null);
>>> + }
>>> }
>>> catch ( Exception ex )
>>> {
>>> ex.printStackTrace();
>>> throw new ParseException( "Exception during
>>> parsing", ex );
>>> }
>>> - }
>>> + } else {
>>> + throw new ParseException("Invalid SCR xml element found in
>>> " + m_bundle.getLocation() +
>>> + ": \"" + localName + "\"", null);
>>> + }
>>> }
>>> -> Here is the complete/modifed method:
>>> public void startElement( String uri, String localName,
>>> Properties attrib ) throws ParseException
>>> {
>>> // according to the spec, the elements should have the
>>> namespace,
>>> // except when the root element is the "component" element
>>> // So we check this for the first element, we receive.
>>> if ( firstElement )
>>> {
>>> firstElement = false;
>>> if ( localName.equals( "component" ) && "".equals( uri ) )
>>> {
>>> overrideNamespace = NAMESPACE_URI;
>>> }
>>> }
>>> if ( overrideNamespace != null && "".equals( uri ) )
>>> {
>>> uri = overrideNamespace;
>>> }
>>> if ( NAMESPACE_URI.equals( uri ) )
>>> {
>>> try
>>> {
>>> // 112.4.3 Component Element
>>> if ( localName.equals( "component" ) )
>>> {
>>> // Create a new ComponentMetadata
>>> m_currentComponent = new ComponentMetadata();
>>> // name attribute is mandatory
>>> m_currentComponent.setName( attrib.getProperty(
>>> "name" ) );
>>> // enabled attribute is optional
>>> if ( attrib.getProperty( "enabled" ) != null )
>>> {
>>> m_currentComponent.setEnabled(
>>> attrib.getProperty( "enabled" ).equals( "true" ) );
>>> }
>>> // immediate attribute is optional
>>> if ( attrib.getProperty( "immediate" ) != null )
>>> {
>>> m_currentComponent.setImmediate(
>>> attrib.getProperty( "immediate" ).equals( "true" ) );
>>> }
>>> // factory attribute is optional
>>> if ( attrib.getProperty( "factory" ) != null )
>>> {
>>> m_currentComponent.setFactoryIdentifier(
>>> attrib.getProperty( "factory" ) );
>>> }
>>> // Add this component to the list
>>> m_components.add( m_currentComponent );
>>> }
>>> // 112.4.4 Implementation
>>> else if ( localName.equals( "implementation" ) )
>>> {
>>> // Set the implementation class name (mandatory)
>>> m_currentComponent.setImplementationClassName(
>>> attrib.getProperty( "class" ) );
>>> }
>>> // 112.4.5 [...] Property Elements
>>> else if ( localName.equals( "property" ) )
>>> {
>>> PropertyMetadata prop = new PropertyMetadata();
>>> // name attribute is mandatory
>>> prop.setName( attrib.getProperty( "name" ) );
>>> // type attribute is optional
>>> if ( attrib.getProperty( "type" ) != null )
>>> {
>>> prop.setType( attrib.getProperty( "type" ) );
>>> }
>>> // 112.4.5: If the value attribute is specified,
>>> the body of the element is ignored.
>>> if ( attrib.getProperty( "value" ) != null )
>>> {
>>> prop.setValue( attrib.getProperty( "value" ) );
>>> m_currentComponent.addProperty( prop );
>>> }
>>> else
>>> {
>>> // hold the metadata pending
>>> m_pendingProperty = prop;
>>> }
>>> }
>>> // 112.4.5 Properties [...] Elements
>>> else if ( localName.equals( "properties" ) )
>>> {
>>> readPropertiesEntry( attrib.getProperty( "entry"
>>> ) );
>>> }
>>> // 112.4.6 Service Element
>>> else if ( localName.equals( "service" ) )
>>> {
>>> m_currentService = new ServiceMetadata();
>>> // servicefactory attribute is optional
>>> if ( attrib.getProperty( "servicefactory" ) !=
>>> null )
>>> {
>>> m_currentService.setServiceFactory(
>>> attrib.getProperty( "servicefactory" ).equals( "true" ) );
>>> }
>>> m_currentComponent.setService( m_currentService );
>>> }
>>> else if ( localName.equals( "provide" ) )
>>> {
>>> m_currentService.addProvide( attrib.getProperty(
>>> "interface" ) );
>>> }
>>> // 112.4.7 Reference element
>>> else if ( localName.equals( "reference" ) )
>>> {
>>> ReferenceMetadata ref = new ReferenceMetadata();
>>> ref.setName( attrib.getProperty( "name" ) );
>>> ref.setInterface( attrib.getProperty( "interface"
>>> ) );
>>> // Cardinality
>>> if ( attrib.getProperty( "cardinality" ) != null )
>>> {
>>> ref.setCardinality( attrib.getProperty(
>>> "cardinality" ) );
>>> }
>>> if ( attrib.getProperty( "policy" ) != null )
>>> {
>>> ref.setPolicy( attrib.getProperty( "policy" ) );
>>> }
>>> //if
>>> ref.setTarget( attrib.getProperty( "target" ) );
>>> ref.setBind( attrib.getProperty( "bind" ) );
>>> ref.setUnbind( attrib.getProperty( "unbind" ) );
>>> m_currentComponent.addDependency( ref );
>>> } else {
>>> throw new ParseException("Invalid SCR xml element found
>>> in " + m_bundle.getLocation() + ": \"" +
>>> localName + "\"", null);
>>> }
>>> }
>>> catch ( Exception ex )
>>> {
>>> ex.printStackTrace();
>>> throw new ParseException( "Exception during parsing",
>>> ex );
>>> }
>>> } else {
>>> throw new ParseException("Invalid SCR xml element found in " +
>>> m_bundle.getLocation() + ": \"" + localName +
>>> "\"", null);
>>> }
>>> }
>>>
>>
>>
>
>
Re: [jira] Issue Comment Edited: (FELIX-639) Need more logs from
SCR
Posted by Pierre De Rop <pi...@alcatel-lucent.fr>.
Hi Felix;
Sorry for my wrong suggestion about raising exceptions on invalid
scr.xml descriptor.
But, as you suggested, some logs (warn/info, or whatever level) would be
just fine and helpful.
/Pierre
Felix Meschberger (JIRA) wrote:
> [ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616036#action_12616036 ]
>
> fmeschbe edited comment on FELIX-639 at 8/6/08 2:31 AM:
> -----------------------------------------------------------------
>
> Thanks for submitting this issue. Please allow me to comment on this quickly:
>
> (1) "Unexpected Elements": Such elements are explicitly allowed as per the Declarative Services specification. So your example is perfectly valid, except that you have to use namespaces here (as Carsten Ziegeler already stated). The namespace may only be omitted in case of having a single <component> element in the XML file. So the ParseException in the XMLHandler is not appropriate as this would violate the spec.
>
> What I could imagine is, that we should add a check for this situation: if there is no namespace, only a single <component> element is allowed, else an ERROR is logged. Otherwise multiple <component> elements are allowed.
>
> (2) Duplicate Reference names: I could not find a note in the Declarative Services speicifcation which states that these names must be unique inside a component. The spec only states that they are local to the component and may be used for ComponentContext.locateService. So throwing an exception during validation in case of duplicate names is IMHO not appropriate because validation failure means the component is not going to be activated. Rather I would log a warning (at most, if not just an INFO) message and go on.
>
> was (Author: fmeschbe):
> Thanks for submitting this issue. Please allow me to comment on this quickly:
>
> (1) "Unexpected Elements": Such elements are explicitly allowed as per the Declarative Services specification. So your example is perfectly valid, except that you have to use namespaces here (as Carsten Ziegeler already stated). The namespace may only be omitted in case of having a sling <component> element in the XML file. So The ParseException in the XMLHandler will certainly not be done as proposed as this would violate the spec.
>
> What I could imagine is, that we should add a check for this situation: if there is no namespace, only a single <component> element is allowed, else an ERROR is logged. Otherwise multiple <component> elements are allowed.
>
> (2) Duplicate Reference names: I could not find a note in the Declarative Services speicifcation which states that these names must be unique inside a component. The spec only states that they are local to the component and may be used for ComponentContext.locateService. So throwing an exception during validation in case of duplicate names is IMHO not appropriate because validation failure means the component is not activated. Rather I would log a warning (at most, if not just an INFO) message and go on.
>
>
>> Need more logs from SCR
>> -----------------------
>>
>> Key: FELIX-639
>> URL: https://issues.apache.org/jira/browse/FELIX-639
>> Project: Felix
>> Issue Type: Improvement
>> Components: Declarative Services (SCR)
>> Affects Versions: scr-1.0.2
>> Environment: linux
>> Reporter: Pierre De Rop
>> Priority: Minor
>> Attachments: ComponentMetadata.java, XmlHandler.java
>>
>>
>> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
>> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
>> for example, the following SCR.xml has two errors:
>> <components>
>> <component name="Component1">
>> <implementation class="test.ds.hello.HelloComponent1"/>
>> <reference name="LOG" interface="org.osgi.service.log.LogService"
>> bind="setLog"
>> unbind="unsetLog"
>> cardinality="1..n"
>> policy="dynamic"/>
>> </component>
>> <component name=""Component2">
>> <implementation class="test.ds.hello.HelloComponent2"/>
>> <reference name="LOG" interface="org.osgi.service.log.LogService"
>> bind="setLog"
>> unbind="unsetLog"
>> cardinality="1..n"
>> policy="dynamic"/>
>>
>> <reference name="LOG" interface="org.osgi.service.log.LogService"
>> bind="setLog2"
>> unbind="unsetLog2"
>> cardinality="1..n"
>> policy="dynamic"/>
>> </component>
>> </components>
>> -> the two components are embedded in an invalid <components> xml root element
>> -> the component "Component2" has two references with the SAME name
>> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
>> 1/ log a WARNING message when finding an unknown/invalid xml element
>> 2/ log a WARNING message when duplicate reference names are detected for one given component.
>> Here is a tentative fix which is working for my use cases:
>> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
>> // Check that the references are ok.
>> // We'll especially Check if there's not duplicate names in the component references.
>> HashSet refs = new HashSet();
>> Iterator referenceIterator = m_references.iterator();
>> while ( referenceIterator.hasNext() )
>> {
>> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
>> refMeta.validate( this );
>> if (! refs.add(refMeta.getName())) {
>> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
>> }
>> }
>> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
>> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
>> @@ -211,14 +211,21 @@
>> ref.setUnbind( attrib.getProperty( "unbind" ) );
>> m_currentComponent.addDependency( ref );
>> - }
>> + }
>> + else {
>> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
>> + ": \"" + localName + "\"", null);
>> + }
>> }
>> catch ( Exception ex )
>> {
>> ex.printStackTrace();
>> throw new ParseException( "Exception during parsing", ex );
>> }
>> - }
>> + } else {
>> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
>> + ": \"" + localName + "\"", null);
>> + }
>> }
>> -> Here is the complete/modifed method:
>> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
>> {
>> // according to the spec, the elements should have the namespace,
>> // except when the root element is the "component" element
>> // So we check this for the first element, we receive.
>> if ( firstElement )
>> {
>> firstElement = false;
>> if ( localName.equals( "component" ) && "".equals( uri ) )
>> {
>> overrideNamespace = NAMESPACE_URI;
>> }
>> }
>> if ( overrideNamespace != null && "".equals( uri ) )
>> {
>> uri = overrideNamespace;
>> }
>> if ( NAMESPACE_URI.equals( uri ) )
>> {
>> try
>> {
>> // 112.4.3 Component Element
>> if ( localName.equals( "component" ) )
>> {
>> // Create a new ComponentMetadata
>> m_currentComponent = new ComponentMetadata();
>> // name attribute is mandatory
>> m_currentComponent.setName( attrib.getProperty( "name" ) );
>> // enabled attribute is optional
>> if ( attrib.getProperty( "enabled" ) != null )
>> {
>> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
>> }
>> // immediate attribute is optional
>> if ( attrib.getProperty( "immediate" ) != null )
>> {
>> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
>> }
>> // factory attribute is optional
>> if ( attrib.getProperty( "factory" ) != null )
>> {
>> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
>> }
>> // Add this component to the list
>> m_components.add( m_currentComponent );
>> }
>> // 112.4.4 Implementation
>> else if ( localName.equals( "implementation" ) )
>> {
>> // Set the implementation class name (mandatory)
>> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
>> }
>> // 112.4.5 [...] Property Elements
>> else if ( localName.equals( "property" ) )
>> {
>> PropertyMetadata prop = new PropertyMetadata();
>> // name attribute is mandatory
>> prop.setName( attrib.getProperty( "name" ) );
>> // type attribute is optional
>> if ( attrib.getProperty( "type" ) != null )
>> {
>> prop.setType( attrib.getProperty( "type" ) );
>> }
>> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
>> if ( attrib.getProperty( "value" ) != null )
>> {
>> prop.setValue( attrib.getProperty( "value" ) );
>> m_currentComponent.addProperty( prop );
>> }
>> else
>> {
>> // hold the metadata pending
>> m_pendingProperty = prop;
>> }
>> }
>> // 112.4.5 Properties [...] Elements
>> else if ( localName.equals( "properties" ) )
>> {
>> readPropertiesEntry( attrib.getProperty( "entry" ) );
>> }
>> // 112.4.6 Service Element
>> else if ( localName.equals( "service" ) )
>> {
>> m_currentService = new ServiceMetadata();
>> // servicefactory attribute is optional
>> if ( attrib.getProperty( "servicefactory" ) != null )
>> {
>> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
>> }
>> m_currentComponent.setService( m_currentService );
>> }
>> else if ( localName.equals( "provide" ) )
>> {
>> m_currentService.addProvide( attrib.getProperty( "interface" ) );
>> }
>> // 112.4.7 Reference element
>> else if ( localName.equals( "reference" ) )
>> {
>> ReferenceMetadata ref = new ReferenceMetadata();
>> ref.setName( attrib.getProperty( "name" ) );
>> ref.setInterface( attrib.getProperty( "interface" ) );
>> // Cardinality
>> if ( attrib.getProperty( "cardinality" ) != null )
>> {
>> ref.setCardinality( attrib.getProperty( "cardinality" ) );
>> }
>> if ( attrib.getProperty( "policy" ) != null )
>> {
>> ref.setPolicy( attrib.getProperty( "policy" ) );
>> }
>> //if
>> ref.setTarget( attrib.getProperty( "target" ) );
>> ref.setBind( attrib.getProperty( "bind" ) );
>> ref.setUnbind( attrib.getProperty( "unbind" ) );
>> m_currentComponent.addDependency( ref );
>> }
>> else {
>> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
>> ": \"" + localName + "\"", null);
>> }
>> }
>> catch ( Exception ex )
>> {
>> ex.printStackTrace();
>> throw new ParseException( "Exception during parsing", ex );
>> }
>> } else {
>> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
>> ": \"" + localName + "\"", null);
>> }
>> }
>>
>
>
[jira] Issue Comment Edited: (FELIX-639) Need more logs from SCR
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616036#action_12616036 ]
fmeschbe edited comment on FELIX-639 at 8/6/08 2:31 AM:
-----------------------------------------------------------------
Thanks for submitting this issue. Please allow me to comment on this quickly:
(1) "Unexpected Elements": Such elements are explicitly allowed as per the Declarative Services specification. So your example is perfectly valid, except that you have to use namespaces here (as Carsten Ziegeler already stated). The namespace may only be omitted in case of having a single <component> element in the XML file. So the ParseException in the XMLHandler is not appropriate as this would violate the spec.
What I could imagine is, that we should add a check for this situation: if there is no namespace, only a single <component> element is allowed, else an ERROR is logged. Otherwise multiple <component> elements are allowed.
(2) Duplicate Reference names: I could not find a note in the Declarative Services speicifcation which states that these names must be unique inside a component. The spec only states that they are local to the component and may be used for ComponentContext.locateService. So throwing an exception during validation in case of duplicate names is IMHO not appropriate because validation failure means the component is not going to be activated. Rather I would log a warning (at most, if not just an INFO) message and go on.
was (Author: fmeschbe):
Thanks for submitting this issue. Please allow me to comment on this quickly:
(1) "Unexpected Elements": Such elements are explicitly allowed as per the Declarative Services specification. So your example is perfectly valid, except that you have to use namespaces here (as Carsten Ziegeler already stated). The namespace may only be omitted in case of having a sling <component> element in the XML file. So The ParseException in the XMLHandler will certainly not be done as proposed as this would violate the spec.
What I could imagine is, that we should add a check for this situation: if there is no namespace, only a single <component> element is allowed, else an ERROR is logged. Otherwise multiple <component> elements are allowed.
(2) Duplicate Reference names: I could not find a note in the Declarative Services speicifcation which states that these names must be unique inside a component. The spec only states that they are local to the component and may be used for ComponentContext.locateService. So throwing an exception during validation in case of duplicate names is IMHO not appropriate because validation failure means the component is not activated. Rather I would log a warning (at most, if not just an INFO) message and go on.
> Need more logs from SCR
> -----------------------
>
> Key: FELIX-639
> URL: https://issues.apache.org/jira/browse/FELIX-639
> Project: Felix
> Issue Type: Improvement
> Components: Declarative Services (SCR)
> Affects Versions: scr-1.0.2
> Environment: linux
> Reporter: Pierre De Rop
> Priority: Minor
> Attachments: ComponentMetadata.java, XmlHandler.java
>
>
> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
> for example, the following SCR.xml has two errors:
> <components>
> <component name="Component1">
> <implementation class="test.ds.hello.HelloComponent1"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> <component name=""Component2">
> <implementation class="test.ds.hello.HelloComponent2"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog2"
> unbind="unsetLog2"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> </components>
> -> the two components are embedded in an invalid <components> xml root element
> -> the component "Component2" has two references with the SAME name
> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
> 1/ log a WARNING message when finding an unknown/invalid xml element
> 2/ log a WARNING message when duplicate reference names are detected for one given component.
> Here is a tentative fix which is working for my use cases:
> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
> // Check that the references are ok.
> // We'll especially Check if there's not duplicate names in the component references.
> HashSet refs = new HashSet();
> Iterator referenceIterator = m_references.iterator();
> while ( referenceIterator.hasNext() )
> {
> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
> refMeta.validate( this );
> if (! refs.add(refMeta.getName())) {
> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
> }
> }
> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
> @@ -211,14 +211,21 @@
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> - }
> + }
> + else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> - }
> + } else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> -> Here is the complete/modifed method:
> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
> {
> // according to the spec, the elements should have the namespace,
> // except when the root element is the "component" element
> // So we check this for the first element, we receive.
> if ( firstElement )
> {
> firstElement = false;
> if ( localName.equals( "component" ) && "".equals( uri ) )
> {
> overrideNamespace = NAMESPACE_URI;
> }
> }
> if ( overrideNamespace != null && "".equals( uri ) )
> {
> uri = overrideNamespace;
> }
> if ( NAMESPACE_URI.equals( uri ) )
> {
> try
> {
> // 112.4.3 Component Element
> if ( localName.equals( "component" ) )
> {
> // Create a new ComponentMetadata
> m_currentComponent = new ComponentMetadata();
> // name attribute is mandatory
> m_currentComponent.setName( attrib.getProperty( "name" ) );
> // enabled attribute is optional
> if ( attrib.getProperty( "enabled" ) != null )
> {
> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
> }
> // immediate attribute is optional
> if ( attrib.getProperty( "immediate" ) != null )
> {
> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
> }
> // factory attribute is optional
> if ( attrib.getProperty( "factory" ) != null )
> {
> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
> }
> // Add this component to the list
> m_components.add( m_currentComponent );
> }
> // 112.4.4 Implementation
> else if ( localName.equals( "implementation" ) )
> {
> // Set the implementation class name (mandatory)
> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
> }
> // 112.4.5 [...] Property Elements
> else if ( localName.equals( "property" ) )
> {
> PropertyMetadata prop = new PropertyMetadata();
> // name attribute is mandatory
> prop.setName( attrib.getProperty( "name" ) );
> // type attribute is optional
> if ( attrib.getProperty( "type" ) != null )
> {
> prop.setType( attrib.getProperty( "type" ) );
> }
> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
> if ( attrib.getProperty( "value" ) != null )
> {
> prop.setValue( attrib.getProperty( "value" ) );
> m_currentComponent.addProperty( prop );
> }
> else
> {
> // hold the metadata pending
> m_pendingProperty = prop;
> }
> }
> // 112.4.5 Properties [...] Elements
> else if ( localName.equals( "properties" ) )
> {
> readPropertiesEntry( attrib.getProperty( "entry" ) );
> }
> // 112.4.6 Service Element
> else if ( localName.equals( "service" ) )
> {
> m_currentService = new ServiceMetadata();
> // servicefactory attribute is optional
> if ( attrib.getProperty( "servicefactory" ) != null )
> {
> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
> }
> m_currentComponent.setService( m_currentService );
> }
> else if ( localName.equals( "provide" ) )
> {
> m_currentService.addProvide( attrib.getProperty( "interface" ) );
> }
> // 112.4.7 Reference element
> else if ( localName.equals( "reference" ) )
> {
> ReferenceMetadata ref = new ReferenceMetadata();
> ref.setName( attrib.getProperty( "name" ) );
> ref.setInterface( attrib.getProperty( "interface" ) );
> // Cardinality
> if ( attrib.getProperty( "cardinality" ) != null )
> {
> ref.setCardinality( attrib.getProperty( "cardinality" ) );
> }
> if ( attrib.getProperty( "policy" ) != null )
> {
> ref.setPolicy( attrib.getProperty( "policy" ) );
> }
> //if
> ref.setTarget( attrib.getProperty( "target" ) );
> ref.setBind( attrib.getProperty( "bind" ) );
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> }
> else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> } else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (FELIX-639) Need more logs from SCR
Posted by "Pierre De Rop (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12671373#action_12671373 ]
Pierre De Rop commented on FELIX-639:
-------------------------------------
A big thanks, Felix.
Now there is a strong logging in the scr and it will help us when debugging our scr xml descriptors.
> Need more logs from SCR
> -----------------------
>
> Key: FELIX-639
> URL: https://issues.apache.org/jira/browse/FELIX-639
> Project: Felix
> Issue Type: Improvement
> Components: Declarative Services (SCR)
> Affects Versions: scr-1.0.2
> Environment: linux
> Reporter: Pierre De Rop
> Assignee: Felix Meschberger
> Priority: Minor
> Fix For: scr-1.0.8
>
> Attachments: ComponentMetadata.java, XmlHandler.java
>
>
> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
> for example, the following SCR.xml has two errors:
> <components>
> <component name="Component1">
> <implementation class="test.ds.hello.HelloComponent1"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> <component name=""Component2">
> <implementation class="test.ds.hello.HelloComponent2"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog2"
> unbind="unsetLog2"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> </components>
> -> the two components are embedded in an invalid <components> xml root element
> -> the component "Component2" has two references with the SAME name
> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
> 1/ log a WARNING message when finding an unknown/invalid xml element
> 2/ log a WARNING message when duplicate reference names are detected for one given component.
> Here is a tentative fix which is working for my use cases:
> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
> // Check that the references are ok.
> // We'll especially Check if there's not duplicate names in the component references.
> HashSet refs = new HashSet();
> Iterator referenceIterator = m_references.iterator();
> while ( referenceIterator.hasNext() )
> {
> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
> refMeta.validate( this );
> if (! refs.add(refMeta.getName())) {
> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
> }
> }
> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
> @@ -211,14 +211,21 @@
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> - }
> + }
> + else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> - }
> + } else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> -> Here is the complete/modifed method:
> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
> {
> // according to the spec, the elements should have the namespace,
> // except when the root element is the "component" element
> // So we check this for the first element, we receive.
> if ( firstElement )
> {
> firstElement = false;
> if ( localName.equals( "component" ) && "".equals( uri ) )
> {
> overrideNamespace = NAMESPACE_URI;
> }
> }
> if ( overrideNamespace != null && "".equals( uri ) )
> {
> uri = overrideNamespace;
> }
> if ( NAMESPACE_URI.equals( uri ) )
> {
> try
> {
> // 112.4.3 Component Element
> if ( localName.equals( "component" ) )
> {
> // Create a new ComponentMetadata
> m_currentComponent = new ComponentMetadata();
> // name attribute is mandatory
> m_currentComponent.setName( attrib.getProperty( "name" ) );
> // enabled attribute is optional
> if ( attrib.getProperty( "enabled" ) != null )
> {
> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
> }
> // immediate attribute is optional
> if ( attrib.getProperty( "immediate" ) != null )
> {
> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
> }
> // factory attribute is optional
> if ( attrib.getProperty( "factory" ) != null )
> {
> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
> }
> // Add this component to the list
> m_components.add( m_currentComponent );
> }
> // 112.4.4 Implementation
> else if ( localName.equals( "implementation" ) )
> {
> // Set the implementation class name (mandatory)
> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
> }
> // 112.4.5 [...] Property Elements
> else if ( localName.equals( "property" ) )
> {
> PropertyMetadata prop = new PropertyMetadata();
> // name attribute is mandatory
> prop.setName( attrib.getProperty( "name" ) );
> // type attribute is optional
> if ( attrib.getProperty( "type" ) != null )
> {
> prop.setType( attrib.getProperty( "type" ) );
> }
> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
> if ( attrib.getProperty( "value" ) != null )
> {
> prop.setValue( attrib.getProperty( "value" ) );
> m_currentComponent.addProperty( prop );
> }
> else
> {
> // hold the metadata pending
> m_pendingProperty = prop;
> }
> }
> // 112.4.5 Properties [...] Elements
> else if ( localName.equals( "properties" ) )
> {
> readPropertiesEntry( attrib.getProperty( "entry" ) );
> }
> // 112.4.6 Service Element
> else if ( localName.equals( "service" ) )
> {
> m_currentService = new ServiceMetadata();
> // servicefactory attribute is optional
> if ( attrib.getProperty( "servicefactory" ) != null )
> {
> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
> }
> m_currentComponent.setService( m_currentService );
> }
> else if ( localName.equals( "provide" ) )
> {
> m_currentService.addProvide( attrib.getProperty( "interface" ) );
> }
> // 112.4.7 Reference element
> else if ( localName.equals( "reference" ) )
> {
> ReferenceMetadata ref = new ReferenceMetadata();
> ref.setName( attrib.getProperty( "name" ) );
> ref.setInterface( attrib.getProperty( "interface" ) );
> // Cardinality
> if ( attrib.getProperty( "cardinality" ) != null )
> {
> ref.setCardinality( attrib.getProperty( "cardinality" ) );
> }
> if ( attrib.getProperty( "policy" ) != null )
> {
> ref.setPolicy( attrib.getProperty( "policy" ) );
> }
> //if
> ref.setTarget( attrib.getProperty( "target" ) );
> ref.setBind( attrib.getProperty( "bind" ) );
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> }
> else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> } else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (FELIX-639) Need more logs from SCR
Posted by "Pierre De Rop (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pierre De Rop updated FELIX-639:
--------------------------------
Attachment: XmlHandler.java
ComponentMetadata.java
In ComponentMetadata.java, I have modified the validate() method in order to detect duplicate reference names for the same component.
In XmlHandler.java, I raise some ParserExceptions at the end of the startElement method.
> Need more logs from SCR
> -----------------------
>
> Key: FELIX-639
> URL: https://issues.apache.org/jira/browse/FELIX-639
> Project: Felix
> Issue Type: Improvement
> Components: Declarative Services (SCR)
> Affects Versions: scr-1.0.2
> Environment: linux
> Reporter: Pierre De Rop
> Priority: Minor
> Attachments: ComponentMetadata.java, XmlHandler.java
>
>
> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
> for example, the following SCR.xml has two errors:
> <components>
> <component name="Component1">
> <implementation class="test.ds.hello.HelloComponent1"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> <component name=""Component2">
> <implementation class="test.ds.hello.HelloComponent2"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog2"
> unbind="unsetLog2"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> </components>
> -> the two components are embedded in an invalid <components> xml root element
> -> the component "Component2" has two references with the SAME name
> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
> 1/ log a WARNING message when finding an unknown/invalid xml element
> 2/ log a WARNING message when duplicate reference names are detected for one given component.
> Here is a tentative fix which is working for my use cases:
> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
> // Check that the references are ok.
> // We'll especially Check if there's not duplicate names in the component references.
> HashSet refs = new HashSet();
> Iterator referenceIterator = m_references.iterator();
> while ( referenceIterator.hasNext() )
> {
> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
> refMeta.validate( this );
> if (! refs.add(refMeta.getName())) {
> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
> }
> }
> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
> @@ -211,14 +211,21 @@
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> - }
> + }
> + else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> - }
> + } else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> -> Here is the complete/modifed method:
> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
> {
> // according to the spec, the elements should have the namespace,
> // except when the root element is the "component" element
> // So we check this for the first element, we receive.
> if ( firstElement )
> {
> firstElement = false;
> if ( localName.equals( "component" ) && "".equals( uri ) )
> {
> overrideNamespace = NAMESPACE_URI;
> }
> }
> if ( overrideNamespace != null && "".equals( uri ) )
> {
> uri = overrideNamespace;
> }
> if ( NAMESPACE_URI.equals( uri ) )
> {
> try
> {
> // 112.4.3 Component Element
> if ( localName.equals( "component" ) )
> {
> // Create a new ComponentMetadata
> m_currentComponent = new ComponentMetadata();
> // name attribute is mandatory
> m_currentComponent.setName( attrib.getProperty( "name" ) );
> // enabled attribute is optional
> if ( attrib.getProperty( "enabled" ) != null )
> {
> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
> }
> // immediate attribute is optional
> if ( attrib.getProperty( "immediate" ) != null )
> {
> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
> }
> // factory attribute is optional
> if ( attrib.getProperty( "factory" ) != null )
> {
> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
> }
> // Add this component to the list
> m_components.add( m_currentComponent );
> }
> // 112.4.4 Implementation
> else if ( localName.equals( "implementation" ) )
> {
> // Set the implementation class name (mandatory)
> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
> }
> // 112.4.5 [...] Property Elements
> else if ( localName.equals( "property" ) )
> {
> PropertyMetadata prop = new PropertyMetadata();
> // name attribute is mandatory
> prop.setName( attrib.getProperty( "name" ) );
> // type attribute is optional
> if ( attrib.getProperty( "type" ) != null )
> {
> prop.setType( attrib.getProperty( "type" ) );
> }
> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
> if ( attrib.getProperty( "value" ) != null )
> {
> prop.setValue( attrib.getProperty( "value" ) );
> m_currentComponent.addProperty( prop );
> }
> else
> {
> // hold the metadata pending
> m_pendingProperty = prop;
> }
> }
> // 112.4.5 Properties [...] Elements
> else if ( localName.equals( "properties" ) )
> {
> readPropertiesEntry( attrib.getProperty( "entry" ) );
> }
> // 112.4.6 Service Element
> else if ( localName.equals( "service" ) )
> {
> m_currentService = new ServiceMetadata();
> // servicefactory attribute is optional
> if ( attrib.getProperty( "servicefactory" ) != null )
> {
> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
> }
> m_currentComponent.setService( m_currentService );
> }
> else if ( localName.equals( "provide" ) )
> {
> m_currentService.addProvide( attrib.getProperty( "interface" ) );
> }
> // 112.4.7 Reference element
> else if ( localName.equals( "reference" ) )
> {
> ReferenceMetadata ref = new ReferenceMetadata();
> ref.setName( attrib.getProperty( "name" ) );
> ref.setInterface( attrib.getProperty( "interface" ) );
> // Cardinality
> if ( attrib.getProperty( "cardinality" ) != null )
> {
> ref.setCardinality( attrib.getProperty( "cardinality" ) );
> }
> if ( attrib.getProperty( "policy" ) != null )
> {
> ref.setPolicy( attrib.getProperty( "policy" ) );
> }
> //if
> ref.setTarget( attrib.getProperty( "target" ) );
> ref.setBind( attrib.getProperty( "bind" ) );
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> }
> else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> } else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (FELIX-639) Need more logs from SCR
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Felix Meschberger updated FELIX-639:
------------------------------------
Fix Version/s: scr-1.0.8
Assignee: Felix Meschberger
> Need more logs from SCR
> -----------------------
>
> Key: FELIX-639
> URL: https://issues.apache.org/jira/browse/FELIX-639
> Project: Felix
> Issue Type: Improvement
> Components: Declarative Services (SCR)
> Affects Versions: scr-1.0.2
> Environment: linux
> Reporter: Pierre De Rop
> Assignee: Felix Meschberger
> Priority: Minor
> Fix For: scr-1.0.8
>
> Attachments: ComponentMetadata.java, XmlHandler.java
>
>
> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
> for example, the following SCR.xml has two errors:
> <components>
> <component name="Component1">
> <implementation class="test.ds.hello.HelloComponent1"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> <component name=""Component2">
> <implementation class="test.ds.hello.HelloComponent2"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog2"
> unbind="unsetLog2"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> </components>
> -> the two components are embedded in an invalid <components> xml root element
> -> the component "Component2" has two references with the SAME name
> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
> 1/ log a WARNING message when finding an unknown/invalid xml element
> 2/ log a WARNING message when duplicate reference names are detected for one given component.
> Here is a tentative fix which is working for my use cases:
> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
> // Check that the references are ok.
> // We'll especially Check if there's not duplicate names in the component references.
> HashSet refs = new HashSet();
> Iterator referenceIterator = m_references.iterator();
> while ( referenceIterator.hasNext() )
> {
> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
> refMeta.validate( this );
> if (! refs.add(refMeta.getName())) {
> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
> }
> }
> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
> @@ -211,14 +211,21 @@
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> - }
> + }
> + else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> - }
> + } else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> -> Here is the complete/modifed method:
> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
> {
> // according to the spec, the elements should have the namespace,
> // except when the root element is the "component" element
> // So we check this for the first element, we receive.
> if ( firstElement )
> {
> firstElement = false;
> if ( localName.equals( "component" ) && "".equals( uri ) )
> {
> overrideNamespace = NAMESPACE_URI;
> }
> }
> if ( overrideNamespace != null && "".equals( uri ) )
> {
> uri = overrideNamespace;
> }
> if ( NAMESPACE_URI.equals( uri ) )
> {
> try
> {
> // 112.4.3 Component Element
> if ( localName.equals( "component" ) )
> {
> // Create a new ComponentMetadata
> m_currentComponent = new ComponentMetadata();
> // name attribute is mandatory
> m_currentComponent.setName( attrib.getProperty( "name" ) );
> // enabled attribute is optional
> if ( attrib.getProperty( "enabled" ) != null )
> {
> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
> }
> // immediate attribute is optional
> if ( attrib.getProperty( "immediate" ) != null )
> {
> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
> }
> // factory attribute is optional
> if ( attrib.getProperty( "factory" ) != null )
> {
> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
> }
> // Add this component to the list
> m_components.add( m_currentComponent );
> }
> // 112.4.4 Implementation
> else if ( localName.equals( "implementation" ) )
> {
> // Set the implementation class name (mandatory)
> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
> }
> // 112.4.5 [...] Property Elements
> else if ( localName.equals( "property" ) )
> {
> PropertyMetadata prop = new PropertyMetadata();
> // name attribute is mandatory
> prop.setName( attrib.getProperty( "name" ) );
> // type attribute is optional
> if ( attrib.getProperty( "type" ) != null )
> {
> prop.setType( attrib.getProperty( "type" ) );
> }
> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
> if ( attrib.getProperty( "value" ) != null )
> {
> prop.setValue( attrib.getProperty( "value" ) );
> m_currentComponent.addProperty( prop );
> }
> else
> {
> // hold the metadata pending
> m_pendingProperty = prop;
> }
> }
> // 112.4.5 Properties [...] Elements
> else if ( localName.equals( "properties" ) )
> {
> readPropertiesEntry( attrib.getProperty( "entry" ) );
> }
> // 112.4.6 Service Element
> else if ( localName.equals( "service" ) )
> {
> m_currentService = new ServiceMetadata();
> // servicefactory attribute is optional
> if ( attrib.getProperty( "servicefactory" ) != null )
> {
> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
> }
> m_currentComponent.setService( m_currentService );
> }
> else if ( localName.equals( "provide" ) )
> {
> m_currentService.addProvide( attrib.getProperty( "interface" ) );
> }
> // 112.4.7 Reference element
> else if ( localName.equals( "reference" ) )
> {
> ReferenceMetadata ref = new ReferenceMetadata();
> ref.setName( attrib.getProperty( "name" ) );
> ref.setInterface( attrib.getProperty( "interface" ) );
> // Cardinality
> if ( attrib.getProperty( "cardinality" ) != null )
> {
> ref.setCardinality( attrib.getProperty( "cardinality" ) );
> }
> if ( attrib.getProperty( "policy" ) != null )
> {
> ref.setPolicy( attrib.getProperty( "policy" ) );
> }
> //if
> ref.setTarget( attrib.getProperty( "target" ) );
> ref.setBind( attrib.getProperty( "bind" ) );
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> }
> else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> } else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (FELIX-639) Need more logs from SCR
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Felix Meschberger closed FELIX-639.
-----------------------------------
Resolution: Fixed
Thanks Pierre for the feedback.
So I close this issue now.
> Need more logs from SCR
> -----------------------
>
> Key: FELIX-639
> URL: https://issues.apache.org/jira/browse/FELIX-639
> Project: Felix
> Issue Type: Improvement
> Components: Declarative Services (SCR)
> Affects Versions: scr-1.0.2
> Environment: linux
> Reporter: Pierre De Rop
> Assignee: Felix Meschberger
> Priority: Minor
> Fix For: scr-1.0.8
>
> Attachments: ComponentMetadata.java, XmlHandler.java
>
>
> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
> for example, the following SCR.xml has two errors:
> <components>
> <component name="Component1">
> <implementation class="test.ds.hello.HelloComponent1"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> <component name=""Component2">
> <implementation class="test.ds.hello.HelloComponent2"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog2"
> unbind="unsetLog2"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> </components>
> -> the two components are embedded in an invalid <components> xml root element
> -> the component "Component2" has two references with the SAME name
> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
> 1/ log a WARNING message when finding an unknown/invalid xml element
> 2/ log a WARNING message when duplicate reference names are detected for one given component.
> Here is a tentative fix which is working for my use cases:
> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
> // Check that the references are ok.
> // We'll especially Check if there's not duplicate names in the component references.
> HashSet refs = new HashSet();
> Iterator referenceIterator = m_references.iterator();
> while ( referenceIterator.hasNext() )
> {
> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
> refMeta.validate( this );
> if (! refs.add(refMeta.getName())) {
> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
> }
> }
> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
> @@ -211,14 +211,21 @@
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> - }
> + }
> + else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> - }
> + } else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> -> Here is the complete/modifed method:
> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
> {
> // according to the spec, the elements should have the namespace,
> // except when the root element is the "component" element
> // So we check this for the first element, we receive.
> if ( firstElement )
> {
> firstElement = false;
> if ( localName.equals( "component" ) && "".equals( uri ) )
> {
> overrideNamespace = NAMESPACE_URI;
> }
> }
> if ( overrideNamespace != null && "".equals( uri ) )
> {
> uri = overrideNamespace;
> }
> if ( NAMESPACE_URI.equals( uri ) )
> {
> try
> {
> // 112.4.3 Component Element
> if ( localName.equals( "component" ) )
> {
> // Create a new ComponentMetadata
> m_currentComponent = new ComponentMetadata();
> // name attribute is mandatory
> m_currentComponent.setName( attrib.getProperty( "name" ) );
> // enabled attribute is optional
> if ( attrib.getProperty( "enabled" ) != null )
> {
> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
> }
> // immediate attribute is optional
> if ( attrib.getProperty( "immediate" ) != null )
> {
> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
> }
> // factory attribute is optional
> if ( attrib.getProperty( "factory" ) != null )
> {
> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
> }
> // Add this component to the list
> m_components.add( m_currentComponent );
> }
> // 112.4.4 Implementation
> else if ( localName.equals( "implementation" ) )
> {
> // Set the implementation class name (mandatory)
> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
> }
> // 112.4.5 [...] Property Elements
> else if ( localName.equals( "property" ) )
> {
> PropertyMetadata prop = new PropertyMetadata();
> // name attribute is mandatory
> prop.setName( attrib.getProperty( "name" ) );
> // type attribute is optional
> if ( attrib.getProperty( "type" ) != null )
> {
> prop.setType( attrib.getProperty( "type" ) );
> }
> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
> if ( attrib.getProperty( "value" ) != null )
> {
> prop.setValue( attrib.getProperty( "value" ) );
> m_currentComponent.addProperty( prop );
> }
> else
> {
> // hold the metadata pending
> m_pendingProperty = prop;
> }
> }
> // 112.4.5 Properties [...] Elements
> else if ( localName.equals( "properties" ) )
> {
> readPropertiesEntry( attrib.getProperty( "entry" ) );
> }
> // 112.4.6 Service Element
> else if ( localName.equals( "service" ) )
> {
> m_currentService = new ServiceMetadata();
> // servicefactory attribute is optional
> if ( attrib.getProperty( "servicefactory" ) != null )
> {
> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
> }
> m_currentComponent.setService( m_currentService );
> }
> else if ( localName.equals( "provide" ) )
> {
> m_currentService.addProvide( attrib.getProperty( "interface" ) );
> }
> // 112.4.7 Reference element
> else if ( localName.equals( "reference" ) )
> {
> ReferenceMetadata ref = new ReferenceMetadata();
> ref.setName( attrib.getProperty( "name" ) );
> ref.setInterface( attrib.getProperty( "interface" ) );
> // Cardinality
> if ( attrib.getProperty( "cardinality" ) != null )
> {
> ref.setCardinality( attrib.getProperty( "cardinality" ) );
> }
> if ( attrib.getProperty( "policy" ) != null )
> {
> ref.setPolicy( attrib.getProperty( "policy" ) );
> }
> //if
> ref.setTarget( attrib.getProperty( "target" ) );
> ref.setBind( attrib.getProperty( "bind" ) );
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> }
> else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> } else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (FELIX-639) Need more logs from SCR
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12671307#action_12671307 ]
Felix Meschberger commented on FELIX-639:
-----------------------------------------
Alright, I added a very simple Logger interface, which helps abstracting the logging for the XmlHandler and the ComponentMetadata without compromising the unit tests. This is used to add logging for the XmlHandler class for unexpected elements.
Applied in Rev. 741708 and 741709.
Deployed SNAPSHOT version 1.0.7-20090206.204036-2
Does this suite your needs ?
> Need more logs from SCR
> -----------------------
>
> Key: FELIX-639
> URL: https://issues.apache.org/jira/browse/FELIX-639
> Project: Felix
> Issue Type: Improvement
> Components: Declarative Services (SCR)
> Affects Versions: scr-1.0.2
> Environment: linux
> Reporter: Pierre De Rop
> Priority: Minor
> Fix For: scr-1.0.8
>
> Attachments: ComponentMetadata.java, XmlHandler.java
>
>
> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
> for example, the following SCR.xml has two errors:
> <components>
> <component name="Component1">
> <implementation class="test.ds.hello.HelloComponent1"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> <component name=""Component2">
> <implementation class="test.ds.hello.HelloComponent2"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog2"
> unbind="unsetLog2"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> </components>
> -> the two components are embedded in an invalid <components> xml root element
> -> the component "Component2" has two references with the SAME name
> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
> 1/ log a WARNING message when finding an unknown/invalid xml element
> 2/ log a WARNING message when duplicate reference names are detected for one given component.
> Here is a tentative fix which is working for my use cases:
> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
> // Check that the references are ok.
> // We'll especially Check if there's not duplicate names in the component references.
> HashSet refs = new HashSet();
> Iterator referenceIterator = m_references.iterator();
> while ( referenceIterator.hasNext() )
> {
> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
> refMeta.validate( this );
> if (! refs.add(refMeta.getName())) {
> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
> }
> }
> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
> @@ -211,14 +211,21 @@
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> - }
> + }
> + else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> - }
> + } else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> -> Here is the complete/modifed method:
> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
> {
> // according to the spec, the elements should have the namespace,
> // except when the root element is the "component" element
> // So we check this for the first element, we receive.
> if ( firstElement )
> {
> firstElement = false;
> if ( localName.equals( "component" ) && "".equals( uri ) )
> {
> overrideNamespace = NAMESPACE_URI;
> }
> }
> if ( overrideNamespace != null && "".equals( uri ) )
> {
> uri = overrideNamespace;
> }
> if ( NAMESPACE_URI.equals( uri ) )
> {
> try
> {
> // 112.4.3 Component Element
> if ( localName.equals( "component" ) )
> {
> // Create a new ComponentMetadata
> m_currentComponent = new ComponentMetadata();
> // name attribute is mandatory
> m_currentComponent.setName( attrib.getProperty( "name" ) );
> // enabled attribute is optional
> if ( attrib.getProperty( "enabled" ) != null )
> {
> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
> }
> // immediate attribute is optional
> if ( attrib.getProperty( "immediate" ) != null )
> {
> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
> }
> // factory attribute is optional
> if ( attrib.getProperty( "factory" ) != null )
> {
> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
> }
> // Add this component to the list
> m_components.add( m_currentComponent );
> }
> // 112.4.4 Implementation
> else if ( localName.equals( "implementation" ) )
> {
> // Set the implementation class name (mandatory)
> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
> }
> // 112.4.5 [...] Property Elements
> else if ( localName.equals( "property" ) )
> {
> PropertyMetadata prop = new PropertyMetadata();
> // name attribute is mandatory
> prop.setName( attrib.getProperty( "name" ) );
> // type attribute is optional
> if ( attrib.getProperty( "type" ) != null )
> {
> prop.setType( attrib.getProperty( "type" ) );
> }
> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
> if ( attrib.getProperty( "value" ) != null )
> {
> prop.setValue( attrib.getProperty( "value" ) );
> m_currentComponent.addProperty( prop );
> }
> else
> {
> // hold the metadata pending
> m_pendingProperty = prop;
> }
> }
> // 112.4.5 Properties [...] Elements
> else if ( localName.equals( "properties" ) )
> {
> readPropertiesEntry( attrib.getProperty( "entry" ) );
> }
> // 112.4.6 Service Element
> else if ( localName.equals( "service" ) )
> {
> m_currentService = new ServiceMetadata();
> // servicefactory attribute is optional
> if ( attrib.getProperty( "servicefactory" ) != null )
> {
> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
> }
> m_currentComponent.setService( m_currentService );
> }
> else if ( localName.equals( "provide" ) )
> {
> m_currentService.addProvide( attrib.getProperty( "interface" ) );
> }
> // 112.4.7 Reference element
> else if ( localName.equals( "reference" ) )
> {
> ReferenceMetadata ref = new ReferenceMetadata();
> ref.setName( attrib.getProperty( "name" ) );
> ref.setInterface( attrib.getProperty( "interface" ) );
> // Cardinality
> if ( attrib.getProperty( "cardinality" ) != null )
> {
> ref.setCardinality( attrib.getProperty( "cardinality" ) );
> }
> if ( attrib.getProperty( "policy" ) != null )
> {
> ref.setPolicy( attrib.getProperty( "policy" ) );
> }
> //if
> ref.setTarget( attrib.getProperty( "target" ) );
> ref.setBind( attrib.getProperty( "bind" ) );
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> }
> else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> } else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (FELIX-639) Need more logs from SCR
Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12671183#action_12671183 ]
Felix Meschberger commented on FELIX-639:
-----------------------------------------
Pierre,
As explained above, I am reluctant to applying the patch to XmlHandler. Would that be ok with you ?
> Need more logs from SCR
> -----------------------
>
> Key: FELIX-639
> URL: https://issues.apache.org/jira/browse/FELIX-639
> Project: Felix
> Issue Type: Improvement
> Components: Declarative Services (SCR)
> Affects Versions: scr-1.0.2
> Environment: linux
> Reporter: Pierre De Rop
> Priority: Minor
> Attachments: ComponentMetadata.java, XmlHandler.java
>
>
> As explained in the dev post http://www.mail-archive.com/dev@felix.apache.org/msg05030.html,
> I would like the SCR to be improved in order to log some WARNINGs, when a SCR.xml descriptor contains invalid elements, event if it is well formed.
> for example, the following SCR.xml has two errors:
> <components>
> <component name="Component1">
> <implementation class="test.ds.hello.HelloComponent1"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> <component name=""Component2">
> <implementation class="test.ds.hello.HelloComponent2"/>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog"
> unbind="unsetLog"
> cardinality="1..n"
> policy="dynamic"/>
>
> <reference name="LOG" interface="org.osgi.service.log.LogService"
> bind="setLog2"
> unbind="unsetLog2"
> cardinality="1..n"
> policy="dynamic"/>
> </component>
> </components>
> -> the two components are embedded in an invalid <components> xml root element
> -> the component "Component2" has two references with the SAME name
> I would like the SCR to log some WARNINGs message, when finding these sort of errors:
> 1/ log a WARNING message when finding an unknown/invalid xml element
> 2/ log a WARNING message when duplicate reference names are detected for one given component.
> Here is a tentative fix which is working for my use cases:
> 1/ in the org.apache.felix.scr.impl.ComponentMetadata.validate() method (around line 299):
> // Check that the references are ok.
> // We'll especially Check if there's not duplicate names in the component references.
> HashSet refs = new HashSet();
> Iterator referenceIterator = m_references.iterator();
> while ( referenceIterator.hasNext() )
> {
> ReferenceMetadata refMeta = ( ReferenceMetadata ) referenceIterator.next();
> refMeta.validate( this );
> if (! refs.add(refMeta.getName())) {
> throw validationFailure( "Detected duplicate reference name: \"" + refMeta.getName() + "\"");
> }
> }
> 2/ in org.apache.felix.scr.impl.XmlHandler.startElement method (around line 215, at the end of the method):
> +++ src/main/java/org/apache/felix/scr/impl/XmlHandler.java (working copy)
> @@ -211,14 +211,21 @@
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> - }
> + }
> + else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> - }
> + } else {
> + throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> + ": \"" + localName + "\"", null);
> + }
> }
> -> Here is the complete/modifed method:
> public void startElement( String uri, String localName, Properties attrib ) throws ParseException
> {
> // according to the spec, the elements should have the namespace,
> // except when the root element is the "component" element
> // So we check this for the first element, we receive.
> if ( firstElement )
> {
> firstElement = false;
> if ( localName.equals( "component" ) && "".equals( uri ) )
> {
> overrideNamespace = NAMESPACE_URI;
> }
> }
> if ( overrideNamespace != null && "".equals( uri ) )
> {
> uri = overrideNamespace;
> }
> if ( NAMESPACE_URI.equals( uri ) )
> {
> try
> {
> // 112.4.3 Component Element
> if ( localName.equals( "component" ) )
> {
> // Create a new ComponentMetadata
> m_currentComponent = new ComponentMetadata();
> // name attribute is mandatory
> m_currentComponent.setName( attrib.getProperty( "name" ) );
> // enabled attribute is optional
> if ( attrib.getProperty( "enabled" ) != null )
> {
> m_currentComponent.setEnabled( attrib.getProperty( "enabled" ).equals( "true" ) );
> }
> // immediate attribute is optional
> if ( attrib.getProperty( "immediate" ) != null )
> {
> m_currentComponent.setImmediate( attrib.getProperty( "immediate" ).equals( "true" ) );
> }
> // factory attribute is optional
> if ( attrib.getProperty( "factory" ) != null )
> {
> m_currentComponent.setFactoryIdentifier( attrib.getProperty( "factory" ) );
> }
> // Add this component to the list
> m_components.add( m_currentComponent );
> }
> // 112.4.4 Implementation
> else if ( localName.equals( "implementation" ) )
> {
> // Set the implementation class name (mandatory)
> m_currentComponent.setImplementationClassName( attrib.getProperty( "class" ) );
> }
> // 112.4.5 [...] Property Elements
> else if ( localName.equals( "property" ) )
> {
> PropertyMetadata prop = new PropertyMetadata();
> // name attribute is mandatory
> prop.setName( attrib.getProperty( "name" ) );
> // type attribute is optional
> if ( attrib.getProperty( "type" ) != null )
> {
> prop.setType( attrib.getProperty( "type" ) );
> }
> // 112.4.5: If the value attribute is specified, the body of the element is ignored.
> if ( attrib.getProperty( "value" ) != null )
> {
> prop.setValue( attrib.getProperty( "value" ) );
> m_currentComponent.addProperty( prop );
> }
> else
> {
> // hold the metadata pending
> m_pendingProperty = prop;
> }
> }
> // 112.4.5 Properties [...] Elements
> else if ( localName.equals( "properties" ) )
> {
> readPropertiesEntry( attrib.getProperty( "entry" ) );
> }
> // 112.4.6 Service Element
> else if ( localName.equals( "service" ) )
> {
> m_currentService = new ServiceMetadata();
> // servicefactory attribute is optional
> if ( attrib.getProperty( "servicefactory" ) != null )
> {
> m_currentService.setServiceFactory( attrib.getProperty( "servicefactory" ).equals( "true" ) );
> }
> m_currentComponent.setService( m_currentService );
> }
> else if ( localName.equals( "provide" ) )
> {
> m_currentService.addProvide( attrib.getProperty( "interface" ) );
> }
> // 112.4.7 Reference element
> else if ( localName.equals( "reference" ) )
> {
> ReferenceMetadata ref = new ReferenceMetadata();
> ref.setName( attrib.getProperty( "name" ) );
> ref.setInterface( attrib.getProperty( "interface" ) );
> // Cardinality
> if ( attrib.getProperty( "cardinality" ) != null )
> {
> ref.setCardinality( attrib.getProperty( "cardinality" ) );
> }
> if ( attrib.getProperty( "policy" ) != null )
> {
> ref.setPolicy( attrib.getProperty( "policy" ) );
> }
> //if
> ref.setTarget( attrib.getProperty( "target" ) );
> ref.setBind( attrib.getProperty( "bind" ) );
> ref.setUnbind( attrib.getProperty( "unbind" ) );
> m_currentComponent.addDependency( ref );
> }
> else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
> catch ( Exception ex )
> {
> ex.printStackTrace();
> throw new ParseException( "Exception during parsing", ex );
> }
> } else {
> throw new ParseException("Invalid SCR xml element found in " + m_bundle.getLocation() +
> ": \"" + localName + "\"", null);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.