Browse Source

Distinguish `LicensedFeature` by family field (#116809) (#117348)

This PR fixes unintentional licensed feature overlaps for features with
the same name but different family fields.
Nikolaj Volgushev 10 months ago
parent
commit
fc09559da4

+ 5 - 0
docs/changelog/116809.yaml

@@ -0,0 +1,5 @@
+pr: 116809
+summary: "Distinguish `LicensedFeature` by family field"
+area: License
+type: bug
+issues: []

+ 2 - 2
x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicensedFeature.java

@@ -136,11 +136,11 @@ public abstract class LicensedFeature {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         LicensedFeature that = (LicensedFeature) o;
-        return Objects.equals(name, that.name);
+        return Objects.equals(name, that.name) && Objects.equals(family, that.family);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(name);
+        return Objects.hash(name, family);
     }
 }

+ 45 - 0
x-pack/plugin/core/src/test/java/org/elasticsearch/license/XPackLicenseStateTests.java

@@ -14,6 +14,7 @@ import org.elasticsearch.xpack.core.XPackField;
 
 import java.util.Arrays;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -228,6 +229,50 @@ public class XPackLicenseStateTests extends ESTestCase {
         assertThat(lastUsed.get(usage), equalTo(200L));
     }
 
+    public void testLastUsedMomentaryFeatureWithSameNameDifferentFamily() {
+        LicensedFeature.Momentary featureFamilyA = LicensedFeature.momentary("familyA", "goldFeature", GOLD);
+        LicensedFeature.Momentary featureFamilyB = LicensedFeature.momentary("familyB", "goldFeature", GOLD);
+
+        AtomicInteger currentTime = new AtomicInteger(100); // non zero start time
+        XPackLicenseState licenseState = new XPackLicenseState(currentTime::get);
+
+        featureFamilyA.check(licenseState);
+        featureFamilyB.check(licenseState);
+
+        Map<XPackLicenseState.FeatureUsage, Long> lastUsed = licenseState.getLastUsed();
+        assertThat("feature.check tracks usage separately by family", lastUsed, aMapWithSize(2));
+        Set<FeatureInfoWithTimestamp> actualFeatures = lastUsed.entrySet()
+            .stream()
+            .map(it -> new FeatureInfoWithTimestamp(it.getKey().feature().getFamily(), it.getKey().feature().getName(), it.getValue()))
+            .collect(Collectors.toSet());
+        assertThat(
+            actualFeatures,
+            containsInAnyOrder(
+                new FeatureInfoWithTimestamp("familyA", "goldFeature", 100L),
+                new FeatureInfoWithTimestamp("familyB", "goldFeature", 100L)
+            )
+        );
+
+        currentTime.set(200);
+        featureFamilyB.check(licenseState);
+
+        lastUsed = licenseState.getLastUsed();
+        assertThat("feature.check tracks usage separately by family", lastUsed, aMapWithSize(2));
+        actualFeatures = lastUsed.entrySet()
+            .stream()
+            .map(it -> new FeatureInfoWithTimestamp(it.getKey().feature().getFamily(), it.getKey().feature().getName(), it.getValue()))
+            .collect(Collectors.toSet());
+        assertThat(
+            actualFeatures,
+            containsInAnyOrder(
+                new FeatureInfoWithTimestamp("familyA", "goldFeature", 100L),
+                new FeatureInfoWithTimestamp("familyB", "goldFeature", 200L)
+            )
+        );
+    }
+
+    private record FeatureInfoWithTimestamp(String family, String featureName, Long timestamp) {}
+
     public void testLastUsedPersistentFeature() {
         LicensedFeature.Persistent goldFeature = LicensedFeature.persistent("family", "goldFeature", GOLD);
         AtomicInteger currentTime = new AtomicInteger(100); // non zero start time