Bug 302770 - Change repository suffix to factoryID
Summary: Change repository suffix to factoryID
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: unspecified   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 3.6 M6   Edit
Assignee: Ian Bull CLA
QA Contact:
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2010-02-12 15:47 EST by Ian Bull CLA
Modified: 2010-02-28 00:56 EST (History)
1 user (show)

See Also:


Attachments
patch (18.69 KB, patch)
2010-02-15 19:41 EST, Ian Bull CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Bull CLA 2010-02-12 15:47:00 EST
When you create a repository, you give it a "suffix". This is really a unique ID for the factory. I propose we change our extension point to factoryID.
Comment 1 Ian Bull CLA 2010-02-15 18:07:17 EST
I noticed that we specify our filter (which is really our factory ID) as optional (cardinality 0..1).  To see if we really handle optional filters, I removed the filter for our simple metadata repository and a few hundred test cases failed.  

I wonder if the ID shouldn't be a required field instead?
Comment 2 Ian Bull CLA 2010-02-15 19:41:59 EST
Created attachment 159134 [details]
patch

This is a fairly significant change, but an important one in my opinion.  Instead of specifying two elements (factory and filter), we now just specify a single element (factory) with two attributes (class and ID). Both attributes are required.

In the previous model, filter was "optional", however, there was code that expected it to be set -- it was only really optional if you didn't mind the NPEs ;)

Pascal, can you review this?  It will break anyone who defines custom repos, so either we fix it now or mark as WONTFIX.
Comment 3 Ian Bull CLA 2010-02-28 00:54:47 EST
Pascal and I talked about this and we are going to leave things the way they are. The suffixes actually have value (even if that value is not being utilized today). However, there is a small bug with the schema.  Currently the filter is set to 0..1, but it really should be 1..1 -- that is, each repository *must* have a filter (our code already assumes this). There are interesting cases to have 1..n filters (like content.jar and content.xml being valid names for the simple artifact repo), but for now we are going with 1..1.
Comment 4 Ian Bull CLA 2010-02-28 00:56:05 EST
I've release the fix to the cardinality.