Rework PluginRegistry API and introduce VersionMatchRule enum #1163
Conversation
HannesWell
force-pushed
the
version-cleanUps
branch
from
10be7e4
to
6e29ee0
Compare
Test Results 273 files - 16 273 suites - 16 33m 37s ⏱️ - 22m 13s For more details on these errors, see this check. Results for commit f6d15ec. ± Comparison against base commit 689759d. This pull request removes 2627 tests.♻️ This comment has been updated with latest results. |
HannesWell
force-pushed
the
version-cleanUps
branch
from
6e29ee0
to
35a14aa
Compare
HannesWell
changed the title
HannesWell
force-pushed
the
version-cleanUps
branch
from
35a14aa
to
fead832
Compare
|
This PR now focus only on the rework of the In the meantime I made greater changes to the The rework is not yet completely finished, but the overall direction is settled and I would like to gather feedback already. |
| */ | ||
| @Deprecated(forRemoval = true, since = "3.19 (removal in 2026-09 or later)") |
There was a problem hiding this comment.
Btw. what do you think about mentioning the earliest removal date here?
I think it would help consumers to understand better when the actual removal will happen earliest.
| public static Stream<IPluginModelBase> findModels(String id, String version, VersionMatchRule matchRule) { | ||
| Version reference = version != null ? Version.valueOf(version) : null; | ||
| return selectModels(id, version != null ? p -> matchRule.matches(VersionUtil.getVersion(p), reference) : null); | ||
| } |
There was a problem hiding this comment.
Instead of having one overload of findModel(s) that accepts a VersionRange and one that accepts a version String plus a VersionMatchRule we could also just provide one method with a VersionRange.
With the new VersionMatchRule one would then use
findModel("foo.bar", EQUIVALENT.rangeFor(version))
instead of
findModel("foo.bar", versionStr, EQUIVALENT)
Of course the level of convenience is also influenced from if one has a Version object or a version String.
Maybe this justifies to keep both so in case one has a Version object the former solution can be used and if one has a version String the latter approach is nicer.
I'm torn on this question.
Besides replacing the Equinox resolver VersionRange by the osgi VersionRange, this also remove the rarely used filter parameter from the findModel(s)() methods. Instead of a List the findModels() methods now return a stream that can also be filtered if desired. In all cases plug-ins with null IPluginBase or ID are now discarded. Part of eclipse-pde#1069
HannesWell
force-pushed
the
version-cleanUps
branch
from
fead832
to
f6d15ec
Compare
Modernize the PluginRegistry API that contains the antiquated
org.eclipse.osgi.service.resolver.VersionRangeby providing alternatives using theorg.osgi.framework.VersionRangeand deprecating the existing methods for removal.In conjunction with that also mark
PluginRegistry.PluginFilteras deprecated for removal since it is effectively aPredicate<IPluginModelBase>and just use the latter in the new methods.This is currently an early draft to discuss the overall direction. Since the intention is to focus on the API callers of the now deprecated methods are not replaced in this PR. I'll do that in a follow-up.
About the API I'm also contemplating to further modernize the methods in
PluginRegistrythat now return a List to return a Stream instead. Then we could also save the filter arguments since callers can just filter the returned stream if desired.Furthermore I would also like to replace the integer and string constants in
IMatchRulesby a corresponding EnumVersionMatchRules(or similar) that also provides method to obtain a predicate for a reference version that implements the literals matching rule or to obtain a correspondingVersionRangefrom a reference version (which ever is needed).Part of #1069