You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2022/06/27 08:06:49 UTC
[brooklyn-server] branch master updated: address code review on PR 1322
This is an automated email from the ASF dual-hosted git repository.
heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
The following commit(s) were added to refs/heads/master by this push:
new 48c619673e address code review on PR 1322
48c619673e is described below
commit 48c619673e6ec87ce894d4628487e8f35d0573b0
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Mon Jun 27 09:06:34 2022 +0100
address code review on PR 1322
---
.../mgmt/ha/BrooklynBomOsgiArchiveInstaller.java | 32 ++++++++++++----------
.../typereg/BrooklynCatalogBundleResolvers.java | 8 ++----
.../brooklyn/util/core/file/ArchiveBuilder.java | 1 -
3 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java
index 8aa20f603a..fc1440738d 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java
@@ -99,15 +99,19 @@ public class BrooklynBomOsgiArchiveInstaller {
public static class FileWithTempInfo<T extends File> {
public FileWithTempInfo(T f, boolean isTemp) { this.f = f; this.isTemp = isTemp; }
- public T f;
+ private T f;
public boolean isTemp;
public void deleteIfTemp() {
- if (isTemp && f!=null) {
- f.delete();
+ if (isTemp && getFile() !=null) {
+ getFile().delete();
f = null;
}
}
+
+ public T getFile() {
+ return f;
+ }
}
private FileWithTempInfo<File> zipFile;
@@ -356,9 +360,9 @@ public class BrooklynBomOsgiArchiveInstaller {
} else {
prepareInstallResult.zipFile = new FileWithTempInfo<File>( Os.newTempFile("brooklyn-bundle-transient-" + suppliedKnownBundleMetadata, "zip"), true );
try {
- FileOutputStream fos = new FileOutputStream(prepareInstallResult.zipFile.f);
+ FileOutputStream fos = new FileOutputStream(prepareInstallResult.zipFile.getFile());
Streams.copyClose(zipIn, fos);
- try (ZipFile zf = new ZipFile(prepareInstallResult.zipFile.f)) {
+ try (ZipFile zf = new ZipFile(prepareInstallResult.zipFile.getFile())) {
// validate it is a valid ZIP, otherwise errors are more obscure later.
// can happen esp if user supplies a file://path/to/folder/ as the URL.openStream returns a list of that folder (!)
// the error thrown by the below is useful enough, and caller will wrap with suppliedKnownBundleMetadata details
@@ -384,7 +388,7 @@ public class BrooklynBomOsgiArchiveInstaller {
}
private void discoverManifestFromCatalogBom(boolean isCatalogBomRequired) {
- discoveredManifest = new BundleMaker(mgmt()).getManifest(zipFile.f);
+ discoveredManifest = new BundleMaker(mgmt()).getManifest(zipFile.getFile());
if (Strings.isNonBlank(bomText)) {
discoveredBomVersionedName = BasicBrooklynCatalog.getVersionedName(BasicBrooklynCatalog.getCatalogMetadata(bomText), false );
@@ -394,7 +398,7 @@ public class BrooklynBomOsgiArchiveInstaller {
ZipFile zf = null;
try {
try {
- zf = new ZipFile(zipFile.f);
+ zf = new ZipFile(zipFile.getFile());
} catch (IOException e) {
throw new IllegalArgumentException("Invalid ZIP/JAR archive: "+e);
}
@@ -453,7 +457,7 @@ public class BrooklynBomOsgiArchiveInstaller {
manifestNeedsUpdating = true;
}
if (manifestNeedsUpdating) {
- File zf2 = new BundleMaker(mgmt()).copyAddingManifest(zipFile.f, discoveredManifest);
+ File zf2 = new BundleMaker(mgmt()).copyAddingManifest(zipFile.getFile(), discoveredManifest);
zipFile.deleteIfTemp();
zipFile = new FileWithTempInfo<>(zf2, zipFile.isTemp);
}
@@ -495,7 +499,7 @@ public class BrooklynBomOsgiArchiveInstaller {
if (result.code!=null) return ReferenceWithError.newInstanceWithoutError(result);
assert inferredMetadata.isNameResolved() : "Should have resolved "+inferredMetadata;
assert inferredMetadata instanceof BasicManagedBundle : "Only BasicManagedBundles supported";
- ((BasicManagedBundle)inferredMetadata).setChecksum(getChecksum(new ZipFile(zipFile.f)));
+ ((BasicManagedBundle)inferredMetadata).setChecksum(getChecksum(new ZipFile(zipFile.getFile())));
((BasicManagedBundle)inferredMetadata).setFormat(format);
final boolean updating;
@@ -533,7 +537,7 @@ public class BrooklynBomOsgiArchiveInstaller {
List<Bundle> matchingVsnBundles = findBundlesBySymbolicNameAndVersion(osgiManager, inferredMetadata);
- List<Bundle> sameContentBundles = matchingVsnBundles.stream().filter(b -> isBundleSameOsgiUrlOrSameContents(b, inferredMetadata, zipFile.f)).collect(Collectors.toList());
+ List<Bundle> sameContentBundles = matchingVsnBundles.stream().filter(b -> isBundleSameOsgiUrlOrSameContents(b, inferredMetadata, zipFile.getFile())).collect(Collectors.toList());
if (!sameContentBundles.isEmpty()) {
// e.g. happens if pre-installed bundle is brought under management, and then add it again via a mvn-style url.
// We wouldn't know the checksum from the pre-installed bundle, the osgi locations might be different,
@@ -601,7 +605,7 @@ public class BrooklynBomOsgiArchiveInstaller {
// search for already-installed bundles.
List<Bundle> existingBundles = findBundlesBySymbolicNameAndVersion(osgiManager, inferredMetadata);
- Maybe<Bundle> existingEquivalentBundle = tryFindSameOsgiUrlOrSameContentsBundle(existingBundles, inferredMetadata, zipFile.f);
+ Maybe<Bundle> existingEquivalentBundle = tryFindSameOsgiUrlOrSameContentsBundle(existingBundles, inferredMetadata, zipFile.getFile());
if (existingEquivalentBundle.isPresent()) {
// Identical bundle (by osgi location or binary content) already installed; just bring that under management.
@@ -647,7 +651,7 @@ public class BrooklynBomOsgiArchiveInstaller {
}
startedInstallation = true;
- try (InputStream fin = new FileInputStream(zipFile.f)) {
+ try (InputStream fin = new FileInputStream(zipFile.getFile())) {
if (!updating) {
if (isBringingExistingOsgiInstalledBundleUnderBrooklynManagement) {
assert result.getBundle()!=null;
@@ -669,11 +673,11 @@ public class BrooklynBomOsgiArchiveInstaller {
if (!updating) {
oldZipFile = null;
oldManagedBundle = null;
- osgiManager.managedBundlesRecord.addManagedBundle(result, zipFile.f);
+ osgiManager.managedBundlesRecord.addManagedBundle(result, zipFile.getFile());
result.code = OsgiBundleInstallationResult.ResultCode.INSTALLED_NEW_BUNDLE;
result.message = "Installed Brooklyn catalog bundle "+result.getMetadata().getVersionedName()+" with ID "+result.getMetadata().getId()+" ["+result.bundle.getBundleId()+"]";
} else {
- Pair<File, ManagedBundle> olds = osgiManager.managedBundlesRecord.updateManagedBundleFileAndMetadata(result, zipFile.f);
+ Pair<File, ManagedBundle> olds = osgiManager.managedBundlesRecord.updateManagedBundleFileAndMetadata(result, zipFile.getFile());
oldZipFile = olds.getLeft();
oldManagedBundle = olds.getRight();
diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynCatalogBundleResolvers.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynCatalogBundleResolvers.java
index 54050549c4..4b9548e0d3 100644
--- a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynCatalogBundleResolvers.java
+++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynCatalogBundleResolvers.java
@@ -18,10 +18,9 @@
*/
package org.apache.brooklyn.core.typereg;
-import com.google.common.annotations.Beta;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.*;
-import java.io.File;
+
import java.io.InputStream;
import java.util.*;
import java.util.Map.Entry;
@@ -29,8 +28,6 @@ import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.apache.brooklyn.api.framework.FrameworkLookup;
import org.apache.brooklyn.api.mgmt.ManagementContext;
-import org.apache.brooklyn.api.typereg.RegisteredType;
-import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext;
import org.apache.brooklyn.core.mgmt.entitlement.Entitlements;
import org.apache.brooklyn.core.mgmt.ha.BrooklynBomOsgiArchiveInstaller;
import org.apache.brooklyn.core.mgmt.ha.BrooklynBomOsgiArchiveInstaller.PrepareInstallResult;
@@ -38,7 +35,6 @@ import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult;
import org.apache.brooklyn.core.typereg.BrooklynCatalogBundleResolver.BundleInstallationOptions;
import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.core.ResourceUtils;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.exceptions.PropagatedRuntimeException;
import org.apache.brooklyn.util.exceptions.ReferenceWithError;
@@ -124,7 +120,7 @@ public class BrooklynCatalogBundleResolvers {
}
if (prepareResult.zipFile != null) {
- input = InputStreamSource.of(prepareResult.zipFile.f.getName(), prepareResult.zipFile.f);
+ input = InputStreamSource.of(prepareResult.zipFile.getFile().getName(), prepareResult.zipFile.getFile());
} else {
return ReferenceWithError.newInstanceThrowingError(null, new IllegalArgumentException("Bundle contents or known reference must be supplied; " +
options.knownBundleMetadata + " not known"));
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/file/ArchiveBuilder.java b/core/src/main/java/org/apache/brooklyn/util/core/file/ArchiveBuilder.java
index d1f1bf6761..e15f78b5dd 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/file/ArchiveBuilder.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/file/ArchiveBuilder.java
@@ -44,7 +44,6 @@ import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Multimap;
import com.google.common.io.Files;
-import org.apache.brooklyn.util.time.Duration;
/**
* Build a Zip or Jar archive.