Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the same PID for both pid and factoryPid attribute in @ObjectClassDefinition #6042

Closed
kwin opened this issue Feb 23, 2024 · 6 comments · Fixed by #6052
Closed

Allow the same PID for both pid and factoryPid attribute in @ObjectClassDefinition #6042

kwin opened this issue Feb 23, 2024 · 6 comments · Fixed by #6052

Comments

@kwin
Copy link
Contributor

kwin commented Feb 23, 2024

When using the following code snippet

@ObjectClassDefinition(factoryPid="com.example.MyComponent", pid="com.example.MyComponent")
private static @interface Config {
  @AttributeDefinition(name = "Some setting")
  String setting();
}

Bnd bails out with Duplicate pid com.example.MyComponent from class ...Config when generating the metadata (e.g. with bnd-maven-plugin:bnd-process). This validation happens in

analyzer.error("Duplicate pid %s from class %s", dDef.pid, c.getFQN());
.

According to https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.metatype.html#org.osgi.service.metatype.annotations.ObjectClassDefinition or https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.metatype.html#i1492258 that is not necessarily forbidden.

Although probably an edge case, sometimes the same component class should be used both via configuration factories as well as a singleton configuration (e.g. for migrating purposes from one to the other while staying backwards compatible from the configuration signature)

@stbischof
Copy link
Contributor

stbischof commented Feb 23, 2024

Not an edge case. Each conponent with a pid is implicit an factory pid

Use the pid in configadmin to configured the factoryconfiguration

E.g. the felix http.whiteboard

Multiple Servers
It is possible to configure several Http Services, each running on a different port. The first service can be configured as outlined above using the service PID for "org.apache.felix.http". Additional servers can be configured through OSGi factory configurations using "org.apache.felix.http" as the factory PID. The properties for the configuration are outlined as above.

The default server using the PID "org.apache.felix.http" can be disabled by specifying a negative port and then all servers can be used through factory configurations.

@kwin
Copy link
Contributor Author

kwin commented Feb 23, 2024

The problem is the existing persisted configurations with a PID not containing a ~. But according to https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.component.html#service.component-deployment an OSGi DS Component is either configured through a PID or a Factory PID (never both).

@pkriens
Copy link
Member

pkriens commented Mar 15, 2024

If you make PR that changes this and it actually works (the creation of the Designate element seems problematic, but you'll find it out by trying.

If you're not intending to make a PR, let's move this to abeyeance?

@kwin
Copy link
Contributor Author

kwin commented Mar 15, 2024

Before starting to work on this I would like to get confirmation from someone more familiar with the spec that the duplicate id check in

analyzer.error("Duplicate pid %s from class %s", dDef.pid, c.getFQN());
is not based on some specification detail I am not aware of. Does anyone see any problem with one designate for a factory PID and another one to a regular PID with the same value?

kwin added a commit to kwin/bnd that referenced this issue Mar 15, 2024
factory pid

This closes bndtools#6042

Signed-off-by: Konrad Windszus <kwin@apache.org>
kwin added a commit to kwin/bnd that referenced this issue Mar 15, 2024
factory pid

This closes bndtools#6042

Signed-off-by: Konrad Windszus <kwin@apache.org>
kwin added a commit to kwin/bnd that referenced this issue Mar 15, 2024
factory pid

This closes bndtools#6042

Signed-off-by: Konrad Windszus <kwin@apache.org>
@pkriens
Copy link
Member

pkriens commented Mar 15, 2024

I think the code is in error. As far as I can see the pid/factoryPid must be checked in separate namespaces. I can't see any reason why using the same pid for factory and singleton would be problematic.

We're now using the set pidsto check for the uniqueness, we should add a set factoryPids and check factories against that. Or append the factory boolean to the PID?

I'd prefer to close this because I find it an edge case but a PR is ok as long as this issue is closed I am happy :-)

@kwin
Copy link
Contributor Author

kwin commented Mar 15, 2024

@pkriens Please check #6052 which does exactly that, i.e. use separate namespaces for factory and non factory pids.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants