This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch karaf-4.4.x
in repository https://gitbox.apache.org/repos/asf/karaf.git
The following commit(s) were added to refs/heads/karaf-4.4.x by this push:
new c058cc3580 Fix state of features with a leading zero in a version
segment (#2993)
c058cc3580 is described below
commit c058cc35803ef796b8713de6cad7724e9eaa96d6
Author: JB Onofré <[email protected]>
AuthorDate: Thu Oct 15 13:30:25 2026 +0100
Fix state of features with a leading zero in a version segment (#2993)
A feature version such as 1.0.01 was used as-is in Feature.getId(), while
the deployment state keys features by the id of their resolver resource,
where the version is an OSGi Version and is therefore normalized to 1.0.1.
Because the two ids never matched, such a feature was reported as not
installed (UNINSTALLED) even though its bundles were running, feature
state changes were ignored, and its configurations and libraries were not
installed.
Normalize feature ids the OSGi way everywhere they are compared with the
deployment state: in FeaturesServiceImpl (isInstalled/getState) and in the
Deployer (features by id, requested state changes, configuration and
library installation). The persisted state already used the normalized
form, so existing installations need no migration.
Fixes KARAF-5784 and KARAF-7163.
(cherry picked from commit 0e2b7b03c86db62c873d4c21978adb845afab8d9)
Co-authored-by: ponziani <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
---
.../features/internal/resolver/ResourceUtils.java | 17 ++++++
.../karaf/features/internal/service/Deployer.java | 9 +--
.../internal/service/FeaturesServiceImpl.java | 4 +-
.../internal/service/FeaturesServiceImplTest.java | 68 ++++++++++++++++++++++
.../internal/service/leadingzero/features.xml | 25 ++++++++
5 files changed, 118 insertions(+), 5 deletions(-)
diff --git
a/features/core/src/main/java/org/apache/karaf/features/internal/resolver/ResourceUtils.java
b/features/core/src/main/java/org/apache/karaf/features/internal/resolver/ResourceUtils.java
index c8b5ab28d5..8929504eb1 100644
---
a/features/core/src/main/java/org/apache/karaf/features/internal/resolver/ResourceUtils.java
+++
b/features/core/src/main/java/org/apache/karaf/features/internal/resolver/ResourceUtils.java
@@ -91,6 +91,23 @@ public final class ResourceUtils {
return null;
}
+ /**
+ * Returns the feature ID in the same form as {@link
#getFeatureId(Resource)}, which is the form used as key
+ * in the deployment state. The version is normalized the OSGi way, so
<code>1.0.01</code> becomes
+ * <code>1.0.1</code>.
+ */
+ public static String getFeatureId(String name, String version) {
+ return version != null ? name + "/" + VersionTable.getVersion(version)
: name;
+ }
+
+ /**
+ * Normalizes a feature ID (name[/version]) to the form returned by {@link
#getFeatureId(Resource)}.
+ */
+ public static String normalizeFeatureId(String featureId) {
+ int idx = featureId.indexOf('/');
+ return idx < 0 ? featureId : getFeatureId(featureId.substring(0, idx),
featureId.substring(idx + 1));
+ }
+
public static RequirementImpl addIdentityRequirement(ResourceImpl
resource, String name, String type, String range) {
return addIdentityRequirement(resource, name, type, range, true);
}
diff --git
a/features/core/src/main/java/org/apache/karaf/features/internal/service/Deployer.java
b/features/core/src/main/java/org/apache/karaf/features/internal/service/Deployer.java
index 8ade7a76ff..9c5c71f6a5 100644
---
a/features/core/src/main/java/org/apache/karaf/features/internal/service/Deployer.java
+++
b/features/core/src/main/java/org/apache/karaf/features/internal/service/Deployer.java
@@ -230,7 +230,8 @@ public class Deployer {
featuresById = new HashMap<>();
for (Feature feature : featuresList) {
features.computeIfAbsent(feature.getName(), name -> new
ArrayList<>()).add(feature);
- featuresById.put(feature.getId(), feature);
+ // keyed like the deployment state, which uses normalized
versions (1.0.01 -> 1.0.1)
+ featuresById.put(getFeatureId(feature.getName(),
feature.getVersion()), feature);
}
}
}
@@ -427,7 +428,7 @@ public class Deployer {
Map<String, String> regionStates = stateFeatures.get(region);
if (regionStates != null) {
for (Map.Entry<String, FeatureState> entry2 :
entry1.getValue().entrySet()) {
- String feature = entry2.getKey();
+ String feature =
ResourceUtils.normalizeFeatureId(entry2.getKey());
if (regionStates.containsKey(feature)) {
regionStates.put(feature, entry2.getValue().name());
}
@@ -957,13 +958,13 @@ public class Deployer {
if (!newFeatures.isEmpty()) {
Set<String> featureIds = flatten(newFeatures);
for (Feature feature : dstate.featuresById.values()) {
- if (featureIds.contains(feature.getId())) {
+ if (featureIds.contains(getFeatureId(feature.getName(),
feature.getVersion()))) {
callback.installConfigs(feature);
callback.installLibraries(feature);
}
for (Conditional cond : feature.getConditional()) {
Feature condFeature = cond.asFeature();
- if (featureIds.contains(condFeature.getId())) {
+ if
(featureIds.contains(getFeatureId(condFeature.getName(),
condFeature.getVersion()))) {
callback.installConfigs(condFeature);
callback.installLibraries(condFeature);
}
diff --git
a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
index fed0f3413f..c4033b4513 100644
---
a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
+++
b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
@@ -69,6 +69,7 @@ import org.apache.karaf.features.internal.model.Features;
import org.apache.karaf.features.internal.model.JacksonUtil;
import org.apache.karaf.features.internal.model.JaxbUtil;
import org.apache.karaf.features.internal.region.DigraphHelper;
+import org.apache.karaf.features.internal.resolver.ResourceUtils;
import
org.apache.karaf.features.internal.service.BundleInstallSupport.FrameworkInfo;
import org.apache.karaf.util.ThreadUtils;
import org.apache.karaf.util.json.JsonReader;
@@ -955,7 +956,8 @@ public class FeaturesServiceImpl implements
FeaturesService, Deployer.DeployCall
}
String name = feature.substring(0, idx);
String version = feature.substring(idx + 1);
- return name + VERSION_SEPARATOR + VersionCleaner.clean(version);
+ // same form as the ids in the deployment state, so 1.0.01 matches
1.0.1
+ return ResourceUtils.getFeatureId(name, VersionCleaner.clean(version));
}
/**
diff --git
a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
index c351ad7a82..252c19b7aa 100644
---
a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
+++
b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
@@ -237,6 +237,74 @@ public class FeaturesServiceImplTest extends TestBase {
assertSame(bundle, stoppedBundle.getValue());
}
+ /**
+ * A feature version with a leading zero in a numeric segment (1.0.01)
must be reported as installed and started,
+ * and its state must be changeable using its own id (KARAF-5784,
KARAF-7163).
+ */
+ @Test
+ public void testInstallAndStopFeatureWithLeadingZeroInVersion() throws
Exception {
+ Capture<Bundle> stoppedBundle = Capture.newInstance();
+
+ Bundle bundle = EasyMock.niceMock(Bundle.class);
+ BundleStartLevel bundleStartLevel =
EasyMock.niceMock(BundleStartLevel.class);
+ BundleRevision bundleRevision =
EasyMock.niceMock(BundleRevision.class);
+
+ FeaturesServiceConfig cfg = new FeaturesServiceConfig();
+ BundleInstallSupport installSupport =
EasyMock.niceMock(BundleInstallSupport.class);
+ FrameworkInfo dummyInfo = new FrameworkInfo();
+ expect(installSupport.getInfo()).andReturn(dummyInfo).atLeastOnce();
+ expect(installSupport.installBundle(EasyMock.eq("root"),
EasyMock.eq("a100"), anyObject())).andReturn(bundle);
+ installSupport.startBundle(bundle);
+ expectLastCall();
+ expect(bundle.getBundleId()).andReturn(1L).anyTimes();
+ expect(bundle.getSymbolicName()).andReturn("a").anyTimes();
+ expect(bundle.getVersion()).andReturn(new Version("1.0.0")).anyTimes();
+ expect(bundle.getHeaders()).andReturn(new Hashtable<>()).anyTimes();
+
expect(bundle.adapt(BundleStartLevel.class)).andReturn(bundleStartLevel).anyTimes();
+
expect(bundle.adapt(BundleRevision.class)).andReturn(bundleRevision).anyTimes();
+ expect(bundleRevision.getBundle()).andReturn(bundle).anyTimes();
+
expect(bundleRevision.getCapabilities(null)).andReturn(Collections.emptyList()).anyTimes();
+
expect(bundleRevision.getRequirements(null)).andReturn(Collections.emptyList()).anyTimes();
+ EasyMock.replay(installSupport, bundle, bundleStartLevel,
bundleRevision);
+ FeaturesService featureService = new FeaturesServiceImpl(new
Storage(), null, null, this.resolver,
+ installSupport, null, cfg) {
+ @Override
+ protected DownloadManager createDownloadManager() throws
IOException {
+ return new TestDownloadManager(FeaturesServiceImplTest.class,
"data1");
+ }
+ };
+
+
featureService.addRepository(URI.create("custom:leadingzero/features.xml"));
+ Feature feature = featureService.getFeature("f", "1.0.01");
+ assertEquals("1.0.01", feature.getVersion());
+ installFeature(featureService, feature);
+ assertInstalled(featureService, feature);
+ assertEquals(FeatureState.Started,
featureService.getState(feature.getId()));
+ assertEquals(1, featureService.listInstalledFeatures().length);
+
+ dummyInfo.bundles.put(1L, bundle);
+ Map<String, Map<String, FeatureState>> states = new HashMap<>();
+ states.computeIfAbsent("root", k -> new
HashMap<>()).put(feature.getId(), FeatureState.Resolved);
+ EasyMock.reset(installSupport, bundle, bundleRevision,
bundleStartLevel);
+ expect(installSupport.getInfo()).andReturn(dummyInfo).anyTimes();
+ installSupport.stopBundle(EasyMock.capture(stoppedBundle),
EasyMock.anyInt());
+ expectLastCall();
+ expect(bundle.getBundleId()).andReturn(1L).anyTimes();
+ expect(bundle.getSymbolicName()).andReturn("a").anyTimes();
+ expect(bundle.getVersion()).andReturn(new Version("1.0.0")).anyTimes();
+ expect(bundle.getHeaders()).andReturn(new Hashtable<>()).anyTimes();
+
expect(bundle.adapt(BundleStartLevel.class)).andReturn(bundleStartLevel).anyTimes();
+
expect(bundle.adapt(BundleRevision.class)).andReturn(bundleRevision).anyTimes();
+ expect(bundleRevision.getBundle()).andReturn(bundle).anyTimes();
+
expect(bundleRevision.getCapabilities(null)).andReturn(Collections.emptyList()).anyTimes();
+
expect(bundleRevision.getRequirements(null)).andReturn(Collections.emptyList()).anyTimes();
+ EasyMock.replay(installSupport, bundle, bundleRevision,
bundleStartLevel);
+
+ featureService.updateFeaturesState(states,
EnumSet.noneOf(Option.class));
+ assertSame(bundle, stoppedBundle.getValue());
+ assertEquals(FeatureState.Resolved,
featureService.getState(feature.getId()));
+ }
+
@Test
public void testRemoveRepo2() throws Exception {
final FeaturesService featureService = createTestFeatureService();
diff --git
a/features/core/src/test/resources/org/apache/karaf/features/internal/service/leadingzero/features.xml
b/features/core/src/test/resources/org/apache/karaf/features/internal/service/leadingzero/features.xml
new file mode 100644
index 0000000000..496c31c723
--- /dev/null
+++
b/features/core/src/test/resources/org/apache/karaf/features/internal/service/leadingzero/features.xml
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+
+-->
+<features name="test" xmlns="http://karaf.apache.org/xmlns/features/v1.3.0";>
+
+ <feature name="f" version="1.0.01">
+ <bundle>a100</bundle>
+ </feature>
+
+</features>
\ No newline at end of file
