ideas-into-software commented on PR #218: URL: https://github.com/apache/felix-dev/pull/218#issuecomment-1672266266
@cziegeler As per your request, I switched this PR to https://github.com/apache/felix-dev/tree/jakarta-servlet-6 branch you created. I also took care of verifying upgraded artifacts (Apache Felix HTTP, Apache Felix Inventory as well as Apache Felix Web Console and plugins) as part of an actual project. The tests apparently do not have sufficient coverage, because even though all tests passed (see my earlier comment, https://github.com/apache/felix-dev/pull/218#issue-1828103395, for more info about tests and the `org.apache.felix.webconsole.plugins.scriptconsole` exception), there were multiple issues which came up when integrating these artifacts into actual project. All those issues were fixed and that’s what those latest commits in this PR address, i.e. fixes for issues with Felix Web Console, Web Console plugins & related, including propagation of custom configuration, missing Whiteboard service parameters resulting in plugins not being registered properly, path resolution issues, etc. As mentioned, those cases are not covered by any of the existing tests and were discovered during exploratory testing as part of an actual project. That project is https://github.com/geckoprojects-org/org.gecko.graphql and in this PR (and its commits) you will find more details: https://github.com/geckoprojects-org/org.gecko.graphql/pull/8 Just in case this is not clear (and I do not mean you, @cziegeler): since there are multiple artifacts from 3 different projects (Apache Felix being one of those 3), such must be first build and installed into local Maven repository before being used in a project, until they’re officially released, i.e. : - `org.apache.commons:commons-fileupload2-core:2.0.0-M3-SNAPSHOT` and `org.apache.commons:commons-fileupload2-jakarta-servlet6:2.0.0-M3-SNAPSHOT` artifacts – until they’re released, you can build and install those into your local Maven repository from https://github.com/ideas-into-software/commons-fileupload/tree/osgi-enhancements branch - `org.osgi:org.osgi.service.servlet:3.0.0-SNAPSHOT` – until it’s released, build and install it from https://github.com/ideas-into-software/osgi/tree/requirements/597 branch - upgraded Apache Felix artifacts – until it’s released, build and install it from https://github.com/DataInMotion/felix-dev/tree/jakarta-servlet-6-x branch (soon https://github.com/apache/felix-dev/tree/jakarta-servlet-6 branch) I am not sure when you or Felix team will conduct review of code in this PR, but I would like to ask for clarification regarding creation of pattern matcher dependent on service ID of handler used (previously `HttpServiceFactory.HTTP_SERVICE_CONTEXT_SERVICE_ID`) – prior to this upgrade/refactor, such was checked in `org.apache.felix.http.base.internal.registry.PathResolverFactory.createPatternMatcher(ServletHandler, String)` and instance of `org.apache.felix.http.base.internal.registry.PathResolverFactory.ExactAndPathMatcher.ExactAndPathMatcher(ServletHandler, String)` created; should such exceptional case still exist, e.g. `org.apache.felix.http.base.internal.registry.PathResolverFactory.ExactAndPathMatcher.ExactAndPathMatcher(ServletHandler, String)` pattern matcher created if given handler is for default context ? Also, some improvement suggestions: - add tests to cover cases which were only discovered during exploratory testing (and hence these new commits); - consistent registration of internal / external web console plugins – currently, those are registered in four different ways, including via `org.apache.felix.webconsole.SimpleWebConsolePlugin.register(BundleContext)`, as well as via their activators (`org.apache.felix.webconsole.plugins.event.internal.Activator`), constructors (`org.apache.felix.webconsole.plugins.scriptconsole.internal.ScriptConsolePlugin`, `org.apache.felix.inventory.impl.InventoryPrinterManagerImpl.InventoryPrinterManagerImpl(BundleContext)`) and specialized methods (`org.apache.felix.inventory.impl.InventoryPrinterAdapter.registerConsole(BundleContext, InventoryPrinterManagerImpl)`; - refactor Apache Felix Inventory Web Console plugin to Apache Felix Webconsole as internal plugin – currently, class hierarchy and constants are duplicated as to avoid cyclic dependencies, but this looks like a candidate for another internal Web Console plugin, instead of being maintained separately; I will appreciate providing improvement suggestions for any of the new / refactored code in this PR, where needed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@felix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
