Modularize the HTTP transport layer: independent Jetty 9, Jetty 12, servlet, and HttpService bundles by holgerfriedrich · Pull Request #365 · jupnp/jupnp · GitHub
Skip to content

Modularize the HTTP transport layer: independent Jetty 9, Jetty 12, servlet, and HttpService bundles - #365

Open
holgerfriedrich wants to merge 47 commits into
jupnp:mainfrom
holgerfriedrich:modularize-transport
Open

holgerfriedrich wants to merge 47 commits into
jupnp:mainfrom
holgerfriedrich:modularize-transport

Conversation

@holgerfriedrich

@holgerfriedrich holgerfriedrich commented Jul 21, 2026 •

Copy link
Copy Markdown

Use this as the commit message if GitHub squash-merges this PR (paste into the squash commit message box):

Modularize the HTTP transport layer into independent Jetty 9, Jetty 12, servlet, and HttpService bundles, and add a jakarta.servlet/OSGi Whiteboard bridge

Core no longer bundles a transport implementation, servlet support, or an OSGi
HttpService/Whiteboard bridge. Six new/changed bundles take over:

- org.jupnp.transport.jetty9: the existing Jetty 9.4 transport, moved out of
  core unchanged.
- org.jupnp.transport.jetty12 (new, optional): a Jetty 12 transport for Java
  17+, built on Jetty's servlet-free core API, with its own standalone
  StreamServer -- it opens its own port unless a SharedStreamServerProvider
  is also installed (see below).
- org.jupnp.transport.javax.servlet (new): the generic javax.servlet transport
  infrastructure (ServletStreamServerImpl, ServletContainerAdapter, the
  blocking/async servlet variants), shared by jetty9 and Android, neither of
  which is Jetty- or Android-specific about it.
- org.jupnp.transport.javax.httpservice (new, optional): the classic OSGi
  HttpService bridge, registered behind a new generic
  SharedStreamServerProvider SPI so core has no compile-time dependency on
  javax.servlet or the HttpService API at all, in any form.
- org.jupnp.transport.jakarta.servlet (new, optional): a jakarta.servlet 6.1
  port of org.jupnp.transport.javax.servlet, async-only -- jakarta.servlet has
  mandated async support since 5.0, so no blocking/detection fallback is
  needed.
- org.jupnp.transport.jakarta.whiteboard (new, optional): the jakarta.servlet
  counterpart of org.jupnp.transport.javax.httpservice, bridging to the OSGi HTTP
  Whiteboard Service for Jakarta Servlet (org.osgi.service.servlet 2.0.0),
  the Whiteboard-based replacement runtimes like Pax Web 10+ use instead of
  the classic HttpService.

Applications must now depend on org.jupnp.transport.jetty9 or
org.jupnp.transport.jetty12 explicitly -- core no longer pulls in a transport
for free, and org.jupnp.bom (a conventional Maven BOM: import it, then declare
exactly the artifacts you want) does not auto-select one either, including
based on build JDK version. Android is already updated to add this
dependency; other consumers will need the same change.

Transport discovery: outside OSGi, both transports register via
java.util.ServiceLoader, the same mechanism core previously used for its
built-in transport; ServiceLoader finding more than one fails fast with a
clear error instead of picking one non-deterministically. In OSGi, transports
and shared-server providers are wired via Declarative Services instead:
OSGiUpnpServiceConfiguration takes each through a reference that collects all
matching services and fails the same way on ambiguity, so discovery doesn't
race bundle start order and doesn't silently pick a bundle when more than one
is present. Port sharing is decided purely by which SharedStreamServerProvider
is bound, independent of which transport bundle supplies the StreamClient.

Dependency updates: Jetty 9 (9.4.53 -> 9.4.58), Pax Web 8.x (8.0.25 ->
8.0.36), Jetty 12 (12.1.10 -> 12.1.12), Pax Web 11.x (11.1.2 -> 11.1.3),
Pax Logging (-> 2.3.5), logback (1.5.35 -> 1.6.2), slf4j (2.0.11 -> 2.0.18).

Project version is set to 4.0.0-SNAPSHOT.

This PR was generated with significant use of AI.

What this does

This started as the transport split requested in #297 (comment) and grew into a full modularization of jUPnP's HTTP layer. Six bundles now do what core used to do internally:

Bundle Status Contains
org.jupnp.transport.jetty9 moved, unchanged Jetty 9.4 client + server, existing behavior
org.jupnp.transport.jetty12 new, optional Jetty 12 client + server for Java 17+, servlet-free
org.jupnp.transport.javax.servlet new generic javax.servlet transport infra, used by jetty9 and Android
org.jupnp.transport.javax.httpservice new, optional classic OSGi HttpService port-sharing bridge
org.jupnp.transport.jakarta.servlet new, optional generic jakarta.servlet 6.1 transport infra, async-only
org.jupnp.transport.jakarta.whiteboard new, optional OSGi HTTP Whiteboard for Jakarta Servlet port-sharing bridge

Core (org.jupnp) no longer has any compile-time or OSGi-manifest dependency on javax.servlet, jakarta.servlet, org.osgi.service.http, or org.osgi.service.servlet -- not even an optional one. It talks to transports through TransportConfiguration and to an optional shared HTTP server through a new SharedStreamServerProvider SPI; neither interface mentions servlet or HttpService/Whiteboard types.

Discovery is fail-fast on ambiguity in both the ServiceLoader path (non-OSGi) and the Declarative Services path (OSGi), so a misconfigured classpath/bundle-set with two transport bundles -- or two shared-server providers -- throws a clear error instead of picking one non-deterministically. Which transport to use is always an explicit choice the consumer makes: org.jupnp.bom manages versions for org.jupnp.transport.jetty9 and org.jupnp.transport.jetty12 side by side, but importing it doesn't add either (or any other artifact) as a dependency by itself -- the consumer still declares exactly the one they want. Port sharing (via SharedStreamServerProvider) is a separate, orthogonal choice from which transport is in use -- org.jupnp.transport.jetty12 plus org.jupnp.transport.jakarta.whiteboard shares a port with a Jakarta HTTP Whiteboard runtime (e.g. Pax Web 11+) the same way org.jupnp.transport.jetty9 plus org.jupnp.transport.javax.httpservice shares a port with a classic HttpService.

Along the way, dependencies exercised by the new itests got a pass too: Jetty 9 (9.4.53 -> 9.4.58) and its matching Pax Web 8.x (8.0.25 -> 8.0.36), Jetty 12 (12.1.10 -> 12.1.12) and Pax Web 11.x (11.1.2 -> 11.1.3), Pax Logging (-> 2.3.5), logback (1.5.35 -> 1.6.2), and slf4j (2.0.11 -> 2.0.18).

Compatibility

  • Source/binary compatible for consumers going through TransportConfiguration/UpnpServiceConfiguration -- with one exception: subclasses of OSGiUpnpServiceConfiguration. No API changes to StreamServer, StreamClient, Router, or the discovery entry points. OSGiUpnpServiceConfiguration itself moved from manually binding HttpService to Declarative Services, which removes or changes several protected/public extension points a subclass may have relied on: the protected BundleContext context, protected TransportConfiguration transportConfiguration, and protected HttpService httpService fields are gone, as are the activate(BundleContext, Map<String, Object>), setHttpService(HttpService), and unsetHttpService(HttpService) methods (activate(Map<String, Object>), without the BundleContext parameter, replaces the old activate override point). A subclass no longer binds HttpService itself at all -- install org.jupnp.transport.javax.httpservice or org.jupnp.transport.jakarta.whiteboard (steps 3/4 below) instead, and DS injects a SharedStreamServerProvider automatically. See step 6 below.
  • Not binary compatible for consumers referencing servlet/HttpService classes directly. Anyone who imported org.jupnp.transport.impl.ServletStreamServerImpl, ServletContainerAdapter, HttpServiceServletContainerAdapter, etc. from org.jupnp will get compile errors -- those classes moved to new packages in the new bundles (see migration steps). This also covers org.jupnp.http.RequestInfo's HttpServletRequest-based methods -- reportRequest, reportParameters, reportHeaders, reportCookies, reportClient, dumpRequestHeaders, dumpRequestString, getRequestInfoString, getRequestFullURL, and the HttpServletRequest-typed overloads of isPS3Request/isJRiverRequest/isXbox360Request/isXbox360AlbumArtRequest -- which moved unchanged to the new org.jupnp.transport.javax.servlet.ServletRequestInfo in org.jupnp.transport.javax.servlet (step 3 below already covers adding that dependency). RequestInfo stays in core with only its String-based overloads (isPS3Request(String, String), isAndroidBubbleUPnPRequest, isJRiverRequest(String), isWMPRequest, isXbox360Request(String, String)) -- nothing was dropped, it was only split by whether it needs javax.servlet.
  • OSGi manifests changed. org.jupnp's Import-Package no longer lists javax.servlet, javax.servlet.http, org.osgi.service.http, jakarta.servlet, or org.osgi.service.servlet at all (previously optional for the javax/HttpService ones, now absent everywhere). Runtimes that don't have those packages will now resolve org.jupnp exactly as before (it never needed them to resolve); runtimes that do have them no longer get core wired to them implicitly.
  • Jetty 9 implementation classes renamed for symmetry with Jetty 12. org.jupnp.transport.impl.jetty.JettyTransportConfiguration -> org.jupnp.transport.jetty9.Jetty9TransportConfiguration (same package/name change for JettyStreamClientImpl -> Jetty9StreamClientImpl, JettyServletContainer -> Jetty9ServletContainer, and StreamClientConfigurationImpl -> Jetty9StreamClientConfigurationImpl). Only relevant if you referenced these directly (e.g. a custom createStreamClient()/createStreamServer() override, per the docs); consumers going through TransportConfiguration/UpnpServiceConfiguration are unaffected.

Steps for existing consumers of the Jetty 9 implementation

  1. org.jupnp.bom is now a conventional Maven BOM (previously a convenience aggregator that pulled in org.jupnp/org.jupnp.osgi/org.jupnp.support transitively). Import it via dependencyManagement/scope=import, then declare exactly the jUPnP artifacts you want as plain dependencies with no <version> (every example below omits it accordingly):
    <dependencyManagement>
      <dependencies>
        <dependency>
          <groupId>org.jupnp</groupId>
          <artifactId>org.jupnp.bom</artifactId>
          <version>...</version>
          <type>pom</type>
          <scope>import</scope>
        </dependency>
      </dependencies>
    </dependencyManagement>
    <dependencies>
      <dependency>
        <groupId>org.jupnp</groupId>
        <artifactId>org.jupnp</artifactId>
      </dependency>
      <dependency>
        <groupId>org.jupnp</groupId>
        <artifactId>org.jupnp.transport.jetty9</artifactId>
      </dependency>
    </dependencies>
    This is the one to not miss: importing the BOM no longer adds org.jupnp core (or a transport) to your classpath by itself -- list every artifact you need explicitly, same as if you weren't using the BOM at all (you can also skip the BOM entirely and pin each artifact's <version> directly, as before). Skipping the transport dependency doesn't break your build, it compiles fine and then fails at runtime with no transport found; skipping org.jupnp itself is a compile error, so that one's harder to miss. Nothing is auto-selected based on your build's JDK version.
  2. If you reference servlet transport classes directly (e.g. a custom UpnpServiceConfiguration.createStreamServer() override using ServletStreamServerImpl/ServletContainerAdapter, per the docs), add the new servlet dependency:
    <dependency>
      <groupId>org.jupnp</groupId>
      <artifactId>org.jupnp.transport.javax.servlet</artifactId>
    </dependency>
    and update the import: org.jupnp.transport.impl.ServletStreamServerImpl -> org.jupnp.transport.javax.servlet.ServletStreamServerImpl (same rename for ServletStreamServerConfigurationImpl, ServletContainerAdapter, ServletUpnpStream, ServletConnection).
  3. If you rely on the OSGi HttpService port-sharing path (deploying OSGiUpnpServiceConfiguration where a classic HttpService is present and you want jUPnP to share its port), add the new HttpService bridge bundle to your runtime:
    <dependency>
      <groupId>org.jupnp</groupId>
      <artifactId>org.jupnp.transport.javax.httpservice</artifactId>
    </dependency>
    No code changes needed for this path -- it's picked up automatically via Declarative Services once installed. If you don't install it, OSGiUpnpServiceConfiguration activates anyway and falls through to the transport's own native StreamServer on its own port -- unlike before this PR, where a missing HttpService simply meant the component never activated. If that fallback port collides with an already-running server (e.g. openHAB/openhab-addons commonly do, since their web UI's HttpService and jUPnP's default streamListenPort are both 8080), stream server init fails with a BindException and the whole router stays disabled. See openhab/openhab-core#5740 for a worked example of the required consumer-side change.
  4. If your runtime uses the OSGi HTTP Whiteboard Service for Jakarta Servlet instead of the classic HttpService (e.g. Pax Web 10+, which dropped HttpService; this is the runtime you'll have if you've migrated to Jetty 12/Jakarta EE 10), use the jakarta bridge instead -- pair it with org.jupnp.transport.jetty12:
    <dependency>
      <groupId>org.jupnp</groupId>
      <artifactId>org.jupnp.transport.jetty12</artifactId>
    </dependency>
    <dependency>
      <groupId>org.jupnp</groupId>
      <artifactId>org.jupnp.transport.jakarta.whiteboard</artifactId>
    </dependency>
    Same activation model as step 3: picked up automatically via Declarative Services once installed, no code changes needed; if not installed, OSGiUpnpServiceConfiguration falls through to jetty12's own standalone port.
  5. Android is already updated in this PR (depends on org.jupnp.transport.jetty9 and org.jupnp.transport.javax.servlet); no action needed for AndroidUpnpServiceConfiguration consumers.
  6. If you subclass OSGiUpnpServiceConfiguration: drop any override of setHttpService/unsetHttpService/the removed httpService/context fields -- port sharing is now handled entirely by installing a bridge bundle (steps 3/4), with no code on your side. If you overrode activate(BundleContext, Map<String, Object>), override activate(Map<String, Object>) instead (the BundleContext parameter is gone, since this class no longer needs it directly).

This PR was generated with significant use of AI.

@wborn

wborn commented Jul 22, 2026

Copy link
Copy Markdown
Member

Great I'll have a look! I was also thinking about giving it a go with AI soon. I'll use them to review instead now. 😄

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR modularizes jUPnP’s network transport layer so that transport implementations live in separate artifacts, enabling Jetty 9.4 and (on Java 17+) Jetty 12 to coexist while keeping the project’s baseline Java release at 11.

Changes:

  • Introduces TransportConfigurationProvider transport discovery via ServiceLoader, with a reflective fallback for environments like OSGi.
  • Splits transports into new modules/artifacts: org.jupnp.transport.jetty9 (Servlet-based) and org.jupnp.transport.jetty12 (Jetty core API, Java 17+).
  • Updates build/BOM/docs/CI to account for transport modularization and to add an additional Java 17 CI build.

Reviewed changes

Copilot reviewed 30 out of 34 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
website/docs/getting-started.md Updates getting-started dependency guidance to mention separate transport artifacts.
website/docs/android.md Documents Android requirement for Jetty 9 transport and adds Maven dependency example.
website/docs/advanced.md Updates transport documentation to describe modular transports and new implementation classes.
tools/org.jupnp.tool/pom.xml Adds an explicit dependency on the Jetty 9 transport module.
pom.xml Introduces a separate jetty12.version property while keeping Java 11 baseline.
itests/org.jupnp.osgi.tests/pom.xml Adds Jetty 9 transport dependency for OSGi integration tests.
itests/org.jupnp.osgi.tests/itest.bndrun Adds org.jupnp.transport.jetty9 bundle to the OSGi test runtime.
bundles/pom.xml Adds Jetty 9 transport module and activates Jetty 12 transport module only on JDK 17+.
bundles/org.jupnp/src/main/java/org/jupnp/transport/TransportConfigurationProvider.java Replaces hard-wired Jetty transport selection with discovery logic.
bundles/org.jupnp/src/main/java/org/jupnp/transport/impl/StreamClientConfigurationImpl.java Moves “default” stream client configuration to a transport-independent package.
bundles/org.jupnp/src/main/java/org/jupnp/OSGiUpnpServiceConfiguration.java Lazily discovers the transport implementation and adjusts optional HttpService handling.
bundles/org.jupnp/src/main/java/org/jupnp/DefaultUpnpServiceConfiguration.java Lazily discovers the transport implementation instead of constructing it directly.
bundles/org.jupnp/bnd.bnd Adjusts OSGi import strategy for transport implementation packages.
bundles/org.jupnp.transport.jetty9/src/test/java/org/jupnp/transport/JettyServerJettyClientTest.java Updates tests to use the relocated transport-independent client configuration class.
bundles/org.jupnp.transport.jetty9/src/main/resources/META-INF/services/org.jupnp.transport.TransportConfiguration Registers Jetty 9 transport via ServiceLoader.
bundles/org.jupnp.transport.jetty9/src/main/java/org/jupnp/transport/impl/jetty/StreamClientConfigurationImpl.java Adds a compatibility wrapper for the old Jetty-specific configuration class name.
bundles/org.jupnp.transport.jetty9/src/main/java/org/jupnp/transport/impl/jetty/JettyTransportConfiguration.java Defines the Jetty 9 transport implementation entrypoint.
bundles/org.jupnp.transport.jetty9/src/main/java/org/jupnp/transport/impl/jetty/JettyStreamClientImpl.java Adds the Jetty 9 HTTP client implementation.
bundles/org.jupnp.transport.jetty9/src/main/java/org/jupnp/transport/impl/jetty/JettyServletContainer.java Adds a standalone Jetty 9 servlet container adapter.
bundles/org.jupnp.transport.jetty9/src/main/java/org/jupnp/transport/impl/jetty/HeaderUtil.java Adds Jetty 9 header/content conversion helpers.
bundles/org.jupnp.transport.jetty9/pom.xml Introduces the Jetty 9 transport module build configuration.
bundles/org.jupnp.transport.jetty12/src/test/java/org/jupnp/transport/Jetty12ServerJetty12ClientTest.java Adds Jetty 12 transport integration test coverage.
bundles/org.jupnp.transport.jetty12/src/main/resources/META-INF/services/org.jupnp.transport.TransportConfiguration Registers Jetty 12 transport via ServiceLoader.
bundles/org.jupnp.transport.jetty12/src/main/java/org/jupnp/transport/impl/jetty12/Jetty12UpnpStream.java Adds Jetty 12 server-side request/response bridging to UPnP streams.
bundles/org.jupnp.transport.jetty12/src/main/java/org/jupnp/transport/impl/jetty12/Jetty12TransportConfiguration.java Defines the Jetty 12 transport implementation entrypoint.
bundles/org.jupnp.transport.jetty12/src/main/java/org/jupnp/transport/impl/jetty12/Jetty12StreamServerImpl.java Adds a Jetty 12 core-API stream server (non-servlet).
bundles/org.jupnp.transport.jetty12/src/main/java/org/jupnp/transport/impl/jetty12/Jetty12StreamServerConfigurationImpl.java Adds Jetty 12 server configuration implementation.
bundles/org.jupnp.transport.jetty12/src/main/java/org/jupnp/transport/impl/jetty12/Jetty12StreamClientImpl.java Adds the Jetty 12 HTTP client implementation.
bundles/org.jupnp.transport.jetty12/src/main/java/org/jupnp/transport/impl/jetty12/HeaderUtil.java Adds Jetty 12 header conversion helpers.
bundles/org.jupnp.transport.jetty12/pom.xml Introduces the Jetty 12 transport module build configuration (Java 17 release).
bundles/org.jupnp.android/pom.xml Ensures Android bundle depends on the Jetty 9 transport module.
bom/jupnp/pom.xml Adds transport artifacts to the BOM, with Jetty 12 included only on JDK 17+.
bom/compile/pom.xml Removes Jetty dependencies from the compile BOM since they move to transport modules.
.github/workflows/ci-build.yml Adds an extra Java 17 CI build to cover the optional Jetty 12 transport.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread website/docs/getting-started.md Outdated
Comment thread website/docs/advanced.md Outdated
@wborn

wborn commented Jul 22, 2026 •

Copy link
Copy Markdown
Member

@kaikreuzer perhaps you can enable the "ChatGPT Codex Connector" for this repo so I can also ask Codex for a review in this repo as well.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 31 out of 34 changed files in this pull request and generated 2 comments.

Comment thread website/docs/advanced.md
@wborn wborn linked an issue Jul 23, 2026 that may be closed by this pull request
@holgerfriedrich

Copy link
Copy Markdown
Author

@wborn I pushed the AI to get this done, and it turned out there really was a bug in the ServiceLoader-based discovery: the code called the two-argument ServiceLoader.load(Class, ClassLoader) overload, which silently bypasses SPI Fly's weaving (SPI Fly only intercepts the one-argument overload). Fixed that.

Getting a reliable itest was still complicated: the transport bundle only registers itself with SPI Fly after jUPnP's own bundle already attempts discovery, so the two race, and a short startup retry doesn't reliably close that gap. Pinning bundle start levels in the bnd file did work, but isn't a good way — bnd's resolver silently drops those attributes every time it recomputes -runbundles (e.g. -DwithResolver), which is a routine dev step.

So instead, OSGiUpnpServiceConfiguration now consumes the transport via a mandatory Declarative Services @reference instead of through TransportConfigurationProvider. DS won't activate the component until the transport bundle has actually registered, so it's deterministic rather than a race. The ServiceLoader/SPI Fly path is kept for non-DS consumers (e.g. plain DefaultUpnpServiceConfiguration use).

WDYT?

@holgerfriedrich
holgerfriedrich force-pushed the modularize-transport branch 2 times, most recently from ba06f23 to 1966817 Compare July 23, 2026 20:23
@holgerfriedrich
holgerfriedrich marked this pull request as draft July 23, 2026 22:45
@holgerfriedrich

Copy link
Copy Markdown
Author

Back to draft, last commit needs some rework...

@wborn

wborn commented Jul 26, 2026 •

Copy link
Copy Markdown
Member

After having a first look the changes are quite straightforward and seem pretty nice overall. 👍
Have to look at it a bit more in depth and meet up with some AI friends to discuss it. 😉

@wborn

wborn commented Jul 26, 2026

Copy link
Copy Markdown
Member

Is it also possible to move the classes using javax.servlet to the jetty 9 transport or will something break?

@holgerfriedrich

Copy link
Copy Markdown
Author

Is it also possible to move the classes using javax.servlet to the jetty 9 transport or will something break?

This is likely a change bigger than this one. Everything still relays on javax.servlet - which will be still pulled in even if we use the jetty 12 transport!

We can try to modularize the the code around HttpServlet to achive javax for jetty9 and jakarta for jetty12.

I will give this a try - the transport modularization itself is only half of the story.

@holgerfriedrich

Copy link
Copy Markdown
Author

@wborn If I see it correctly, we use the fallback with a separate StreamServer in OH - in this case, we could relay on this fallback and make the servlet optional.
This could be a way to get rid of javax servlet and get a working integration without too much effort.

See last commit. I will give it a try with OH patch.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 41 out of 43 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

bundles/org.jupnp/src/main/java/org/jupnp/transport/TransportConfigurationProvider.java:87

  • The thrown message says "No transport implementation found", but discovery can also fail when a provider is present yet can’t be loaded (e.g. linkage/config errors). Adjusting the message to "No usable transport implementation" makes it accurate for both cases.
        throw new InitializationException("No transport implementation found via ServiceLoader. "
                + "Add a jUPnP transport bundle (e.g. org.jupnp:org.jupnp.transport.jetty9) as a dependency. "
                + "In OSGi, also ensure an OSGi Service Loader Mediator (e.g. Apache Aries SPI Fly) is "
                + "installed and started.");

Comment thread itests/org.jupnp.osgi.tests/pom.xml
Comment thread itests/org.jupnp.osgi.tests.jetty12/pom.xml

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 41 out of 43 changed files in this pull request and generated 1 comment.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 41 out of 43 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

bundles/org.jupnp/src/main/java/org/jupnp/transport/TransportConfigurationProvider.java:128

  • The PR description says an application can depend on Jetty 9 and Jetty 12 transports "(or both)", but TransportConfigurationProvider hard-fails when more than one ServiceLoader provider is present. Either update the stated behavior, or add a deterministic selection mechanism (e.g., system property / service ranking) so "both present" is actually supported.
        // ServiceLoader iteration order is unspecified, so silently picking the first of several
        // candidates would make transport selection non-deterministic across runs. Fail loudly instead,
        // the same way the no-provider-found case does, rather than risk a random pick.
        if (foundClassNames.size() > 1) {
            throw new InitializationException("Multiple transport implementations found via ServiceLoader: "
                    + foundClassNames + ". Make sure only one jUPnP transport bundle (e.g. "
                    + "org.jupnp.transport.jetty9 or org.jupnp.transport.jetty12) is on the class path.");
        }

@holgerfriedrich
holgerfriedrich requested a review from Copilot July 27, 2026 15:22
holgerfriedrich and others added 5 commits August 26, 2026 18:27
* Upgrade jetty9 from 9.4.53 to 9.4.58
* Upgrade pax-web8 from 8.0.25 to 8.0.36
* Upgrade jetty12 to 12.1.12
* Upgrade pax-web11 to 11.1.3
* Upgrade pax-logging to 2.3.5
* Upgrade slf4j from 2.0.11 to 2.0.18
* Upgrade logback from 1.5.35 to 1.6.2

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
Manage all public jUPnP artifacts, including both transports and the
servlet/HttpService bridges, in dependencyManagement instead of listing a
subset as plain dependencies, so importing it with scope=import actually
works the standard way. Consumers now declare exactly the artifacts they
want, with no version needed on any of them. This also removes the need
for the org.jupnp.android/jetty9 exclusion, since dependencyManagement
entries don't add anything to the classpath by themselves.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
Align artifact, bundle/module, and Java package names across all six
transport bundles before they become part of the 4.0 API surface:

- org.jupnp.transport.servlet -> org.jupnp.transport.javax.servlet
  (package org.jupnp.transport.impl.servlet -> org.jupnp.transport.javax.servlet)
- org.jupnp.transport.httpservice -> org.jupnp.transport.javax.httpservice
  (package org.jupnp.transport.impl.osgi -> org.jupnp.transport.javax.httpservice)
- org.jupnp.transport.jakarta.servlet: artifact unchanged, package
  org.jupnp.transport.impl.servlet.jakarta -> org.jupnp.transport.jakarta.servlet
- org.jupnp.transport.jakarta.httpservice: artifact unchanged (kept, not
  renamed to "whiteboard", to preserve symmetry with javax.httpservice),
  package org.jupnp.transport.impl.osgi.jakarta -> org.jupnp.transport.jakarta.httpservice
- org.jupnp.transport.jetty12: artifact unchanged, package
  org.jupnp.transport.impl.jetty12 -> org.jupnp.transport.jetty12
- org.jupnp.transport.jetty9: artifact unchanged, package
  org.jupnp.transport.impl.jetty -> org.jupnp.transport.jetty9, and its
  classes renamed for symmetry with Jetty 12's Jetty12* prefix:
  JettyTransportConfiguration -> Jetty9TransportConfiguration,
  JettyStreamClientImpl -> Jetty9StreamClientImpl,
  JettyServletContainer -> Jetty9ServletContainer,
  StreamClientConfigurationImpl -> Jetty9StreamClientConfigurationImpl

Also renames the itest module org.jupnp.osgi.tests to
org.jupnp.osgi.tests.jetty9 (it specifically exercises Jetty 9 now that
jetty12/jetty12.jakarta are separate modules), sorts bundles/pom.xml's
module lists, updates bom/jupnp/pom.xml's managed artifacts to match, and
regenerates every itest's -runbundles via -DwithResolver. Adds the two
newer itest modules (org.jupnp.osgi.tests.jetty12 and
org.jupnp.osgi.tests.jetty12.jakarta) to the Central-publishing
excludeArtifacts list, which had only ever listed the original module --
a pre-existing gap unrelated to this rename, found while regenerating that
list's entries.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
…ace in its itest

The Jakarta bridge implements the OSGi Servlet Whiteboard, not a Jakarta
port of the classic HttpService, so name it accordingly and free up
javax.whiteboard for a possible future javax-based Whiteboard bridge:

- org.jupnp.transport.jakarta.httpservice -> org.jupnp.transport.jakarta.whiteboard
  (package org.jupnp.transport.jakarta.httpservice -> org.jupnp.transport.jakarta.whiteboard)
- JakartaHttpServiceServletContainerAdapter -> JakartaWhiteboardServletContainerAdapter
- JakartaHttpServiceSharedStreamServerProvider -> JakartaWhiteboardSharedStreamServerProvider
- itest module org.jupnp.osgi.tests.jetty12.jakarta -> org.jupnp.osgi.tests.jetty12.jakarta.whiteboard,
  matching what it actually exercises (Jetty 12 + the Jakarta Whiteboard bridge)

Updates every cross-reference: bundles/pom.xml and bom/jupnp/pom.xml
module/dependency lists (re-sorted), itests/pom.xml, root pom.xml's
Central-publishing excludeArtifacts (which still had the itest's old
name), and regenerates -runbundles via -DwithResolver.

Also fixes the DS lifecycle race behind the intermittent Java 17 CI
failure in that itest's RealSharedServerActivationTest: enabling
OSGiUpnpServiceConfiguration before the Jakarta Whiteboard bridge's
SharedStreamServerProvider was available let the component activate
without it, then reactivate once the provider showed up --
UpnpServiceImpl.unsetUpnpServiceConfiguration() briefly nulls the
configuration mid-rebind, which a UpnpService reference obtained just
before that reactivation could observe as null. Waiting for
SharedStreamServerProvider first means activation always happens with
the provider already bound, so no later reactivation. Verified with
repeated itest runs (previously intermittent, now consistently green).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
@holgerfriedrich

Copy link
Copy Markdown
Author

I rebased to resolve recent merge conflicts.
@wborn Anything left I should do here? How do you want to proceed to get a 4.0 snapshot or rc published?

holgerfriedrich and others added 3 commits August 26, 2026 18:59
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
Felix SCR 2.2.10 (Jan 2024) never activates delayed Declarative
Services components under Java 25/26: bind methods and constructors
are simply never invoked, even though all mandatory @reference
dependencies are registered and available the whole time. This broke
all three OSGi itests (jetty9, jetty12, jetty12.jakarta.whiteboard) on
Java 25 and 26 in CI, while Java 11/17/21 were unaffected.

Confirmed via direct reproduction: instrumenting the DS component's
constructor and bind method showed both firing correctly under Java
21 but never firing under Java 25, with an otherwise identical
bndrun/classpath. Bumping org.eclipse.osgi (the OSGi framework itself)
alone did not fix it; the fix is specifically in Felix SCR.

org.apache.felix.scr 2.2.18 requires org.osgi.util.promise/function
>=1.1.0, but org.osgi.service.component (1.5.0/1.5.1) still
transitively pulls 1.0.0, so both are pinned explicitly to win over
that transitive resolution. The three affected itest.bndrun files were
regenerated via -DwithResolver.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
The felix.scr bump alone did not fully fix the OSGi itests on Java
25/26: they still failed intermittently in CI and in a full local
reactor build, even though a repeated isolated single-module build
kept passing. The real, separate cause: org.apache.aries.spifly's
WeavingHook uses a bundled ASM 9.6, whose class file parser doesn't
recognize Java 25/26 (major versions 69/70). It throws
IllegalArgumentException whenever it processes a JVM-synthesized
lambda proxy class, which is generated with the running JVM's own
class file version regardless of --release used at compile time. That
throw is caught and logged as a FrameworkEvent.ERROR rather than
crashing, but depending on exactly when in the bundle startup sequence
it hits (a genuinely racy timing), it can corrupt spifly's
TransportConfiguration provider registration for that run, leaving the
DS component that depends on it permanently unsatisfied.

Pinned the whole ASM family (asm, asm-commons, asm-tree, asm-analysis,
asm-util) to 9.10.1 in itests/pom.xml's dependencyManagement, inherited
by all three OSGi itest modules, overriding the transitive 9.6 pulled
in by aries-spifly-dynamic-bundle:1.3.7 (the latest available spifly
release, which itself has not been updated for newer JDKs). Verified
with 5 consecutive clean runs of jetty9 and 3 of jetty12/jetty12.jakarta.whiteboard
under Java 25 (previously flaky), a full `clean verify` reactor build
under Java 25, and re-verified Java 21 still passes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>

@wborn wborn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for addressing all the comments. 👍 After another pass it all looks very nice. Here are a few minor comments I found along the way. I also still want to give it a small test.

Comment thread bom/jupnp/pom.xml
Comment thread website/docs/advanced.md
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
@holgerfriedrich

Copy link
Copy Markdown
Author

I also still want to give it a small test.

Thanks, @wborn.
In case I should rebase the PR for easier testing, pls. let me know.

@holgerfriedrich

Copy link
Copy Markdown
Author

@wborn did you find some time to test this a bit? We could test the new modularization on Jetty9/Karaf 4.4.12 in the next OH milestone....

@lsiepel

lsiepel commented Oct 11, 2026

Copy link
Copy Markdown

Maybe @kaikreuzer ? Would be good to have this prior to the big karaf upgrade in openHAB.

@kaikreuzer
kaikreuzer requested a review from wborn October 13, 2026 21:15
@kaikreuzer

Copy link
Copy Markdown
Member

I would leave it with @wborn unless he states that he won't be able to do it and ask me instead.

@wborn

wborn commented Oct 14, 2026

Copy link
Copy Markdown
Member

Yes, it's still on my radar. I'm hoping to find some time to test it this coming weekend. 🙂

During a Karaf bundle refresh (e.g. installing an openHAB add-on that
pulls in GraalVM), jUPnP was activated on the refresh thread before Pax
Web was restarted. JakartaWhiteboardServletContainerAdapter then polled
for up to 60s for HttpServiceRuntime's endpoint on that same thread, so
the endpoint could never appear: the refresh stalled for the full
timeout and jUPnP ended up without a stream server.

- Gate JakartaWhiteboardSharedStreamServerProvider on an
  HttpServiceRuntime advertising an http endpoint, so DS (re)starts
  jUPnP on the shared server once Pax Web is actually up.
- Read the port once in addConnector(), using the same filter, instead
  of polling.
- Confirm the Whiteboard picked up the servlet on a background thread,
  cancelled by stopIfRunning(), instead of blocking registerServlet().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>

@wborn wborn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I gave it a test today and it works great. 🙂
Many thanks! 👍

As the PR also updates the website docs, I've created #416 so we can show the 3.x docs and further update the 4.x docs before it is released.

@holgerfriedrich

Copy link
Copy Markdown
Author

Signed-off-by: Wouter Born <github@maindrain.net>
wborn added a commit to wborn/jupnp that referenced this pull request Oct 17, 2026
Preserve the existing documentation as a 3.x snapshot before jupnp#365 updates the main docs for 4.x.

Keep 3.x as the default and expose 4.x through a version selector.
Enable versioned search and make edit links target the correct docs.

Signed-off-by: Wouter Born <github@maindrain.net>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support newer Jetty versions

5 participants