You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by "Felix Meschberger (JIRA)" <ji...@apache.org> on 2008/07/23 16:19:33 UTC
[jira] Commented: (FELIX-639) Need more logs from SCR
[ 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.