Bugzilla – Full Text Bug Listing
| Summary: | [RemoteSrvs] org.eclipse.ecf.osgi.services.distribution.AbstractHostContainerFinder.matchExistingHostContainer(...) fails to match host containers if no configs are given and defaults should be used | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [RT] ECF | Reporter: | Markus Kuppe <bugs.eclipse.org> | ||||||
| Component: | ecf.remoteservices | Assignee: | Markus Kuppe <bugs.eclipse.org> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | slewis | ||||||
| Version: | 3.3.0 | ||||||||
| Target Milestone: | 3.4.0 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 326122 | ||||||||
| Attachments: |
|
||||||||
|
Description
Markus Kuppe
*** Bug 326054 has been marked as a duplicate of this bug. *** Hi Markus. As far as I can tell, the specification doesn't say anything about handling the case of when service.exported.configs is null. Perhaps I've missed it, but I can't find in the spec any mention of this case. I expected to find some mention of things in section 13.5 Configuration Types, but I don't see any mention of that case. My suspicion and expectation is that null is a problematic case for the spec because it's ambiguous *which* provider should handle this default case. That is, if there are multiple providers (e.g. ECF (with multiple providers), CXF, etc), then which should actually create endpoints for this case? So unless the spec is clarified wrt proper multi-provider handling of *no* service.exported.configs, I don't think it would be a great idea to introduce a hard-coded default for this. I think it would be a good idea to clarify this explicitly in the spec (and if the behavior you describe in this bug is the appropriate one, then we can surely add/change that). (In reply to comment #2) > > My suspicion and expectation is that null is a problematic case for the spec > because it's ambiguous *which* provider should handle this default case. That > is, if there are multiple providers (e.g. ECF (with multiple providers), CXF, > etc), then which should actually create endpoints for this case? From a consumer (service implementor/provider) point of view I would probably not want to hard-wire the distribution provider via a mandatory service.exported.configs. I'd favor a solution where I either decide upon the distribution provider during deployment or use intent to generically describe/request distribution provider features. In case more than one distribution provider is deployed, no intents are given to narrow do possible providers and service.exported.configs is not set, I'd expect that all deployed distribution provider remote the service (which is totally valid IMO). > So unless the spec is clarified wrt proper multi-provider handling of *no* > service.exported.configs, I don't think it would be a great idea to introduce a > hard-coded default for this. I think it would be a good idea to clarify this > explicitly in the spec (and if the behavior you describe in this bug is the > appropriate one, then we can surely add/change that). I'm not saying we need a default which distribution provider to use. And I might even go as far as saying that if there are multiple ECF remoteservice providers installed, all of them should remote the service (like advertising via multiple discovery provider). (In reply to comment #3) > (In reply to comment #2) > > > > My suspicion and expectation is that null is a problematic case for the spec > > because it's ambiguous *which* provider should handle this default case. That > > is, if there are multiple providers (e.g. ECF (with multiple providers), CXF, > > etc), then which should actually create endpoints for this case? > > From a consumer (service implementor/provider) point of view I would probably > not want to hard-wire the distribution provider via a mandatory > service.exported.configs. I'd favor a solution where I either decide upon the > distribution provider during deployment or use intent to generically > describe/request distribution provider features. In case more than one > distribution provider is deployed, no intents are given to narrow do possible > providers and service.exported.configs is not set, I'd expect that all deployed > distribution provider remote the service (which is totally valid IMO). Although I don't think I fully agree with you about the solution described, I see these ideas as completely valid. But in any case, I think this is a spec-level discussion...i.e. whatever approach is taken to dealing with this case should be specified in the Remote Services specification. I would suggest that you bring it up with Peter Kriens directly via email...if you like I will initiate the email...as I've corresponded directly with Peter wrt other remote services spec issues. If you send directly to him then please copy me. Just let me know what you want to do. > > > So unless the spec is clarified wrt proper multi-provider handling of *no* > > service.exported.configs, I don't think it would be a great idea to introduce a > > hard-coded default for this. I think it would be a good idea to clarify this > > explicitly in the spec (and if the behavior you describe in this bug is the > > appropriate one, then we can surely add/change that). > > I'm not saying we need a default which distribution provider to use. And I > might even go as far as saying that if there are multiple ECF remoteservice > providers installed, all of them should remote the service (like advertising > via multiple discovery provider). But this (the default distribution provider to use when no config is given) is the key spec-level issue IMHO. i.e. without any spec about what config to use when none is given, what should be the actual behavior? Agreed, it could be that all of them remote the service...or none of them...or some singleton default...etc. The spec is currently ambiguous about this...unless I'm misreading (I would suggest that you take a look and see if I'm not catching it). In the face of this ambiguity, I think the current policy (none...from ECF's point of view) is the safest (note that their are some security questions/issues here since we're talking about distribution)...but if the spec is clarified otherwise, I'm comfortable with doing it differently. I just think we should get spec-level agreement about the solution. Irregardless of how the spec expects a distribution provider to behave, I think we have a bug here. The behavior I'm seeing is, that for two services both registered with service.exported.interfaces only, the DefaultHostContainerFinder fails to match the ECF generic server started for the first service with the second service. Subsequently DefaultHostContainerFinder tries to instantiate a second generic server which obviously fails due to a port conflict with the first instance. Bottom line, if I only register a single service with just service.exported.interfaces set, remoting works perfectly. (In reply to comment #5) > Irregardless of how the spec expects a distribution provider to behave, I think > we have a bug here. > The behavior I'm seeing is, that for two services both registered with > service.exported.interfaces only, the DefaultHostContainerFinder fails to match > the ECF generic server started for the first service with the second service. > Subsequently DefaultHostContainerFinder tries to instantiate a second generic > server which obviously fails due to a port conflict with the first instance. > Bottom line, if I only register a single service with just > service.exported.interfaces set, remoting works perfectly. Ok...please open bug report for this. My current surprise is that without a config specified that it does anything at all...but perhaps the check for null isn't done right for two cases: 1) using existing container; 2) creating a new container. In any event...we'll figure out and fix on new bug...please add me to the bug when created. For the time being, moving this bug's importance to normal though...as I don't think the ambiguity described in comments is going to be settled by us anytime soon. (In reply to comment #6) > Ok...please open bug report for this. My current surprise is that without a > config specified that it does anything at all...but perhaps the check for null > isn't done right for two cases: 1) using existing container; 2) creating a new > container. In any event...we'll figure out and fix on new bug...please add me > to the bug when created. Filed bug #326122 > For the time being, moving this bug's importance to normal though...as I don't > think the ambiguity described in comments is going to be settled by us anytime > soon. What about section 13.2.1: "If no configuration types are recognized, the distribution provider should create an endpoint with a default configuration type except when one of the listed configuration types is <<nodefault>>." It's not 100% clear, but I read it as if a distribution provider should per default create an endpoint (as long as <<nodefault>> is not set). Anyway I raised a discussion on the OSGi developer mailinglist [0]. [0] https://mail.osgi.org/pipermail/osgi-dev/2010-September/002641.html (In reply to comment #7) > > For the time being, moving this bug's importance to normal though...as I don't > > think the ambiguity described in comments is going to be settled by us anytime > > soon. > > What about section 13.2.1: > > "If no configuration types are recognized, the distribution provider should > create an endpoint with a default configuration type except when one of the > listed configuration types is <<nodefault>>." > > It's not 100% clear, but I read it as if a distribution provider should per > default create an endpoint (as long as <<nodefault>> is not set). Anyway I > raised a discussion on the OSGi developer mailinglist [0]. > > [0] https://mail.osgi.org/pipermail/osgi-dev/2010-September/002641.html Here's a response from David Bosschaert: "If service.exported.configs is not specified, the Distribution Provider should use its default mechanism. >From section 122.5.1: • service.exported.configs – (String+ ) A list of configuration types that should be used to export this service. Each configuration type represents the configuration parameters for an Endpoint. A Remote Service Admin service should create an Endpoint for each configuration type that it supports and ignore the types it does not recognize. If this property is not set, then the Remote Service Admin implementation must choose a convenient configuration type that then must be reported on the Endpoint Description with the service.imported.configs associated with the returned Export Registration." FWIW: flipping org.eclipse.ecf.osgi.services.distribution.AbstractHostContainerFinder.matchHostSupportedConfigTypes(String[], ContainerTypeDescription) line 179 from false to true fixes my problems with two containers being created. Created attachment 179505 [details]
Accept null config types
(In reply to comment #8) <stuff deleted> > Here's a response from David Bosschaert: > > "If service.exported.configs is not specified, the Distribution > Provider should use its default mechanism. > >From section 122.5.1: > > • service.exported.configs – (String+ ) A list of configuration types > that should be used to export > this service. Each configuration type represents the configuration > parameters for an Endpoint. A > Remote Service Admin service should create an Endpoint for each > configuration type that it supports > and ignore the types it does not recognize. If this property is not > set, then the Remote Service > Admin implementation must choose a convenient configuration type that > then must be reported > on the Endpoint Description with the service.imported.configs > associated with the returned > Export Registration." Ok. My comment about the spec is: it should be clearer. Perhaps it's just textual organization (burying this info in a single place), but it should still be clearer. I recommend that the spec be updated to be clearer. Actually, I would suggest having an indication for every service property (since that's the remote services API, effectively), of the appropriate/expected behavior when not set/null. Please go ahead and implement logic to use ECF generic as default (actually the container finder has a configurable mechanism to use the ECF generic as default, so it should use whatever is configured as the default)...see AbstractHostContainerFinder.getDefaultConfigTypes(). I suppose it already does this on line 205 of AbstractHostContainerFinder...this is what you are seeing with bug 326122. So I think the only thing remaining to do is to fix the container finder search for an *existing* container of default type...that is, the logic should be (assuming no required configs) 1) When the container doesn't exist, create it (as I think it already does on line 205 of AbstractHostContainerFinder) 2) When an instance of default type *does* exist, find/use/select it Note also for reference that the default is defined here when the DefaultHostContainerFinder is registered: org.eclipse.ecf.internal.osgi.services.distribution.Activator.defaultHostConfigType Released patch to HEAD Created attachment 179562 [details]
mylyn/context/zip
|