Skip to content

Add generic support for XEP-0128: Service Discovery Extensions#3138

Open
Fishbowler wants to merge 7 commits intomainfrom
ext-info-extensions
Open

Add generic support for XEP-0128: Service Discovery Extensions#3138
Fishbowler wants to merge 7 commits intomainfrom
ext-info-extensions

Conversation

@Fishbowler
Copy link
Member

I want to implement XEP-0504: Data Policy (https://xmpp.org/extensions/xep-0504.html) which relies on XEP-0128: Service Discovery Extensions.

We've already got 2 specific XEP implementations (Contact Addresses, Software Info) that depend on XEP-0128, and we've got the MUC Extended Info plugin that extends disco responses too.

This change creates a pluggable ExtendedDiscoInfoProviders capability for plugins to add support for XEPs (or other functionality) that rely on XEP-0128.

It also moves the Contact Addresses and the Software Info implementations to use the new thing, and updates the DOAP.

Lots of AI in the JavaDoc and unit tests, which I think I've fixed up, but use sufficient scepticism.

ExtendedDiscoInfoProviders can be registered with IQDiscoInfoHandler, and each will be called on disco requests to potentially add forms or form fields to the response.
Also made XEP-0128 a first class citizen, since there's now generic capability here.
@Fishbowler Fishbowler requested a review from guusdk February 12, 2026 22:50
Copy link
Member

@guusdk guusdk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left some minor feedback inline. There's an overarching concern: conceptually, this change feels odd to me. It doesn’t introduce new functionality, but rather an alternative way to achieve the same result. Openfire already provides an extension point for obtaining “extended infos” via org.jivesoftware.openfire.disco.DiscoInfoProvider#getExtendedInfos. This commit introduces a second mechanism for essentially the same purpose. In fact, the getExtendedInfos method in the new ExtendedDiscoInfoProvider has the exact same signature as the existing one, which feels like a code smell.

The main difference seems to be how the data from DiscoInfoProvider and ExtendedDiscoInfoProvider is combined into the final result, with the new provider adding more complex merging logic. I wonder whether this PR could be improved by eliminating the second provider and instead applying the merging logic to the existing extension point. Is there any functionality enabled by the current approach that wouldn’t be possible that way?

@Fishbowler
Copy link
Member Author

@guusdk

this change feels odd to me. It doesn’t introduce new functionality, but rather an alternative way to achieve the same result. Openfire already provides an extension point for obtaining “extended infos”

The existing functionality doesn't expose an ability to extend disco results via a plugin, except via the proxy approach in mucextinfo.

This felt like a good way to offer a simple hook to plugins. There's a much simpler version that deals with stuff like Data Policy, but I also wanted to cater for mucextinfo and similar, and bring the merge stuff in-house at the same time.

Right now, the testing has shown that this implementation has gaps (like not covering MultiUserChatServiceImpl or PubSubModule's implementations of getExtendedInfos), so if you've got thoughts on how to better encapsulate this functionality for a plugin to extend a disco#info, am happy to adapt.

I'm thinking that I might pull some stuff out of the anonymous DiscoInfoProvider at L616 and put it into the IQDiscoInfoHandler and call it from handleIQ instead (where it calls out to the other implementations). But at the point where its being hoisted to the root of the class, it could also all be moved to some sensible place in a file nearby, probably as mostly static methods... WDYT?

This means that we'll see requests to Conference, PubSub, etc be routed through this method. It adds a domain to the method signature in ExtendedDiscoInfoProvider.getExtendedInfos to allow an ExtendedDiscoInfoProvider to apply domain-specific logic.
@Fishbowler Fishbowler requested a review from guusdk February 20, 2026 23:04
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 this pull request may close these issues.

2 participants