[
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/[email protected]/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.
