Bug 349711 - [metatype] Improve metatype implementation in equinox to allow better use of schema extensions
Summary: [metatype] Improve metatype implementation in equinox to allow better use of ...
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: Compendium (show other bugs)
Version: 3.7   Edit
Hardware: PC Mac OS X - Carbon (unsup.)
: P3 normal (vote)
Target Milestone: Juno M1   Edit
Assignee: John Ross CLA
QA Contact:
URL:
Whiteboard:
Keywords: plan
Depends on:
Blocks: 353049
  Show dependency tree
 
Reported: 2011-06-17 13:47 EDT by Alasdair Nottingham CLA
Modified: 2011-08-09 09:19 EDT (History)
3 users (show)

See Also:


Attachments
proposed api (5.20 KB, patch)
2011-07-05 14:43 EDT, John Ross CLA
no flags Details | Diff
updated proposed api (6.94 KB, patch)
2011-07-06 19:09 EDT, John Ross CLA
no flags Details | Diff
work in progress patch (264.21 KB, patch)
2011-07-10 19:39 EDT, John Ross CLA
no flags Details | Diff
work in progress binary jar (105.27 KB, application/x-zip-compressed)
2011-07-10 19:40 EDT, John Ross CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alasdair Nottingham CLA 2011-06-17 13:47:54 EDT
The metatype specification from OSGi allows xsd:anyAttribute to be applied to elements such as OCD and AD, however while their is an API for accessing the core parts of OCD and AD their is no way to get information defined about extensions.

So if I decided to add additional information to the metatype I would need to parse the XML independently myself. I would essentially need to implement metatype all over in order to get at this extended information.

I would like some value add for the equinox metatype implementation which would allow me to access this information.

A solution to do this would be to provide equinox specific extensions to the base OSGi classes and register both with the service registry. If I looked it up using the equinox extended it would provide additional methods on ObjectClassDefinition and AttributeDefinition as in this example:

package org.eclipse.equinox.metatype
public interface ObjectClassDefinition extends org.osgi.service.metatype.ObjectClassDefinition
{
    public Collection<String> getExtentionSchemas();
    public Map<String, String> getExtentionData(String schemaId);
    public String getExtentionValue(String schemaId, String attributeName);
}

The first would list all schemas that extend that OCD and the second would give you all the attributes and their value under that OCD, the third would allow you to get a single value directly.

I'm less worried about how this is possible, as long as some way can be provided to make it possible.
Comment 1 BJ Hargrave CLA 2011-06-17 14:51:23 EDT
Or you could just parse the XML yourself and ignore the element and attribute not in your namespace. You don't need to reimplement metatype.

I don't think the right thing to do is to pollute the metatype implementation to save you an XML parse.
Comment 2 Alasdair Nottingham CLA 2011-06-17 15:12:27 EDT
(In reply to comment #1)
> Or you could just parse the XML yourself and ignore the element and attribute
> not in your namespace. You don't need to reimplement metatype.
> 
> I don't think the right thing to do is to pollute the metatype implementation
> to save you an XML parse.

OK, so this data relates to the OCD and AD, so lets see what I would need to do:

1. Access each bundle looking for the metatype according to the metatype spec.
2. Parse the XML, understanding the semantics of both the metatype schema and any extensions, maintain the mapping.
3. Go back to the Metatype API and query for the OCD/AD using that attribute
4. Associate the information I need from the OCD/AD and my extension
5. Do my processing

Step 3 isn't really required, I'm just trying to reuse metatype API here for that data. This seems very complex, for what we gain. If I'm going to do that it is a small delta to just finish implementing metatype.

I'm happy to discuss ways to access this data and be efficient, but if metatype wasn't supposed to allow for this then why does it allows xsd:anyAttribute and xsd:any? All I'm doing is asking for metatype to help me get at the data metatype allows me to add.
Comment 3 BJ Hargrave CLA 2011-06-17 15:35:55 EDT
(In reply to comment #2) 
> OK, so this data relates to the OCD and AD, so lets see what I would need to
> do:
> 
> 1. Access each bundle looking for the metatype according to the metatype spec.
> 2. Parse the XML, understanding the semantics of both the metatype schema and
> any extensions, maintain the mapping.
> 3. Go back to the Metatype API and query for the OCD/AD using that attribute
> 4. Associate the information I need from the OCD/AD and my extension
> 5. Do my processing
> 
> Step 3 isn't really required, I'm just trying to reuse metatype API here for
> that data. 

It seemed you were not trying to reuse the metatype API but rather to change it by extension.

> This seems very complex, for what we gain. If I'm going to do that
> it is a small delta to just finish implementing metatype.
> 
> I'm happy to discuss ways to access this data and be efficient, but if metatype
> wasn't supposed to allow for this then why does it allows xsd:anyAttribute and
> xsd:any? 

Mostly that is future proofing the schema. Certainly in the case of anyattribute. XML Schema 1.0 kinda sucks, so we had to use #other for any.

> All I'm doing is asking for metatype to help me get at the data
> metatype allows me to add.

Well you are asking the metatype implementation to parse your data for you. And create an "overlay" API that will couple people to the Equinox metatype implementation.
Comment 4 Thomas Watson CLA 2011-06-17 16:30:25 EDT
(In reply to comment #3)
> 
> Well you are asking the metatype implementation to parse your data for you. And
> create an "overlay" API that will couple people to the Equinox metatype
> implementation.

On the other hand it allows us to experiment with the idea of exposing this extra data.  At some point we may decide that this is something we want to spec as an enhancement to OSGi.  But until that happens, yes users would be tied to the Equinox API we invent, but others could decide to take that API and impmlement it them selves also.

I'm still unsure about the usercase for placing this extra information into the metadata xml file.  Why is it needed here and who will act upon it?
Comment 5 Alasdair Nottingham CLA 2011-06-17 16:41:48 EDT
(In reply to comment #3)

> It seemed you were not trying to reuse the metatype API but rather to change it
> by extension.
> 

Sorry I should have made that clear

> 
> Mostly that is future proofing the schema. Certainly in the case of
> anyattribute. XML Schema 1.0 kinda sucks, so we had to use #other for any.
> 
Interesting, I guess I would have expected a new version of the schema for future enhancements.

> 
> Well you are asking the metatype implementation to parse your data for you. And
> create an "overlay" API that will couple people to the Equinox metatype
> implementation.

Yes, and I don't see a problem with that. If people want the extra data they can use the API, if they don't they would use the OSGi API. I'm not trying to remove anything, just ask for an addition. I think of this as "value-add"

(In reply to comment #4)
> I'm still unsure about the usercase for placing this extra information into the
> metadata xml file.  Why is it needed here and who will act upon it?

I have several reasons why I want to do this, but I'll describe the one I think shows the most value. I'm writing a management agent that puts data in config admin. It uses the metatype to work out the type of the object to put into config admin. It also wants to allow a mechanism to validate the data entered. Let us assume it wants to allow the specification of a regexp. Now I could store this data externally in another file, but since I'm writing my metatype, and I know this agent will be used I wish to add it into the metatype right along with the rest of the type information. So I do this:

<AD name="%name" id="someId" type="String" validation:regexp="[a-zA-Z0-9]" />

It is then all provided in one place, in a simple way. I know that config admin could still end up with things that don't match that regexp and the consumer needs to cope with that, but it would allow the management agent to provide a nicer message to the user in a more timely fashion.
Comment 6 BJ Hargrave CLA 2011-06-17 16:48:34 EDT
(In reply to comment #5)
> > 
> > Mostly that is future proofing the schema. Certainly in the case of
> > anyattribute. XML Schema 1.0 kinda sucks, so we had to use #other for any.
> > 
> Interesting, I guess I would have expected a new version of the schema for
> future enhancements.

Yes, a new version of the schema will be made, but older document processors will be able to process documents from the new schema (unless the new elements/attributes are marked mustUnderstand). That is the point of the anys.
Comment 7 Alasdair Nottingham CLA 2011-06-17 16:59:10 EDT
(In reply to comment #6)
> (In reply to comment #5)
> > > 
> > > Mostly that is future proofing the schema. Certainly in the case of
> > > anyattribute. XML Schema 1.0 kinda sucks, so we had to use #other for any.
> > > 
> > Interesting, I guess I would have expected a new version of the schema for
> > future enhancements.
> 
> Yes, a new version of the schema will be made, but older document processors
> will be able to process documents from the new schema (unless the new
> elements/attributes are marked mustUnderstand). That is the point of the anys.

Hmm, wouldn't that involve me using both the old and the new schema though? I might just not understand how schema works, but it seems to me you would need to use the v1 and v1.1 schemas in your metatype to support that scenario. Not that this is really relevant to this bug.
Comment 8 BJ Hargrave CLA 2011-06-17 17:29:26 EDT
(In reply to comment #7)
> Hmm, wouldn't that involve me using both the old and the new schema though? I
> might just not understand how schema works, but it seems to me you would need
> to use the v1 and v1.1 schemas in your metatype to support that scenario. Not
> that this is really relevant to this bug.

Using attributes in this example, anyattribute forces document processors for documents in the current schema to ignore unrecognized attributes. So when an attribute is introduced in a future version of the schema, documents in the future schema can be processed by document processors for the older schema. That is the theory anyway...
Comment 9 John Ross CLA 2011-06-20 11:41:49 EDT
(In reply to comment #0)
> public interface ObjectClassDefinition extends
> org.osgi.service.metatype.ObjectClassDefinition
> {
>     public Collection<String> getExtentionSchemas();
>     public Map<String, String> getExtentionData(String schemaId);
>     public String getExtentionValue(String schemaId, String attributeName);
> }

You seem to only be interested in attributes from other namespaces that were added to elements (particularly OCD and AD) within the "metatype" namespace via the <anyAttribute/> extension. However, all of the complex types within the metatype schema support both <anyAttribute/> and <any/>.

To handle it generically, I would be tempted to provide the following, intentionally leaving out the convenience method.

interface Extendable {
     Collection<String> getExtensionSchemas();
     Map<String, String> getExtensionAttributes(String schema);
}

This only handles extension attributes, which I'm sure you intended. To handle extension elements as well, assuming you don't want to reuse Element and Attribute from the XML APIs, you end up with something like this.

interface Extendable {
     Collection<String> getExtensionSchemas();
     Map<String, String> getExtensionAttributes(String schema);
     List<Extension> getExtensionElements(String schema);
}
interface Extension {
     Map<String, String> getAttributes();
     List<Extension> getElements();
     String getSchema();
     String getValue();
}

However, extension schemas could allow their own extensions, so it seems you ultimately end up with this.

interface Extendable {
     Collection<String> getExtensionSchemas();
     Map<String, String> getExtensionAttributes(String schema);
     List<Extension> getExtensionElements(String schema);
}
interface Extension extends Extendable {
     Map<String, String> getAttributes();
     List<Extension> getElements();
     String getSchema();
     String getValue();
}

So I guess the questions are (1) Do we want to do this at all; (2) Do we want to provide only a partial solution that addresses extension attributes, and possibly only for OCD and AD; or (3) Do we provide a complete solution.
Comment 10 Alasdair Nottingham CLA 2011-06-20 11:52:59 EDT
(In reply to comment #9)

> 
> So I guess the questions are (1) Do we want to do this at all; (2) Do we want
> to provide only a partial solution that addresses extension attributes, and
> possibly only for OCD and AD; or (3) Do we provide a complete solution.

Hi,

I was trying to propose the minimally invasive extension to the existing API. Ideally I would like to be able to add attributes to: Designate, Object, OCD and AD. The current API only really gets me access to OCD and AD and I'm therefore slightly uncertain how we would allow this on Designate and Object.

I did consider elements, but attributes are good enough for me at this time. I don't have a use case for elements which is why I didn't detail one. So in my view:

1) Yes
2) Yes
3) Not at this time.
Comment 11 John Ross CLA 2011-07-05 14:43:45 EDT
Created attachment 199144 [details]
proposed api

This patch contains an API proposal. The API classes reside in the temporary org.eclipse.equinox.metatype.i1 package. Ultimately, I think they would move to org.eclipse.equinox.metatype and the existing package renamed to org.eclipse.equinox.metatype.impl.

This API addresses the following requirements:

(1) Access attribute extension data off of OCD and AD.
(2) Access Designate related data as well as any attribute extensions thereof.
(3) Access new functionality as a service.
(4) Transparently use new functionality as standard OSGi API if desired.

Please review.
Comment 12 Alasdair Nottingham CLA 2011-07-06 06:22:50 EDT
Comment on attachment 199144 [details]
proposed api

I have some questions:

1. The Attribute interface doesn't seem to be referenced anywhere, is it required? What is it for?
2. Should Designate.getObject() return java.lang.Object or something else that would allow access to the data?
3. Will getExtensionAttributes do type coercion based on the schema types?
4. The getEquinox... methods look nasty to me. How about getExtension... instead?
Comment 13 John Ross CLA 2011-07-06 09:59:06 EDT
(In reply to comment #12)
> Comment on attachment 199144 [details]
> proposed api
> I have some questions:
> 1. The Attribute interface doesn't seem to be referenced anywhere, is it
> required? What is it for?

Well, it's something you asked for :) You wanted access to the attribute data that could be included with a designate for the purpose of describing an actual configuration based on the metadata. In the current API, you would get to it via Desginate.getObject().getAttributes().

> 2. Should Designate.getObject() return java.lang.Object or something else that
> would allow access to the data?

Not sure I understand what you're getting at. Currently, it returns type org.eclipse.equinox.metatype.i1.Object which gives access to the attributes and OCD. I know some might have an issue with reusing a very common name, and I'm willing to change it. I just wanted the initial API to be as clear as possible in terms of the connection with the XML.

> 3. Will getExtensionAttributes do type coercion based on the schema types?

Not in the first iteration, at least. I used java.lang.Object as the value in case we want to do it in the future. I don't have strong feelings against making it type String instead.

> 4. The getEquinox... methods look nasty to me. How about getExtension...
> instead?

I'm not particularly fond of the name either and considered getExtension... at first. However, that name could be misleading on MetaTypeService since the returned MetaTypeInformation is not (and cannot be) Extendable. The reason is the MetaTypeInformation represents a composite of all metadata XML files included in a bundle.

Note that I removed the MetaTypeInformation.getDesignates() method in my working copy. It's unnecessary since the same information can be accessed via OCD.getDesignates(). It's also wrong because what really needs to be on MetaTypeInformation is getMetaData() : Collection<MetaData>, but it's not necessary in order to meet the current requirements.
Comment 14 John Ross CLA 2011-07-06 19:09:22 EDT
Created attachment 199217 [details]
updated proposed api

The patch has been updated as follows.

Renamed existing types from org.osgi.service.metatype:
	EquinoxMetaTypeService
	EquinoxMetaTypeInformation
	EquinoxObjectClassDefinition
	EquinoxAttributeDefinition

I used Equinox* instead of Extendable* because the Extendable monicker does not apply to all of the types. We could have ExtendableObjectClassDefinition and ExtendableAttributeDefinition instead, and I'm not particular about using Equinox* for the others, but I don't want to use Extendable* on interfaces that don't extend Extendable because it would be misleading.

Changed Extendable.getExtensionAttributes(String) : Map<String, java.lang.Object> to Extendable.getExtensionAttributes(String) : Map<String, String>. Type conversion will not be supported.

Changed method names on EquinoxMetaTypeService, EquinoxMetaTypeInformation, and EquinoxObjectClassDefinition to match the super interface method names. Only the return type has changed.

Renamed org.eclipse.equinox.metatype.i1.Object to DesignateObject.

Removed method EquinoxMetaTypeInformation.getDesignates(). This information is accessable via EquinoxObjectClassDefinition.getDesignates().
Comment 15 Alasdair Nottingham CLA 2011-07-07 07:00:41 EDT
(In reply to comment #14)
> Created attachment 199217 [details]

looks good to me.
Comment 16 John Ross CLA 2011-07-07 20:52:25 EDT
I assume you're expecting the uris, as opposed to the prefixes, to be returned in Extendable.getExtensionSchemas(). This would be ideal since the uri never changes, but the prefixes could vary from document to document.

Awareness of the uris, however, requires a namespace aware parser. This ability isn't guaranteed since Metatype is still required to support minimal environments that might have a skimpy XML parser. eXML, for example, supports neither validation nor, as I recall, namespace awareness.

I'd like to keep it simple and say that extensions will only be available if the parser factory supports namespace awareness. Any issues? I think this will surely be the case in your target environments.

Currently, MetaType grabs the first available SAXParserFactory service with whatever settings and merrily rolls along. I think this needs to change so that Metatype will favor SAXParserFactory services that create namespace aware parsers.

The framework automatically registers a SAXParserFactory service from the system bundle but it uses the default values (no validation, no namespace awareness). I don't think it's acceptable for a bundle to muck with the factory service settings because that would affect all other using bundles. So either the framework should try to register a namespace aware factory or you will have to do this yourself.
Comment 17 Alasdair Nottingham CLA 2011-07-08 05:31:08 EDT
(In reply to comment #16)
> I assume you're expecting the uris, as opposed to the prefixes, to be returned
> in Extendable.getExtensionSchemas(). This would be ideal since the uri never
> changes, but the prefixes could vary from document to document.
> 

Yes, I don't think prefixes are useful for the reason you state.

> Awareness of the uris, however, requires a namespace aware parser. This ability
> isn't guaranteed since Metatype is still required to support minimal
> environments that might have a skimpy XML parser. eXML, for example, supports
> neither validation nor, as I recall, namespace awareness.
> 
> I'd like to keep it simple and say that extensions will only be available if
> the parser factory supports namespace awareness. Any issues? I think this will
> surely be the case in your target environments.

Sounds good to me.

> 
> Currently, MetaType grabs the first available SAXParserFactory service with
> whatever settings and merrily rolls along. I think this needs to change so that
> Metatype will favor SAXParserFactory services that create namespace aware
> parsers.
> 
> The framework automatically registers a SAXParserFactory service from the
> system bundle but it uses the default values (no validation, no namespace
> awareness). I don't think it's acceptable for a bundle to muck with the factory
> service settings because that would affect all other using bundles. So either
> the framework should try to register a namespace aware factory or you will have
> to do this yourself.
Comment 18 John Ross CLA 2011-07-10 19:39:14 EDT
Created attachment 199393 [details]
work in progress patch

This patch has api, impl, and tests. It includes the requested functionality for accessing extension attributes from OCD and AD. 

Now that I'm comfortable the proposed API can easily absorb the potential future functionality, I removed the currently unnecessary aspects like accessing designate info and extension elements for now. 

Everything that used to be under org.eclipse.equinox.metatype is now under org.eclipse.equinox.metatype.impl. The API interfaces are in the original package.

I also bumped the version up to 1.2.0.

Alasdair, not sure how you want to consume this so, in addition to the patch, I'll attach a binary jar exported from my workspace.
Comment 19 John Ross CLA 2011-07-10 19:40:37 EDT
Created attachment 199394 [details]
work in progress binary jar

Here's the jar with the binaries.
Comment 20 John Ross CLA 2011-07-25 17:11:39 EDT
I went ahead and committed these changes for M1. I'll open a new bug as an API review reminder for M2. Please open new bugs for any issues with the current API or requests for new functionality.

https://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=421152f66c0b6b912906a313e81d32424c873eb2

https://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=4c572581cc1d883ce35fba464462ee1911da37fa
Comment 21 Thomas Watson CLA 2011-08-09 09:12:38 EDT
Marking as fixed.  We will do additional review in bug353049