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

Deprecate (and mark for removal) HttpContextExtensionService #437

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

laeubi
Copy link
Member

@laeubi laeubi commented Dec 15, 2023

The HttpContextExtensionService is effectively unused in platform and uses the abandoned HttpService it is API.

We therefore should deprecate and remove that so the org.eclipse.equinox.http.registry bundle becomes free from any API and only contains internal implementation what would make us free to change how it works,

The HttpContextExtensionService is effectively unused in platform and
uses the abandoned HttpService it is API.

We therefore should deprecate and remove that so the
org.eclipse.equinox.http.registry bundle becomes free from any API and
only contains internal implementation what would make us free to change
how it works,
Copy link

Test Results

     24 files  ±0       24 suites  ±0   11m 8s ⏱️ +29s
2 150 tests ±0  2 105 ✔️ ±0  45 💤 ±0  0 ±0 
2 194 runs  ±0  2 149 ✔️ ±0  45 💤 ±0  0 ±0 

Results for commit f354afe. ± Comparison against base commit bf62833.

@vogella
Copy link
Contributor

vogella commented Dec 15, 2023

+1

@tjwatson
Copy link
Contributor

Should check for usage from RAP @mknauer

Note this would also have to remove the extension point:

     <extension-point id="httpcontexts" name="%httpcontextsName" schema="schema/httpcontexts.exsd"/>

I find it odd that we deprecate this one extension point when all other extension points in org.eclipse.equinox.http.registry are also dependent on the old http service.

@laeubi
Copy link
Member Author

laeubi commented Dec 15, 2023

I find it odd that we deprecate this one extension point when all other extension points in org.eclipse.equinox.http.registry are also dependent on the old http service.

As long as the extension point do not depend on org.osgi.http package I don't think they depend on http service as the impl can choose any technique (including whiteboard http service)

@HannesWell
Copy link
Member

In general I fully support deprecating and eventually removing unused elements especially when they cause problems like o.o.service.http.
Nevertheless in this case I'm not familiar with the class to deprecate, but from looking at it I wonder why the only method of the interface HttpContextExtensionService.getHttpContext() is never called (at least Eclipse does not give me a caller)?

I find it odd that we deprecate this one extension point when all other extension points in org.eclipse.equinox.http.registry are also dependent on the old http service.

As long as the extension point do not depend on org.osgi.http package I don't think they depend on http service as the impl can choose any technique (including whiteboard http service)

The extension point contains

<meta.attribute kind="java" basedOn="org.osgi.service.http.HttpContext"/>

so I would say it depends on org.osgi.service.http.

And to give a broader dependencies overview, in the SimRel only RAP and WST depend on equinox.http.registry:

grafik

@laeubi
Copy link
Member Author

laeubi commented Dec 16, 2023

but from looking at it I wonder why the only method of the interface HttpContextExtensionService.getHttpContext() is never called (at least Eclipse does not give me a caller)?

That's what I written above:

The HttpContextExtensionService is effectively unused in platform

Regarding

The extension point contains

that's independent from this interface, that is never called and currently exposes API, so when we get rid of it, the whole bundle is only "internal" and we can decide how to probably deprecate the extension point.

@HannesWell
Copy link
Member

but from looking at it I wonder why the only method of the interface HttpContextExtensionService.getHttpContext() is never called (at least Eclipse does not give me a caller)?

That's what I written above:

The HttpContextExtensionService is effectively unused in platform

OK, but before deprecating an interface usually all references to it in our code-base should be removed.
@laeubi can you please do the corresponding changes/migrations in this PR so that when the interface is eventually removed nothing else has to be changed.

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.

4 participants