You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2017/10/23 11:02:50 UTC
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
GitHub user aledsage opened a pull request:
https://github.com/apache/brooklyn-server/pull/866
catalog-init: add support for item/bundle removal
See https://docs.google.com/document/d/1Lm47Kx-cXPLe8BO34-qrL3ZMPosuUHJILYVQUswEH6Y/edit#heading=h.pdq16wrd8wjf for the Brooklyn proposal for upgrade support.
This PR implements a small part of that doc (and of that section). It adds support for:
* `brooklyn-catalog-force-remove-bundles`: says to ignore the given bundle(s) that are encountered in persisted state.
* `brooklyn-catalog-force-remove-legacy-items`: says to remove the given legacy items that are encountered in persisted state.
The next step is to add support for `brooklyn-catalog-upgrade-for-bundles` (not done in this PR).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/brooklyn-server catalogInit-support-item-removal
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/866.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #866
----
commit 9ebde3fd37597c41eb16a0337c753128ddd5ad20
Author: Aled Sage <al...@gmail.com>
Date: 2017-10-19T08:47:57Z
Support catalog item removal on init
commit 495ea30f7b1451ac73c7a8619d4811c24d79d80e
Author: Aled Sage <al...@gmail.com>
Date: 2017-10-20T14:41:50Z
Support force-bundle-removal on init
----
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146491921
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java ---
@@ -528,8 +528,60 @@ private void startBundle(OsgiBundleInstallationResult br) throws BundleException
}
}
+ private PersistedCatalogState filterPersistedState(PersistedCatalogState persistedState, RebindLogger rebindLogger) {
+ CatalogUpgrades catalogUpgrades = findCatalogUpgrades(rebindLogger);
--- End diff --
feels like this information should be kept in memory not just grabbed for this method. probably keep in `OsgiManager` or `BrooklynTypeRegistry` ? not essential for this PR as i expect it will be needed for follow-on work but for cleanliness we should avoid `findCatalogUpgrades` twice
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r147371764
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BundleUpgradeParser.java ---
@@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+package org.apache.brooklyn.core.catalog.internal;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Dictionary;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.util.osgi.VersionedName;
+import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
+import org.apache.brooklyn.util.text.QuotedStringTokenizer;
+import org.apache.brooklyn.util.text.Strings;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.VersionRange;
+
+import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+public class BundleUpgradeParser {
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS = "brooklyn-catalog-force-remove-legacy-items";
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_BUNDLES = "brooklyn-catalog-force-remove-bundles";
+
+ @Beta
+ public static final String MANIFEST_HEADER_UPGRADE_BUNDLES = "brooklyn-catalog-upgrade-for-bundles";
+
+ /**
+ * The result from parsing bundle(s) to find their upgrade info.
+ */
+ public static class CatalogUpgrades {
+ static final CatalogUpgrades EMPTY = new CatalogUpgrades(builder());
+
+ static class Builder {
+ private Set<VersionRangedName> removedLegacyItems = new LinkedHashSet<>();
+ private Set<VersionRangedName> removedBundles = new LinkedHashSet<>();
+
+ public Builder removedLegacyItems(Collection<VersionRangedName> vals) {
+ removedLegacyItems.addAll(vals);
+ return this;
+ }
+ public Builder removedBundles(Collection<VersionRangedName> vals) {
+ removedBundles.addAll(vals);
+ return this;
+ }
+ public Builder addAll(CatalogUpgrades other) {
+ removedLegacyItems.addAll(other.removedLegacyItems);
+ removedBundles.addAll(other.removedBundles);
+ return this;
+ }
+ public CatalogUpgrades build() {
+ return new CatalogUpgrades(this);
+ }
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ private Set<VersionRangedName> removedLegacyItems;
+ private Set<VersionRangedName> removedBundles;
+
+ public CatalogUpgrades(Builder builder) {
+ this.removedLegacyItems = ImmutableSet.copyOf(builder.removedLegacyItems);
+ this.removedBundles = ImmutableSet.copyOf(builder.removedBundles);
+ }
+
+ public boolean isEmpty() {
+ return removedLegacyItems.isEmpty() && removedBundles.isEmpty();
+ }
+
+ public Set<VersionRangedName> getRemovedLegacyItems() {
+ return removedLegacyItems;
+ }
+
+ public Set<VersionRangedName> getRemovedBundles() {
+ return removedBundles;
+ }
+
+ public boolean isLegacyItemRemoved(CatalogItem<?, ?> legacyCatalogItem) {
+ VersionedName name = new VersionedName(legacyCatalogItem.getSymbolicName(), legacyCatalogItem.getVersion());
+ return contains(removedLegacyItems, name);
+ }
+
+ public boolean isBundleRemoved(VersionedName bundle) {
+ return contains(removedBundles, bundle);
+ }
+
+ public boolean contains(Iterable<VersionRangedName> names, VersionedName name) {
+ for (VersionRangedName contender : names) {
+ if (contender.getSymbolicName().equals(name.getSymbolicName()) && contender.getOsgiVersionRange().includes(name.getOsgiVersion())) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+
+ /**
+ * Records a name (string) and version range (string),
+ * with conveniences for pretty-printing and converting to OSGi format.
+ *
+ * Implementation-wise, this is similar to {@link VersionedName}, but is intended
+ * as internal-only so is cut down to only what is needed.
+ */
+ public static class VersionRangedName {
+ private final String name;
+ private final String v;
+ private transient volatile VersionRange cachedOsgiVersionRange;
+
+ public static VersionRangedName fromString(String val, boolean singleVersionIsOsgiRange) {
+ if (Strings.isBlank(val)) {
+ throw new IllegalArgumentException("Must not be blank");
+ }
+ String[] parts = val.split(":");
+ if (parts.length > 2) {
+ throw new IllegalArgumentException("Identifier '"+val+"' has too many parts; max one ':' symbol");
+ }
+ if (parts.length == 1 || Strings.isBlank(parts[1])) {
+ throw new IllegalArgumentException("Identifier '"+val+"' must be of 'name:versionRange' syntax");
+ } else if (singleVersionIsOsgiRange || (parts[1].startsWith("(") || parts[1].startsWith("["))) {
+ return new VersionRangedName(parts[0], parts[1]);
+ } else {
+ return new VersionRangedName(parts[0], "["+parts[1]+","+parts[1]+"]");
+ }
+ }
+
+ public VersionRangedName(String name, VersionRange v) {
+ this.name = checkNotNull(name, "name").toString();
+ this.v = checkNotNull(v, "versionRange").toString();
+ }
+
+ private VersionRangedName(String name, String v) {
+ this.name = checkNotNull(name, "name");
+ this.v = checkNotNull(v, "versionRange");
+ }
+
+ @Override
+ public String toString() {
+ return name + ":" + v;
+ }
+
+ public String toOsgiString() {
+ return name + ":" + getOsgiVersionRange();
+ }
+
+ public String getSymbolicName() {
+ return name;
+ }
+
+ public VersionRange getOsgiVersionRange() {
+ if (cachedOsgiVersionRange == null) {
+ cachedOsgiVersionRange = VersionRange.valueOf(BrooklynVersionSyntax.toValidOsgiVersionRange(v));
+ }
+ return cachedOsgiVersionRange;
+ }
+
+ public String getVersionString() {
+ return v;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(name, v);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof VersionRangedName)) {
+ return false;
+ }
+ VersionRangedName o = (VersionRangedName) other;
+ return Objects.equal(name, o.name) && Objects.equal(v, o.v);
+ }
+ }
+
+ public static CatalogUpgrades parseBundleManifestForCatalogUpgrades(Bundle bundle) {
+ Dictionary<String, String> headers = bundle.getHeaders();
+ String removedLegacyItemsHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS);
+ String removedBundlesHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_BUNDLES);
+ List<VersionRangedName> removedLegacyItems = ImmutableList.of();
+ List<VersionRangedName> removedBundles = ImmutableList.of();
+ if (removedLegacyItemsHeader == null && removedBundlesHeader == null) {
+ return CatalogUpgrades.EMPTY;
+ }
+ if (removedLegacyItemsHeader != null) {
+ removedLegacyItems = parseVersionRangedNameList(removedLegacyItemsHeader, false);
+ }
+ if (removedBundlesHeader != null) {
+ removedBundles = parseVersionRangedNameList(removedBundlesHeader, false);
+ }
+ return CatalogUpgrades.builder()
+ .removedLegacyItems(removedLegacyItems)
+ .removedBundles(removedBundles)
+ .build();
+ }
+
+ @VisibleForTesting
+ static List<VersionRangedName> parseVersionRangedNameList(String input, boolean singleVersionIsOsgiRange) {
+ List<String> vals = QuotedStringTokenizer.builder()
--- End diff --
sorry i confused the range test with the header test
need to document this well, ugly but not unreasonable.
i've added a good error message if user forgets them (also adds a trim if user puts spaces in):
https://github.com/apache/brooklyn-server/pull/870
i'll update that with any other tweaks i find too
---
[GitHub] brooklyn-server issue #866: catalog-init: add support for item/bundle remova...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/866
[MINOR] I don't understand the messages on the last two commits -- we're not actually supporting these headers yet in that we don't respond.
Some minor tidies in #870. Biggest of these is adding docs and test for replacing a snapshot/RCs with a GA release.
Crazy thing: While I've not looked at what actually happens if a remove range includes things being added, I could see this being a nice trick, say `[1,1.0.1)` when releasing `1.0.0.GA` in order to remove `1.0.0.rc1` and `1.0.0.SNAPSHOT`. As cute as it is it's probably a bad idea even if it sometimes works as it could easily go wrong.
Merging this.
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146701756
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BundleUpgradeParser.java ---
@@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+package org.apache.brooklyn.core.catalog.internal;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Dictionary;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.util.osgi.VersionedName;
+import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
+import org.apache.brooklyn.util.text.QuotedStringTokenizer;
+import org.apache.brooklyn.util.text.Strings;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.VersionRange;
+
+import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+public class BundleUpgradeParser {
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS = "brooklyn-catalog-force-remove-legacy-items";
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_BUNDLES = "brooklyn-catalog-force-remove-bundles";
+
+ @Beta
+ public static final String MANIFEST_HEADER_UPGRADE_BUNDLES = "brooklyn-catalog-upgrade-for-bundles";
+
+ /**
+ * The result from parsing bundle(s) to find their upgrade info.
+ */
+ public static class CatalogUpgrades {
+ static final CatalogUpgrades EMPTY = new CatalogUpgrades(builder());
+
+ static class Builder {
+ private Set<VersionRangedName> removedLegacyItems = new LinkedHashSet<>();
+ private Set<VersionRangedName> removedBundles = new LinkedHashSet<>();
+
+ public Builder removedLegacyItems(Collection<VersionRangedName> vals) {
+ removedLegacyItems.addAll(vals);
+ return this;
+ }
+ public Builder removedBundles(Collection<VersionRangedName> vals) {
+ removedBundles.addAll(vals);
+ return this;
+ }
+ public Builder addAll(CatalogUpgrades other) {
+ removedLegacyItems.addAll(other.removedLegacyItems);
+ removedBundles.addAll(other.removedBundles);
+ return this;
+ }
+ public CatalogUpgrades build() {
+ return new CatalogUpgrades(this);
+ }
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ private Set<VersionRangedName> removedLegacyItems;
+ private Set<VersionRangedName> removedBundles;
+
+ public CatalogUpgrades(Builder builder) {
+ this.removedLegacyItems = ImmutableSet.copyOf(builder.removedLegacyItems);
+ this.removedBundles = ImmutableSet.copyOf(builder.removedBundles);
+ }
+
+ public boolean isEmpty() {
+ return removedLegacyItems.isEmpty() && removedBundles.isEmpty();
+ }
+
+ public Set<VersionRangedName> getRemovedLegacyItems() {
+ return removedLegacyItems;
+ }
+
+ public Set<VersionRangedName> getRemovedBundles() {
+ return removedBundles;
+ }
+
+ public boolean isLegacyItemRemoved(CatalogItem<?, ?> legacyCatalogItem) {
+ VersionedName name = new VersionedName(legacyCatalogItem.getSymbolicName(), legacyCatalogItem.getVersion());
+ return contains(removedLegacyItems, name);
+ }
+
+ public boolean isBundleRemoved(VersionedName bundle) {
+ return contains(removedBundles, bundle);
+ }
+
+ public boolean contains(Iterable<VersionRangedName> names, VersionedName name) {
+ for (VersionRangedName contender : names) {
+ if (contender.getSymbolicName().equals(name.getSymbolicName()) && contender.getOsgiVersionRange().includes(name.getOsgiVersion())) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+
+ /**
+ * Records a name (string) and version range (string),
+ * with conveniences for pretty-printing and converting to OSGi format.
+ *
+ * Implementation-wise, this is similar to {@link VersionedName}, but is intended
+ * as internal-only so is cut down to only what is needed.
+ */
+ public static class VersionRangedName {
+ private final String name;
+ private final String v;
+ private transient volatile VersionRange cachedOsgiVersionRange;
+
+ public static VersionRangedName fromString(String val, boolean singleVersionIsOsgiRange) {
+ if (Strings.isBlank(val)) {
+ throw new IllegalArgumentException("Must not be blank");
+ }
+ String[] parts = val.split(":");
+ if (parts.length > 2) {
+ throw new IllegalArgumentException("Identifier '"+val+"' has too many parts; max one ':' symbol");
+ }
+ if (parts.length == 1 || Strings.isBlank(parts[1])) {
+ throw new IllegalArgumentException("Identifier '"+val+"' must be of 'name:versionRange' syntax");
+ } else if (singleVersionIsOsgiRange || (parts[1].startsWith("(") || parts[1].startsWith("["))) {
+ return new VersionRangedName(parts[0], parts[1]);
+ } else {
+ return new VersionRangedName(parts[0], "["+parts[1]+","+parts[1]+"]");
+ }
+ }
+
+ public VersionRangedName(String name, VersionRange v) {
+ this.name = checkNotNull(name, "name").toString();
+ this.v = checkNotNull(v, "versionRange").toString();
+ }
+
+ private VersionRangedName(String name, String v) {
+ this.name = checkNotNull(name, "name");
+ this.v = checkNotNull(v, "versionRange");
+ }
+
+ @Override
+ public String toString() {
+ return name + ":" + v;
+ }
+
+ public String toOsgiString() {
+ return name + ":" + getOsgiVersionRange();
+ }
+
+ public String getSymbolicName() {
+ return name;
+ }
+
+ public VersionRange getOsgiVersionRange() {
+ if (cachedOsgiVersionRange == null) {
+ cachedOsgiVersionRange = VersionRange.valueOf(BrooklynVersionSyntax.toValidOsgiVersionRange(v));
+ }
+ return cachedOsgiVersionRange;
+ }
+
+ public String getVersionString() {
+ return v;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(name, v);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof VersionRangedName)) {
+ return false;
+ }
+ VersionRangedName o = (VersionRangedName) other;
+ return Objects.equal(name, o.name) && Objects.equal(v, o.v);
+ }
+ }
+
+ public static CatalogUpgrades parseBundleManifestForCatalogUpgrades(Bundle bundle) {
+ Dictionary<String, String> headers = bundle.getHeaders();
+ String removedLegacyItemsHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS);
+ String removedBundlesHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_BUNDLES);
+ List<VersionRangedName> removedLegacyItems = ImmutableList.of();
+ List<VersionRangedName> removedBundles = ImmutableList.of();
+ if (removedLegacyItemsHeader == null && removedBundlesHeader == null) {
+ return CatalogUpgrades.EMPTY;
+ }
+ if (removedLegacyItemsHeader != null) {
+ removedLegacyItems = parseVersionRangedNameList(removedLegacyItemsHeader, false);
+ }
+ if (removedBundlesHeader != null) {
+ removedBundles = parseVersionRangedNameList(removedBundlesHeader, false);
+ }
+ return CatalogUpgrades.builder()
+ .removedLegacyItems(removedLegacyItems)
+ .removedBundles(removedBundles)
+ .build();
+ }
+
+ @VisibleForTesting
+ static List<VersionRangedName> parseVersionRangedNameList(String input, boolean singleVersionIsOsgiRange) {
--- End diff --
That's the default semantics for an OSGi version range I believe (e.g. `1.0.0` actually means `>= 1.0.0`), hence including support for that. However, you're right that it's not actually used.
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146701965
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BundleUpgradeParser.java ---
@@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+package org.apache.brooklyn.core.catalog.internal;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Dictionary;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.util.osgi.VersionedName;
+import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
+import org.apache.brooklyn.util.text.QuotedStringTokenizer;
+import org.apache.brooklyn.util.text.Strings;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.VersionRange;
+
+import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+public class BundleUpgradeParser {
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS = "brooklyn-catalog-force-remove-legacy-items";
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_BUNDLES = "brooklyn-catalog-force-remove-bundles";
+
+ @Beta
+ public static final String MANIFEST_HEADER_UPGRADE_BUNDLES = "brooklyn-catalog-upgrade-for-bundles";
+
+ /**
+ * The result from parsing bundle(s) to find their upgrade info.
+ */
+ public static class CatalogUpgrades {
+ static final CatalogUpgrades EMPTY = new CatalogUpgrades(builder());
+
+ static class Builder {
+ private Set<VersionRangedName> removedLegacyItems = new LinkedHashSet<>();
+ private Set<VersionRangedName> removedBundles = new LinkedHashSet<>();
+
+ public Builder removedLegacyItems(Collection<VersionRangedName> vals) {
+ removedLegacyItems.addAll(vals);
+ return this;
+ }
+ public Builder removedBundles(Collection<VersionRangedName> vals) {
+ removedBundles.addAll(vals);
+ return this;
+ }
+ public Builder addAll(CatalogUpgrades other) {
+ removedLegacyItems.addAll(other.removedLegacyItems);
+ removedBundles.addAll(other.removedBundles);
+ return this;
+ }
+ public CatalogUpgrades build() {
+ return new CatalogUpgrades(this);
+ }
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ private Set<VersionRangedName> removedLegacyItems;
+ private Set<VersionRangedName> removedBundles;
+
+ public CatalogUpgrades(Builder builder) {
+ this.removedLegacyItems = ImmutableSet.copyOf(builder.removedLegacyItems);
+ this.removedBundles = ImmutableSet.copyOf(builder.removedBundles);
+ }
+
+ public boolean isEmpty() {
+ return removedLegacyItems.isEmpty() && removedBundles.isEmpty();
+ }
+
+ public Set<VersionRangedName> getRemovedLegacyItems() {
+ return removedLegacyItems;
+ }
+
+ public Set<VersionRangedName> getRemovedBundles() {
+ return removedBundles;
+ }
+
+ public boolean isLegacyItemRemoved(CatalogItem<?, ?> legacyCatalogItem) {
+ VersionedName name = new VersionedName(legacyCatalogItem.getSymbolicName(), legacyCatalogItem.getVersion());
+ return contains(removedLegacyItems, name);
+ }
+
+ public boolean isBundleRemoved(VersionedName bundle) {
+ return contains(removedBundles, bundle);
+ }
+
+ public boolean contains(Iterable<VersionRangedName> names, VersionedName name) {
+ for (VersionRangedName contender : names) {
+ if (contender.getSymbolicName().equals(name.getSymbolicName()) && contender.getOsgiVersionRange().includes(name.getOsgiVersion())) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+
+ /**
+ * Records a name (string) and version range (string),
+ * with conveniences for pretty-printing and converting to OSGi format.
+ *
+ * Implementation-wise, this is similar to {@link VersionedName}, but is intended
+ * as internal-only so is cut down to only what is needed.
+ */
+ public static class VersionRangedName {
+ private final String name;
+ private final String v;
+ private transient volatile VersionRange cachedOsgiVersionRange;
+
+ public static VersionRangedName fromString(String val, boolean singleVersionIsOsgiRange) {
+ if (Strings.isBlank(val)) {
+ throw new IllegalArgumentException("Must not be blank");
+ }
+ String[] parts = val.split(":");
+ if (parts.length > 2) {
+ throw new IllegalArgumentException("Identifier '"+val+"' has too many parts; max one ':' symbol");
+ }
+ if (parts.length == 1 || Strings.isBlank(parts[1])) {
+ throw new IllegalArgumentException("Identifier '"+val+"' must be of 'name:versionRange' syntax");
+ } else if (singleVersionIsOsgiRange || (parts[1].startsWith("(") || parts[1].startsWith("["))) {
+ return new VersionRangedName(parts[0], parts[1]);
+ } else {
+ return new VersionRangedName(parts[0], "["+parts[1]+","+parts[1]+"]");
+ }
+ }
+
+ public VersionRangedName(String name, VersionRange v) {
+ this.name = checkNotNull(name, "name").toString();
+ this.v = checkNotNull(v, "versionRange").toString();
+ }
+
+ private VersionRangedName(String name, String v) {
+ this.name = checkNotNull(name, "name");
+ this.v = checkNotNull(v, "versionRange");
+ }
+
+ @Override
+ public String toString() {
+ return name + ":" + v;
+ }
+
+ public String toOsgiString() {
+ return name + ":" + getOsgiVersionRange();
+ }
+
+ public String getSymbolicName() {
+ return name;
+ }
+
+ public VersionRange getOsgiVersionRange() {
+ if (cachedOsgiVersionRange == null) {
+ cachedOsgiVersionRange = VersionRange.valueOf(BrooklynVersionSyntax.toValidOsgiVersionRange(v));
+ }
+ return cachedOsgiVersionRange;
+ }
+
+ public String getVersionString() {
+ return v;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(name, v);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof VersionRangedName)) {
+ return false;
+ }
+ VersionRangedName o = (VersionRangedName) other;
+ return Objects.equal(name, o.name) && Objects.equal(v, o.v);
+ }
+ }
+
+ public static CatalogUpgrades parseBundleManifestForCatalogUpgrades(Bundle bundle) {
+ Dictionary<String, String> headers = bundle.getHeaders();
+ String removedLegacyItemsHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS);
+ String removedBundlesHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_BUNDLES);
+ List<VersionRangedName> removedLegacyItems = ImmutableList.of();
+ List<VersionRangedName> removedBundles = ImmutableList.of();
+ if (removedLegacyItemsHeader == null && removedBundlesHeader == null) {
+ return CatalogUpgrades.EMPTY;
+ }
+ if (removedLegacyItemsHeader != null) {
+ removedLegacyItems = parseVersionRangedNameList(removedLegacyItemsHeader, false);
+ }
+ if (removedBundlesHeader != null) {
+ removedBundles = parseVersionRangedNameList(removedBundlesHeader, false);
+ }
+ return CatalogUpgrades.builder()
+ .removedLegacyItems(removedLegacyItems)
+ .removedBundles(removedBundles)
+ .build();
+ }
+
+ @VisibleForTesting
+ static List<VersionRangedName> parseVersionRangedNameList(String input, boolean singleVersionIsOsgiRange) {
+ List<String> vals = QuotedStringTokenizer.builder()
--- End diff --
It works if each `name:versionRange` is in quotes, with commas between them.
---
[GitHub] brooklyn-server issue #866: catalog-init: add support for item/bundle remova...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/866
@ahgittin we do support and respond to:
* `force-remove-bundles: *` - it means ignore from persisted state (i.e. don't rebind) all older versions of this bundle.
* `force-remove-legacy-items: *` - it means find all items in this bundle, and ignore from persisted `/catalog` (i.e. don't rebind) all older versions of those items.
There is still the missing next piece of the `upgrade-types`, to remember what was removed and what the new version for those bundles/items should be, so we can use those new things when someone tries to reference the old one.
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146485482
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BundleUpgradeParser.java ---
@@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+package org.apache.brooklyn.core.catalog.internal;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Dictionary;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.util.osgi.VersionedName;
+import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
+import org.apache.brooklyn.util.text.QuotedStringTokenizer;
+import org.apache.brooklyn.util.text.Strings;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.VersionRange;
+
+import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+public class BundleUpgradeParser {
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS = "brooklyn-catalog-force-remove-legacy-items";
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_BUNDLES = "brooklyn-catalog-force-remove-bundles";
+
+ @Beta
+ public static final String MANIFEST_HEADER_UPGRADE_BUNDLES = "brooklyn-catalog-upgrade-for-bundles";
+
+ /**
+ * The result from parsing bundle(s) to find their upgrade info.
+ */
+ public static class CatalogUpgrades {
+ static final CatalogUpgrades EMPTY = new CatalogUpgrades(builder());
+
+ static class Builder {
+ private Set<VersionRangedName> removedLegacyItems = new LinkedHashSet<>();
+ private Set<VersionRangedName> removedBundles = new LinkedHashSet<>();
+
+ public Builder removedLegacyItems(Collection<VersionRangedName> vals) {
+ removedLegacyItems.addAll(vals);
+ return this;
+ }
+ public Builder removedBundles(Collection<VersionRangedName> vals) {
+ removedBundles.addAll(vals);
+ return this;
+ }
+ public Builder addAll(CatalogUpgrades other) {
+ removedLegacyItems.addAll(other.removedLegacyItems);
+ removedBundles.addAll(other.removedBundles);
+ return this;
+ }
+ public CatalogUpgrades build() {
+ return new CatalogUpgrades(this);
+ }
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ private Set<VersionRangedName> removedLegacyItems;
+ private Set<VersionRangedName> removedBundles;
+
+ public CatalogUpgrades(Builder builder) {
+ this.removedLegacyItems = ImmutableSet.copyOf(builder.removedLegacyItems);
+ this.removedBundles = ImmutableSet.copyOf(builder.removedBundles);
+ }
+
+ public boolean isEmpty() {
+ return removedLegacyItems.isEmpty() && removedBundles.isEmpty();
+ }
+
+ public Set<VersionRangedName> getRemovedLegacyItems() {
+ return removedLegacyItems;
+ }
+
+ public Set<VersionRangedName> getRemovedBundles() {
+ return removedBundles;
+ }
+
+ public boolean isLegacyItemRemoved(CatalogItem<?, ?> legacyCatalogItem) {
+ VersionedName name = new VersionedName(legacyCatalogItem.getSymbolicName(), legacyCatalogItem.getVersion());
+ return contains(removedLegacyItems, name);
+ }
+
+ public boolean isBundleRemoved(VersionedName bundle) {
+ return contains(removedBundles, bundle);
+ }
+
+ public boolean contains(Iterable<VersionRangedName> names, VersionedName name) {
+ for (VersionRangedName contender : names) {
+ if (contender.getSymbolicName().equals(name.getSymbolicName()) && contender.getOsgiVersionRange().includes(name.getOsgiVersion())) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+
+ /**
+ * Records a name (string) and version range (string),
+ * with conveniences for pretty-printing and converting to OSGi format.
+ *
+ * Implementation-wise, this is similar to {@link VersionedName}, but is intended
+ * as internal-only so is cut down to only what is needed.
+ */
+ public static class VersionRangedName {
+ private final String name;
+ private final String v;
+ private transient volatile VersionRange cachedOsgiVersionRange;
+
+ public static VersionRangedName fromString(String val, boolean singleVersionIsOsgiRange) {
+ if (Strings.isBlank(val)) {
+ throw new IllegalArgumentException("Must not be blank");
+ }
+ String[] parts = val.split(":");
+ if (parts.length > 2) {
+ throw new IllegalArgumentException("Identifier '"+val+"' has too many parts; max one ':' symbol");
+ }
+ if (parts.length == 1 || Strings.isBlank(parts[1])) {
+ throw new IllegalArgumentException("Identifier '"+val+"' must be of 'name:versionRange' syntax");
+ } else if (singleVersionIsOsgiRange || (parts[1].startsWith("(") || parts[1].startsWith("["))) {
+ return new VersionRangedName(parts[0], parts[1]);
+ } else {
+ return new VersionRangedName(parts[0], "["+parts[1]+","+parts[1]+"]");
+ }
+ }
+
+ public VersionRangedName(String name, VersionRange v) {
+ this.name = checkNotNull(name, "name").toString();
+ this.v = checkNotNull(v, "versionRange").toString();
+ }
+
+ private VersionRangedName(String name, String v) {
+ this.name = checkNotNull(name, "name");
+ this.v = checkNotNull(v, "versionRange");
+ }
+
+ @Override
+ public String toString() {
+ return name + ":" + v;
+ }
+
+ public String toOsgiString() {
+ return name + ":" + getOsgiVersionRange();
+ }
+
+ public String getSymbolicName() {
+ return name;
+ }
+
+ public VersionRange getOsgiVersionRange() {
+ if (cachedOsgiVersionRange == null) {
+ cachedOsgiVersionRange = VersionRange.valueOf(BrooklynVersionSyntax.toValidOsgiVersionRange(v));
+ }
+ return cachedOsgiVersionRange;
+ }
+
+ public String getVersionString() {
+ return v;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(name, v);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof VersionRangedName)) {
+ return false;
+ }
+ VersionRangedName o = (VersionRangedName) other;
+ return Objects.equal(name, o.name) && Objects.equal(v, o.v);
+ }
+ }
+
+ public static CatalogUpgrades parseBundleManifestForCatalogUpgrades(Bundle bundle) {
+ Dictionary<String, String> headers = bundle.getHeaders();
+ String removedLegacyItemsHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS);
+ String removedBundlesHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_BUNDLES);
+ List<VersionRangedName> removedLegacyItems = ImmutableList.of();
+ List<VersionRangedName> removedBundles = ImmutableList.of();
+ if (removedLegacyItemsHeader == null && removedBundlesHeader == null) {
+ return CatalogUpgrades.EMPTY;
+ }
+ if (removedLegacyItemsHeader != null) {
--- End diff --
if it were me i'd have `parseVersionRangedNameList(null)` return empty list and you can rewrite the entire method as
```
return CatalogUpgrades.builder()
.removedLegacyItems(parseVersionRangedNameList(headers.get(MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS)))
.removedBundles(parseVersionRangedNameList(headers.get(MANIFEST_HEADER_FORCE_REMOVE_BUNDLES)))
.build();
```
not everyone likes my style though :) ... however if you do want the efficiency savings of line 211 then it should live before line 208
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146487298
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BundleUpgradeParser.java ---
@@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+package org.apache.brooklyn.core.catalog.internal;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Dictionary;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.util.osgi.VersionedName;
+import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
+import org.apache.brooklyn.util.text.QuotedStringTokenizer;
+import org.apache.brooklyn.util.text.Strings;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.VersionRange;
+
+import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+public class BundleUpgradeParser {
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS = "brooklyn-catalog-force-remove-legacy-items";
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_BUNDLES = "brooklyn-catalog-force-remove-bundles";
+
+ @Beta
+ public static final String MANIFEST_HEADER_UPGRADE_BUNDLES = "brooklyn-catalog-upgrade-for-bundles";
+
+ /**
+ * The result from parsing bundle(s) to find their upgrade info.
+ */
+ public static class CatalogUpgrades {
+ static final CatalogUpgrades EMPTY = new CatalogUpgrades(builder());
+
+ static class Builder {
+ private Set<VersionRangedName> removedLegacyItems = new LinkedHashSet<>();
+ private Set<VersionRangedName> removedBundles = new LinkedHashSet<>();
+
+ public Builder removedLegacyItems(Collection<VersionRangedName> vals) {
+ removedLegacyItems.addAll(vals);
+ return this;
+ }
+ public Builder removedBundles(Collection<VersionRangedName> vals) {
+ removedBundles.addAll(vals);
+ return this;
+ }
+ public Builder addAll(CatalogUpgrades other) {
+ removedLegacyItems.addAll(other.removedLegacyItems);
+ removedBundles.addAll(other.removedBundles);
+ return this;
+ }
+ public CatalogUpgrades build() {
+ return new CatalogUpgrades(this);
+ }
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ private Set<VersionRangedName> removedLegacyItems;
+ private Set<VersionRangedName> removedBundles;
+
+ public CatalogUpgrades(Builder builder) {
+ this.removedLegacyItems = ImmutableSet.copyOf(builder.removedLegacyItems);
+ this.removedBundles = ImmutableSet.copyOf(builder.removedBundles);
+ }
+
+ public boolean isEmpty() {
+ return removedLegacyItems.isEmpty() && removedBundles.isEmpty();
+ }
+
+ public Set<VersionRangedName> getRemovedLegacyItems() {
+ return removedLegacyItems;
+ }
+
+ public Set<VersionRangedName> getRemovedBundles() {
+ return removedBundles;
+ }
+
+ public boolean isLegacyItemRemoved(CatalogItem<?, ?> legacyCatalogItem) {
+ VersionedName name = new VersionedName(legacyCatalogItem.getSymbolicName(), legacyCatalogItem.getVersion());
+ return contains(removedLegacyItems, name);
+ }
+
+ public boolean isBundleRemoved(VersionedName bundle) {
+ return contains(removedBundles, bundle);
+ }
+
+ public boolean contains(Iterable<VersionRangedName> names, VersionedName name) {
+ for (VersionRangedName contender : names) {
+ if (contender.getSymbolicName().equals(name.getSymbolicName()) && contender.getOsgiVersionRange().includes(name.getOsgiVersion())) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+
+ /**
+ * Records a name (string) and version range (string),
+ * with conveniences for pretty-printing and converting to OSGi format.
+ *
+ * Implementation-wise, this is similar to {@link VersionedName}, but is intended
+ * as internal-only so is cut down to only what is needed.
+ */
+ public static class VersionRangedName {
+ private final String name;
+ private final String v;
+ private transient volatile VersionRange cachedOsgiVersionRange;
+
+ public static VersionRangedName fromString(String val, boolean singleVersionIsOsgiRange) {
+ if (Strings.isBlank(val)) {
+ throw new IllegalArgumentException("Must not be blank");
+ }
+ String[] parts = val.split(":");
+ if (parts.length > 2) {
+ throw new IllegalArgumentException("Identifier '"+val+"' has too many parts; max one ':' symbol");
+ }
+ if (parts.length == 1 || Strings.isBlank(parts[1])) {
+ throw new IllegalArgumentException("Identifier '"+val+"' must be of 'name:versionRange' syntax");
+ } else if (singleVersionIsOsgiRange || (parts[1].startsWith("(") || parts[1].startsWith("["))) {
+ return new VersionRangedName(parts[0], parts[1]);
+ } else {
+ return new VersionRangedName(parts[0], "["+parts[1]+","+parts[1]+"]");
+ }
+ }
+
+ public VersionRangedName(String name, VersionRange v) {
+ this.name = checkNotNull(name, "name").toString();
+ this.v = checkNotNull(v, "versionRange").toString();
+ }
+
+ private VersionRangedName(String name, String v) {
+ this.name = checkNotNull(name, "name");
+ this.v = checkNotNull(v, "versionRange");
+ }
+
+ @Override
+ public String toString() {
+ return name + ":" + v;
+ }
+
+ public String toOsgiString() {
+ return name + ":" + getOsgiVersionRange();
+ }
+
+ public String getSymbolicName() {
+ return name;
+ }
+
+ public VersionRange getOsgiVersionRange() {
+ if (cachedOsgiVersionRange == null) {
+ cachedOsgiVersionRange = VersionRange.valueOf(BrooklynVersionSyntax.toValidOsgiVersionRange(v));
+ }
+ return cachedOsgiVersionRange;
+ }
+
+ public String getVersionString() {
+ return v;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(name, v);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof VersionRangedName)) {
+ return false;
+ }
+ VersionRangedName o = (VersionRangedName) other;
+ return Objects.equal(name, o.name) && Objects.equal(v, o.v);
+ }
+ }
+
+ public static CatalogUpgrades parseBundleManifestForCatalogUpgrades(Bundle bundle) {
+ Dictionary<String, String> headers = bundle.getHeaders();
+ String removedLegacyItemsHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS);
+ String removedBundlesHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_BUNDLES);
+ List<VersionRangedName> removedLegacyItems = ImmutableList.of();
+ List<VersionRangedName> removedBundles = ImmutableList.of();
+ if (removedLegacyItemsHeader == null && removedBundlesHeader == null) {
+ return CatalogUpgrades.EMPTY;
+ }
+ if (removedLegacyItemsHeader != null) {
+ removedLegacyItems = parseVersionRangedNameList(removedLegacyItemsHeader, false);
+ }
+ if (removedBundlesHeader != null) {
+ removedBundles = parseVersionRangedNameList(removedBundlesHeader, false);
+ }
+ return CatalogUpgrades.builder()
+ .removedLegacyItems(removedLegacyItems)
+ .removedBundles(removedBundles)
+ .build();
+ }
+
+ @VisibleForTesting
+ static List<VersionRangedName> parseVersionRangedNameList(String input, boolean singleVersionIsOsgiRange) {
--- End diff --
when does a caller pass `singleVersionIsOsgiRange` true? that seems an odd argument esp in `VersionRangeName.fromString`.
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146490317
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BundleUpgradeParser.java ---
@@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+package org.apache.brooklyn.core.catalog.internal;
--- End diff --
i'm hoping this package and parent is removed shortly. move to `org.apache.brooklyn.core.typereg` and rename `TypeRegistryUpgrades` ?
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146492023
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java ---
@@ -528,8 +528,60 @@ private void startBundle(OsgiBundleInstallationResult br) throws BundleException
}
}
+ private PersistedCatalogState filterPersistedState(PersistedCatalogState persistedState, RebindLogger rebindLogger) {
+ CatalogUpgrades catalogUpgrades = findCatalogUpgrades(rebindLogger);
+
+ if (catalogUpgrades.isEmpty()) {
+ return persistedState;
+ } else {
+ rebindLogger.info("Filtering out persisted catalog: removedBundles="+catalogUpgrades.getRemovedBundles()+"; removedLegacyItems="+catalogUpgrades.getRemovedLegacyItems());
+ }
+
+ Map<VersionedName, InstallableManagedBundle> bundles = new LinkedHashMap<>();
+ for (Map.Entry<VersionedName, InstallableManagedBundle> entry : persistedState.getBundles().entrySet()) {
+ if (catalogUpgrades.isBundleRemoved(entry.getKey())) {
+ rebindLogger.debug("Filtering out persisted bundle "+entry.getKey());
+ } else {
+ bundles.put(entry.getKey(), entry.getValue());
+ }
+ }
+
+ List<CatalogItem<?, ?>> legacyCatalogItems = new ArrayList<>();
+ for (CatalogItem<?, ?> legacyCatalogItem : persistedState.getLegacyCatalogItems()) {
+ if (catalogUpgrades.isLegacyItemRemoved(legacyCatalogItem)) {
+ rebindLogger.debug("Filtering out persisted legacy catalog item "+legacyCatalogItem.getId());
+ } else {
+ legacyCatalogItems.add(legacyCatalogItem);
+ }
+ }
+
+ return new PersistedCatalogState(bundles, legacyCatalogItems);
+ }
+
+ private CatalogUpgrades findCatalogUpgrades(RebindLogger rebindLogger) {
--- End diff --
rename as `collectCatalogUpgradeInstructions` ?
---
[GitHub] brooklyn-server issue #866: catalog-init: add support for item/bundle remova...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/866
@ahgittin can you take another look please. Comments addressed. (Except the class is still named `BundleUpgradeParser`, moved to `org.apache.brooklyn.core.typereg`. I like that name because the upgrade info is in the bundle manifest. Some of that metadata will describe type upgrades (rather than just bundle upgrade/replacement), but this name feels better to me than `TypeRegistryUpgrades`. It's purely internal so we can rename it any time.)
---
[GitHub] brooklyn-server issue #866: catalog-init: add support for item/bundle remova...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/866
general comment ... is `blacklist` a better name to use (throughout) than `remove` ? probably not worth changing but thought i'd ask.
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146488928
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BundleUpgradeParser.java ---
@@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+package org.apache.brooklyn.core.catalog.internal;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Dictionary;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.util.osgi.VersionedName;
+import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
+import org.apache.brooklyn.util.text.QuotedStringTokenizer;
+import org.apache.brooklyn.util.text.Strings;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.VersionRange;
+
+import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+public class BundleUpgradeParser {
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS = "brooklyn-catalog-force-remove-legacy-items";
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_BUNDLES = "brooklyn-catalog-force-remove-bundles";
+
+ @Beta
+ public static final String MANIFEST_HEADER_UPGRADE_BUNDLES = "brooklyn-catalog-upgrade-for-bundles";
+
+ /**
+ * The result from parsing bundle(s) to find their upgrade info.
+ */
+ public static class CatalogUpgrades {
+ static final CatalogUpgrades EMPTY = new CatalogUpgrades(builder());
+
+ static class Builder {
+ private Set<VersionRangedName> removedLegacyItems = new LinkedHashSet<>();
+ private Set<VersionRangedName> removedBundles = new LinkedHashSet<>();
+
+ public Builder removedLegacyItems(Collection<VersionRangedName> vals) {
+ removedLegacyItems.addAll(vals);
+ return this;
+ }
+ public Builder removedBundles(Collection<VersionRangedName> vals) {
+ removedBundles.addAll(vals);
+ return this;
+ }
+ public Builder addAll(CatalogUpgrades other) {
+ removedLegacyItems.addAll(other.removedLegacyItems);
+ removedBundles.addAll(other.removedBundles);
+ return this;
+ }
+ public CatalogUpgrades build() {
+ return new CatalogUpgrades(this);
+ }
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ private Set<VersionRangedName> removedLegacyItems;
+ private Set<VersionRangedName> removedBundles;
+
+ public CatalogUpgrades(Builder builder) {
+ this.removedLegacyItems = ImmutableSet.copyOf(builder.removedLegacyItems);
+ this.removedBundles = ImmutableSet.copyOf(builder.removedBundles);
+ }
+
+ public boolean isEmpty() {
+ return removedLegacyItems.isEmpty() && removedBundles.isEmpty();
+ }
+
+ public Set<VersionRangedName> getRemovedLegacyItems() {
+ return removedLegacyItems;
+ }
+
+ public Set<VersionRangedName> getRemovedBundles() {
+ return removedBundles;
+ }
+
+ public boolean isLegacyItemRemoved(CatalogItem<?, ?> legacyCatalogItem) {
+ VersionedName name = new VersionedName(legacyCatalogItem.getSymbolicName(), legacyCatalogItem.getVersion());
+ return contains(removedLegacyItems, name);
+ }
+
+ public boolean isBundleRemoved(VersionedName bundle) {
+ return contains(removedBundles, bundle);
+ }
+
+ public boolean contains(Iterable<VersionRangedName> names, VersionedName name) {
+ for (VersionRangedName contender : names) {
+ if (contender.getSymbolicName().equals(name.getSymbolicName()) && contender.getOsgiVersionRange().includes(name.getOsgiVersion())) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+
+ /**
+ * Records a name (string) and version range (string),
+ * with conveniences for pretty-printing and converting to OSGi format.
+ *
+ * Implementation-wise, this is similar to {@link VersionedName}, but is intended
+ * as internal-only so is cut down to only what is needed.
+ */
+ public static class VersionRangedName {
+ private final String name;
+ private final String v;
+ private transient volatile VersionRange cachedOsgiVersionRange;
+
+ public static VersionRangedName fromString(String val, boolean singleVersionIsOsgiRange) {
+ if (Strings.isBlank(val)) {
+ throw new IllegalArgumentException("Must not be blank");
+ }
+ String[] parts = val.split(":");
+ if (parts.length > 2) {
+ throw new IllegalArgumentException("Identifier '"+val+"' has too many parts; max one ':' symbol");
+ }
+ if (parts.length == 1 || Strings.isBlank(parts[1])) {
+ throw new IllegalArgumentException("Identifier '"+val+"' must be of 'name:versionRange' syntax");
+ } else if (singleVersionIsOsgiRange || (parts[1].startsWith("(") || parts[1].startsWith("["))) {
+ return new VersionRangedName(parts[0], parts[1]);
+ } else {
+ return new VersionRangedName(parts[0], "["+parts[1]+","+parts[1]+"]");
+ }
+ }
+
+ public VersionRangedName(String name, VersionRange v) {
+ this.name = checkNotNull(name, "name").toString();
+ this.v = checkNotNull(v, "versionRange").toString();
+ }
+
+ private VersionRangedName(String name, String v) {
+ this.name = checkNotNull(name, "name");
+ this.v = checkNotNull(v, "versionRange");
+ }
+
+ @Override
+ public String toString() {
+ return name + ":" + v;
+ }
+
+ public String toOsgiString() {
+ return name + ":" + getOsgiVersionRange();
+ }
+
+ public String getSymbolicName() {
+ return name;
+ }
+
+ public VersionRange getOsgiVersionRange() {
+ if (cachedOsgiVersionRange == null) {
+ cachedOsgiVersionRange = VersionRange.valueOf(BrooklynVersionSyntax.toValidOsgiVersionRange(v));
+ }
+ return cachedOsgiVersionRange;
+ }
+
+ public String getVersionString() {
+ return v;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(name, v);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof VersionRangedName)) {
+ return false;
+ }
+ VersionRangedName o = (VersionRangedName) other;
+ return Objects.equal(name, o.name) && Objects.equal(v, o.v);
+ }
+ }
+
+ public static CatalogUpgrades parseBundleManifestForCatalogUpgrades(Bundle bundle) {
+ Dictionary<String, String> headers = bundle.getHeaders();
+ String removedLegacyItemsHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS);
+ String removedBundlesHeader = headers.get(MANIFEST_HEADER_FORCE_REMOVE_BUNDLES);
+ List<VersionRangedName> removedLegacyItems = ImmutableList.of();
+ List<VersionRangedName> removedBundles = ImmutableList.of();
+ if (removedLegacyItemsHeader == null && removedBundlesHeader == null) {
+ return CatalogUpgrades.EMPTY;
+ }
+ if (removedLegacyItemsHeader != null) {
+ removedLegacyItems = parseVersionRangedNameList(removedLegacyItemsHeader, false);
+ }
+ if (removedBundlesHeader != null) {
+ removedBundles = parseVersionRangedNameList(removedBundlesHeader, false);
+ }
+ return CatalogUpgrades.builder()
+ .removedLegacyItems(removedLegacyItems)
+ .removedBundles(removedBundles)
+ .build();
+ }
+
+ @VisibleForTesting
+ static List<VersionRangedName> parseVersionRangedNameList(String input, boolean singleVersionIsOsgiRange) {
+ List<String> vals = QuotedStringTokenizer.builder()
--- End diff --
judging by the tests i'm surprised this works for mismatched brackets, or at least require a boolean flag to allow mismatched brackets -- but your test indicates otherwise
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146491158
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java ---
@@ -528,8 +528,60 @@ private void startBundle(OsgiBundleInstallationResult br) throws BundleException
}
}
+ private PersistedCatalogState filterPersistedState(PersistedCatalogState persistedState, RebindLogger rebindLogger) {
--- End diff --
rename `filterBundlesAndCatalogInPersistedState` to clarify this isn't doing any other filtering or transforming instances
---
[GitHub] brooklyn-server issue #866: catalog-init: add support for item/bundle remova...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/866
retest this please
Failure is unrelated, and not obvious why it would fail non-deterministically! Needs to be investigated separately:
```
2017-10-26 23:12:19,940 INFO TESTNG INVOKING: "Surefire test" - org.apache.brooklyn.test.framework.LoopOverGroupMembersTestCaseTest.testMultipleChildrenWhichPass()
2017-10-26 23:12:20,019 WARN Problem in child test-case of LoopOverGroupMembersTestCaseImpl{id=d1jcas6xt9}, targetting EmptySoftwareProcessImpl{id=twialf39k2}
2017-10-26 23:12:20,024 WARN Error invoking start at LoopOverGroupMembersTestCaseImpl{id=d1jcas6xt9}: Test failed on group member(s)
2017-10-26 23:12:20,025 WARN Setting Application[joc36trf] on-fire due to problems when expected running, up=false, problems: {service-lifecycle-indicators-from-children-and-members=Required entity not healthy: LoopOverGroupMembersTestCase:d1jc (d1jcas6xt9)}
2017-10-26 23:12:20,025 WARN Setting Application[joc36trf] on-fire due to problems when expected running, up=false, problems: {service-lifecycle-indicators-from-children-and-members=Required entity not healthy: LoopOverGroupMembersTestCase:d1jc (d1jcas6xt9)}
2017-10-26 23:12:20,026 WARN Error invoking start at Application[joc36trf]: ProblemStartingChildrenException: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException: Error invoking start at LoopOverGroupMembersTestCaseImpl{id=d1jcas6xt9}: Test failed on group member(s)
2017-10-26 23:12:20,042 INFO TESTNG FAILED: "Surefire test" - org.apache.brooklyn.test.framework.LoopOverGroupMembersTestCaseTest.testMultipleChildrenWhichPass() finished in 89 ms
org.apache.brooklyn.core.mgmt.internal.EffectorUtils$EffectorCallPropagatedRuntimeException: Error invoking start at Application[joc36trf]: ProblemStartingChildrenException: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException: Error invoking start at LoopOverGroupMembersTestCaseImpl{id=d1jcas6xt9}: Test failed on group member(s)
org.apache.brooklyn.core.mgmt.internal.EffectorUtils$EffectorCallPropagatedRuntimeException: Error invoking start at Application[joc36trf]
at org.apache.brooklyn.test.framework.LoopOverGroupMembersTestCaseTest.testMultipleChildrenWhichPass(LoopOverGroupMembersTestCaseTest.java:99)
Caused by: java.util.concurrent.ExecutionException: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException: ProblemStartingChildrenException: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException: Error invoking start at LoopOverGroupMembersTestCaseImpl{id=d1jcas6xt9}: Test failed on group member(s)
at org.apache.brooklyn.test.framework.LoopOverGroupMembersTestCaseTest.testMultipleChildrenWhichPass(LoopOverGroupMembersTestCaseTest.java:99)
Caused by: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException:
at org.apache.brooklyn.test.framework.LoopOverGroupMembersTestCaseTest.testMultipleChildrenWhichPass(LoopOverGroupMembersTestCaseTest.java:99)
Caused by: java.lang.reflect.InvocationTargetException
at org.apache.brooklyn.test.framework.LoopOverGroupMembersTestCaseTest.testMultipleChildrenWhichPass(LoopOverGroupMembersTestCaseTest.java:99)
Caused by: org.apache.brooklyn.core.entity.AbstractApplication$ProblemStartingChildrenException: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException: Error invoking start at LoopOverGroupMembersTestCaseImpl{id=d1jcas6xt9}: Test failed on group member(s)
at org.apache.brooklyn.test.framework.LoopOverGroupMembersTestCaseTest.testMultipleChildrenWhichPass(LoopOverGroupMembersTestCaseTest.java:99)
Caused by: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException:
at org.apache.brooklyn.test.framework.LoopOverGroupMembersTestCaseTest.testMultipleChildrenWhichPass(LoopOverGroupMembersTestCaseTest.java:99)
Caused by: java.util.concurrent.ExecutionException: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException: Error invoking start at LoopOverGroupMembersTestCaseImpl{id=d1jcas6xt9}: Test failed on group member(s)
at org.apache.brooklyn.test.framework.LoopOverGroupMembersTestCaseTest.testMultipleChildrenWhichPass(LoopOverGroupMembersTestCaseTest.java:99)
Caused by: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException:
Caused by: java.util.concurrent.ExecutionException: org.apache.brooklyn.core.mgmt.internal.EffectorUtils$EffectorCallPropagatedRuntimeException: Error invoking start at LoopOverGroupMembersTestCaseImpl{id=d1jcas6xt9}: Test failed on group member(s)
Caused by: org.apache.brooklyn.core.mgmt.internal.EffectorUtils$EffectorCallPropagatedRuntimeException: Error invoking start at LoopOverGroupMembersTestCaseImpl{id=d1jcas6xt9}
Caused by: java.util.concurrent.ExecutionException: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException: Test failed on group member(s)
Caused by: org.apache.brooklyn.util.exceptions.PropagatedRuntimeException:
Caused by: java.lang.reflect.InvocationTargetException
Caused by: org.apache.brooklyn.util.exceptions.CompoundRuntimeException: Test failed on group member(s)
```
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/866
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146486564
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BundleUpgradeParser.java ---
@@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+package org.apache.brooklyn.core.catalog.internal;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Dictionary;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.util.osgi.VersionedName;
+import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
+import org.apache.brooklyn.util.text.QuotedStringTokenizer;
+import org.apache.brooklyn.util.text.Strings;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.VersionRange;
+
+import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+public class BundleUpgradeParser {
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS = "brooklyn-catalog-force-remove-legacy-items";
+
+ @Beta
+ public static final String MANIFEST_HEADER_FORCE_REMOVE_BUNDLES = "brooklyn-catalog-force-remove-bundles";
+
+ @Beta
+ public static final String MANIFEST_HEADER_UPGRADE_BUNDLES = "brooklyn-catalog-upgrade-for-bundles";
+
+ /**
+ * The result from parsing bundle(s) to find their upgrade info.
+ */
+ public static class CatalogUpgrades {
+ static final CatalogUpgrades EMPTY = new CatalogUpgrades(builder());
+
+ static class Builder {
+ private Set<VersionRangedName> removedLegacyItems = new LinkedHashSet<>();
+ private Set<VersionRangedName> removedBundles = new LinkedHashSet<>();
+
+ public Builder removedLegacyItems(Collection<VersionRangedName> vals) {
+ removedLegacyItems.addAll(vals);
+ return this;
+ }
+ public Builder removedBundles(Collection<VersionRangedName> vals) {
+ removedBundles.addAll(vals);
+ return this;
+ }
+ public Builder addAll(CatalogUpgrades other) {
+ removedLegacyItems.addAll(other.removedLegacyItems);
+ removedBundles.addAll(other.removedBundles);
+ return this;
+ }
+ public CatalogUpgrades build() {
+ return new CatalogUpgrades(this);
+ }
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ private Set<VersionRangedName> removedLegacyItems;
+ private Set<VersionRangedName> removedBundles;
+
+ public CatalogUpgrades(Builder builder) {
+ this.removedLegacyItems = ImmutableSet.copyOf(builder.removedLegacyItems);
+ this.removedBundles = ImmutableSet.copyOf(builder.removedBundles);
+ }
+
+ public boolean isEmpty() {
+ return removedLegacyItems.isEmpty() && removedBundles.isEmpty();
+ }
+
+ public Set<VersionRangedName> getRemovedLegacyItems() {
+ return removedLegacyItems;
+ }
+
+ public Set<VersionRangedName> getRemovedBundles() {
+ return removedBundles;
+ }
+
+ public boolean isLegacyItemRemoved(CatalogItem<?, ?> legacyCatalogItem) {
+ VersionedName name = new VersionedName(legacyCatalogItem.getSymbolicName(), legacyCatalogItem.getVersion());
+ return contains(removedLegacyItems, name);
+ }
+
+ public boolean isBundleRemoved(VersionedName bundle) {
+ return contains(removedBundles, bundle);
+ }
+
+ public boolean contains(Iterable<VersionRangedName> names, VersionedName name) {
+ for (VersionRangedName contender : names) {
+ if (contender.getSymbolicName().equals(name.getSymbolicName()) && contender.getOsgiVersionRange().includes(name.getOsgiVersion())) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+
+ /**
+ * Records a name (string) and version range (string),
+ * with conveniences for pretty-printing and converting to OSGi format.
+ *
+ * Implementation-wise, this is similar to {@link VersionedName}, but is intended
+ * as internal-only so is cut down to only what is needed.
+ */
+ public static class VersionRangedName {
+ private final String name;
+ private final String v;
+ private transient volatile VersionRange cachedOsgiVersionRange;
+
+ public static VersionRangedName fromString(String val, boolean singleVersionIsOsgiRange) {
+ if (Strings.isBlank(val)) {
+ throw new IllegalArgumentException("Must not be blank");
+ }
+ String[] parts = val.split(":");
+ if (parts.length > 2) {
+ throw new IllegalArgumentException("Identifier '"+val+"' has too many parts; max one ':' symbol");
+ }
+ if (parts.length == 1 || Strings.isBlank(parts[1])) {
+ throw new IllegalArgumentException("Identifier '"+val+"' must be of 'name:versionRange' syntax");
--- End diff --
worth noting that range is required by this parser, and in docs we need to specify where ranges are required and where they are optional
---
[GitHub] brooklyn-server issue #866: catalog-init: add support for item/bundle remova...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/866
@aledsage ... rats:
```
[WARNING] Files with unapproved licenses:
/home/jenkins/jenkins-slave/workspace/brooklyn-server-pull-requests/core/src/test/java/org/apache/brooklyn/core/catalog/internal/BundleUpgradeParserTest.java
/home/jenkins/jenkins-slave/workspace/brooklyn-server-pull-requests/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BundleUpgradeParser.java
```
---
[GitHub] brooklyn-server issue #866: catalog-init: add support for item/bundle remova...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/866
looks good apart from comments above. good set of tests.
one other area i've wondered about is snapshot versions. what _is_ the behaviour and what _should_ it be?
OT1H if i say `remove=foo:[1,2)` i'd expect `1.5-SNAPSHOT` to be removed, and `remove=foo[0,2)` for `0.SNAPSHOT` to be removed, but `3-SNAPSHOT` not removed in both cases. not sure what the behaviour should be fore `2-SNAPSHOT` and `2-GA`, probably not removed. OTOH `*-SNAPSHOT` is less than `0` by brooklyn version ordering so there is reason to expect that none are removed.
as far as use cases, i think most common will be to say `remove all 1.* but not any 2.*` (suggested behaviour above). however that's very hard if we take a strict semver ordering (where `2.X` is less than `2` but bigger than any `1.*`) or a brooklyn ordering (where `*-SNAPSHOT` less than `0`). osgi ordering might have the right properties -- i forget.
worth updating `versioning.md` docs to describe this. also need docs on removing/upgrading but expect that will come once part two of this work, upgrading types, is done.
---
[GitHub] brooklyn-server issue #866: catalog-init: add support for item/bundle remova...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/866
retest this please
---
[GitHub] brooklyn-server pull request #866: catalog-init: add support for item/bundle...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/866#discussion_r146702938
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java ---
@@ -528,8 +528,60 @@ private void startBundle(OsgiBundleInstallationResult br) throws BundleException
}
}
+ private PersistedCatalogState filterPersistedState(PersistedCatalogState persistedState, RebindLogger rebindLogger) {
+ CatalogUpgrades catalogUpgrades = findCatalogUpgrades(rebindLogger);
--- End diff --
Absolutely - intent is to first support these first two options. When we then add support for `brooklyn-catalog-upgrade-for-bundles` and `brooklyn-catalog-upgrade-for-types, the signature of this private method will no doubt change.
---
[GitHub] brooklyn-server issue #866: catalog-init: add support for item/bundle remova...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/866
quite right @aledsage - i was thinking we needed to look at the types to ensure removal but of course that's neatly separate. i'm on it. this side of it turns out nice and simple, hurrah, and i can extend your test cases!
---