diff options
95 files changed, 785 insertions, 719 deletions
diff --git a/bundle-plugin-test/src/test/java/com/yahoo/BundleIT.java b/bundle-plugin-test/src/test/java/com/yahoo/BundleIT.java index 5b4fdda06f7..932b78a5c8d 100644 --- a/bundle-plugin-test/src/test/java/com/yahoo/BundleIT.java +++ b/bundle-plugin-test/src/test/java/com/yahoo/BundleIT.java @@ -18,6 +18,7 @@ import java.util.jar.JarFile; import java.util.jar.Manifest; import java.util.zip.ZipEntry; +import static com.yahoo.osgi.maven.ProjectBundleClassPaths.CLASSPATH_MAPPINGS_FILENAME; import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.CoreMatchers.anyOf; import static org.hamcrest.CoreMatchers.containsString; @@ -143,9 +144,9 @@ public class BundleIT { @SuppressWarnings("unchecked") @Test public void bundle_class_path_mappings_are_generated() throws URISyntaxException, IOException { - URL mappingsUrl = getClass().getResource("/" + com.yahoo.osgi.maven.ProjectBundleClassPaths.CLASSPATH_MAPPINGS_FILENAME); + URL mappingsUrl = getClass().getResource("/" + CLASSPATH_MAPPINGS_FILENAME); assertNotNull( - "Could not find " + ProjectBundleClassPaths.CLASSPATH_MAPPINGS_FILENAME + " in the test output directory", + "Could not find " + CLASSPATH_MAPPINGS_FILENAME + " in the test output directory", mappingsUrl); ProjectBundleClassPaths bundleClassPaths = ProjectBundleClassPaths.load(Paths.get(mappingsUrl.toURI())); diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java index 798fea2644e..0626c786822 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java @@ -11,10 +11,14 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.jar.Manifest; import java.util.stream.Collectors; /** + * "Public package" in this context means a package that is either exported or declared as "Global-Package" + * in a bundle's manifest. + * * @author Tony Vaagenes * @author ollivir */ @@ -23,10 +27,18 @@ public class AnalyzeBundle { public final List<Export> exports; public final List<String> globals; - public PublicPackages(List<Export> exports, List<String> globals) { + PublicPackages(List<Export> exports, List<String> globals) { this.exports = exports; this.globals = globals; } + + public Set<String> exportedPackageNames() { + return exports.stream() + .map(Export::getPackageNames) + .flatMap(Collection::stream) + .collect(Collectors.toSet()); + } + } public static PublicPackages publicPackagesAggregated(Collection<File> jarFiles) { diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/classanalysis/PackageTally.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/classanalysis/PackageTally.java index 13bbc63192c..9f5fd2236e7 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/classanalysis/PackageTally.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/classanalysis/PackageTally.java @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.plugin.classanalysis; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Sets; import com.yahoo.container.plugin.util.Maps; @@ -10,6 +11,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; /** * @author Tony Vaagenes @@ -19,7 +21,8 @@ public class PackageTally { private final Map<String, Optional<ExportPackageAnnotation>> definedPackagesMap; private final Set<String> referencedPackagesUnfiltered; - public PackageTally(Map<String, Optional<ExportPackageAnnotation>> definedPackagesMap, Set<String> referencedPackagesUnfiltered) { + @VisibleForTesting + PackageTally(Map<String, Optional<ExportPackageAnnotation>> definedPackagesMap, Set<String> referencedPackagesUnfiltered) { this.definedPackagesMap = definedPackagesMap; this.referencedPackagesUnfiltered = referencedPackagesUnfiltered; } @@ -41,6 +44,19 @@ public class PackageTally { } /** + * Returns the set of packages that is referenced from this tally, but not included in the given set of available packages. + * + * @param definedAndExportedPackages Set of available packages (usually all packages defined in the generated bundle's project + all exported packages of dependencies) + * @return The set of missing packages, that may cause problem when the bundle is deployed in an OSGi container runtime. + */ + public Set<String> referencedPackagesMissingFrom(Set<String> definedAndExportedPackages) { + return Sets.difference(referencedPackages(), definedAndExportedPackages).stream() + .filter(pkg -> !pkg.startsWith("java.")) + .filter(pkg -> !pkg.equals(com.yahoo.api.annotations.PublicApi.class.getPackageName())) + .collect(Collectors.toSet()); + } + + /** * Represents the classes for two package tallies that are deployed as a single unit. * <p> * ExportPackageAnnotations from this has precedence over the other. diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/Artifacts.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/Artifacts.java index fff88d413d0..a0d0e143724 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/Artifacts.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/Artifacts.java @@ -12,8 +12,8 @@ import java.util.List; * @author Tony Vaagenes * @author ollivir */ -public class Artifacts { - public static class ArtifactSet { +class Artifacts { + static class ArtifactSet { private final List<Artifact> jarArtifactsToInclude; private final List<Artifact> jarArtifactsProvided; private final List<Artifact> nonJarArtifacts; @@ -24,20 +24,20 @@ public class Artifacts { this.nonJarArtifacts = nonJarArtifacts; } - public List<Artifact> getJarArtifactsToInclude() { + List<Artifact> getJarArtifactsToInclude() { return jarArtifactsToInclude; } - public List<Artifact> getJarArtifactsProvided() { + List<Artifact> getJarArtifactsProvided() { return jarArtifactsProvided; } - public List<Artifact> getNonJarArtifacts() { + List<Artifact> getNonJarArtifacts() { return nonJarArtifacts; } } - public static ArtifactSet getArtifacts(MavenProject project) { + static ArtifactSet getArtifacts(MavenProject project) { List<Artifact> jarArtifactsToInclude = new ArrayList<>(); List<Artifact> jarArtifactsProvided = new ArrayList<>(); @@ -63,7 +63,7 @@ public class Artifacts { return new ArtifactSet(jarArtifactsToInclude, jarArtifactsProvided, nonJarArtifactsToInclude); } - public static Collection<Artifact> getArtifactsToInclude(MavenProject project) { + static Collection<Artifact> getArtifactsToInclude(MavenProject project) { return getArtifacts(project).getJarArtifactsToInclude(); } } diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java index 9c9957ae718..04f2428d284 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java @@ -10,14 +10,12 @@ import com.yahoo.container.plugin.classanalysis.PackageTally; import com.yahoo.container.plugin.osgi.ExportPackageParser; import com.yahoo.container.plugin.osgi.ExportPackages; import com.yahoo.container.plugin.osgi.ExportPackages.Export; -import com.yahoo.container.plugin.osgi.ImportPackages; import com.yahoo.container.plugin.osgi.ImportPackages.Import; import com.yahoo.container.plugin.util.Strings; import org.apache.commons.lang3.tuple.Pair; import org.apache.maven.artifact.Artifact; import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; -import org.apache.maven.plugin.MojoFailureException; import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; import org.apache.maven.plugins.annotations.ResolutionScope; @@ -41,6 +39,9 @@ import java.util.jar.Manifest; import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.yahoo.container.plugin.bundle.AnalyzeBundle.publicPackagesAggregated; +import static com.yahoo.container.plugin.osgi.ExportPackages.exportsByPackageName; +import static com.yahoo.container.plugin.osgi.ImportPackages.calculateImports; import static com.yahoo.container.plugin.util.Files.allDescendantFiles; import static com.yahoo.container.plugin.util.IO.withFileOutputStream; import static com.yahoo.container.plugin.util.JarFiles.withInputStream; @@ -90,32 +91,43 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { @Parameter(alias = "X-Jersey-Binding") private String jerseyBinding = null; - public void execute() throws MojoExecutionException, MojoFailureException { + public void execute() throws MojoExecutionException { try { Artifacts.ArtifactSet artifactSet = Artifacts.getArtifacts(project); warnOnUnsupportedArtifacts(artifactSet.getNonJarArtifacts()); - AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars = AnalyzeBundle.publicPackagesAggregated( + // Packages from Export-Package and Global-Package headers in provided scoped jars + AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars = publicPackagesAggregated( artifactSet.getJarArtifactsProvided().stream().map(Artifact::getFile).collect(Collectors.toList())); - PackageTally includedJarPackageTally = definedPackages(artifactSet.getJarArtifactsToInclude()); - PackageTally projectPackageTally = analyzeProjectClasses(); - PackageTally pluginPackageTally = projectPackageTally.combine(includedJarPackageTally); + // Packages from Export-Package headers in provided scoped jars + Set<String> exportedPackagesFromProvidedDeps = publicPackagesFromProvidedJars.exportedPackageNames(); - Set<String> definedPackages = new HashSet<>(projectPackageTally.definedPackages()); - definedPackages.addAll(includedJarPackageTally.definedPackages()); + // Packaged defined in this project's code + PackageTally projectPackages = getProjectClassesTally(); - warnIfPackagesDefinedOverlapsGlobalPackages(definedPackages, publicPackagesFromProvidedJars.globals); + // Packages defined in compile scoped jars + PackageTally compileJarsPackages = definedPackages(artifactSet.getJarArtifactsToInclude()); - if (getLog().isDebugEnabled()) { - getLog().debug("Referenced packages = " + pluginPackageTally.referencedPackages()); - getLog().debug("Defined packages = " + pluginPackageTally.definedPackages()); - getLog().debug("Exported packages of dependencies = " + publicPackagesFromProvidedJars.exports.stream() - .map(e -> "(" + e.getPackageNames().toString() + ", " + e.version().orElse("")).collect(Collectors.joining(", "))); + // The union of packages in the project and compile scoped jars + PackageTally includedPackages = projectPackages.combine(compileJarsPackages); + + warnIfPackagesDefinedOverlapsGlobalPackages(includedPackages.definedPackages(), publicPackagesFromProvidedJars.globals); + logDebugPackageSets(publicPackagesFromProvidedJars, includedPackages); + + if (hasJdiscCoreProvided(artifactSet.getJarArtifactsProvided())) { + // jdisc_core being provided guarantees that log output does not contain its exported packages + logMissingPackages(exportedPackagesFromProvidedDeps, projectPackages, compileJarsPackages, includedPackages); + } else { + getLog().warn("This project does not have jdisc_core as provided dependency, so the " + + "generated 'Import-Package' OSGi header may be missing important packages."); } + logOverlappingPackages(projectPackages, exportedPackagesFromProvidedDeps); + logUnnecessaryPackages(compileJarsPackages, exportedPackagesFromProvidedDeps); - Map<String, Import> calculatedImports = ImportPackages.calculateImports(pluginPackageTally.referencedPackages(), - pluginPackageTally.definedPackages(), ExportPackages.exportsByPackageName(publicPackagesFromProvidedJars.exports)); + Map<String, Import> calculatedImports = calculateImports(includedPackages.referencedPackages(), + includedPackages.definedPackages(), + exportsByPackageName(publicPackagesFromProvidedJars.exports)); Map<String, Optional<String>> manualImports = emptyToNone(importPackage).map(GenerateOsgiManifestMojo::getManualImports) .orElseGet(HashMap::new); @@ -123,17 +135,74 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { calculatedImports.remove(packageName); } createManifestFile(new File(project.getBuild().getOutputDirectory()), manifestContent(project, - artifactSet.getJarArtifactsToInclude(), manualImports, calculatedImports.values(), pluginPackageTally)); + artifactSet.getJarArtifactsToInclude(), manualImports, calculatedImports.values(), includedPackages)); } catch (Exception e) { - throw new MojoExecutionException("Failed generating osgi manifest.", e); + throw new MojoExecutionException("Failed generating osgi manifest", e); + } + } + + private void logDebugPackageSets(AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars, PackageTally includedPackages) { + if (getLog().isDebugEnabled()) { + getLog().debug("Referenced packages = " + includedPackages.referencedPackages()); + getLog().debug("Defined packages = " + includedPackages.definedPackages()); + getLog().debug("Exported packages of dependencies = " + publicPackagesFromProvidedJars.exports.stream() + .map(e -> "(" + e.getPackageNames().toString() + ", " + e.version().orElse("")).collect(Collectors.joining(", "))); + } + } + + private boolean hasJdiscCoreProvided(List<Artifact> providedArtifacts) { + return providedArtifacts.stream().anyMatch(artifact -> artifact.getArtifactId().equals("jdisc_core")); + } + + private void logMissingPackages(Set<String> exportedPackagesFromProvidedJars, + PackageTally projectPackages, + PackageTally compileJarPackages, + PackageTally includedPackages) { + + Set<String> definedAndExportedPackages = Sets.union(includedPackages.definedPackages(), exportedPackagesFromProvidedJars); + + Set<String> missingProjectPackages = projectPackages.referencedPackagesMissingFrom(definedAndExportedPackages); + if (! missingProjectPackages.isEmpty()) { + getLog().warn("Packages unavailable runtime are referenced from project classes " + + "(annotations can usually be ignored): " + missingProjectPackages); + } + + Set<String> missingCompilePackages = compileJarPackages.referencedPackagesMissingFrom(definedAndExportedPackages); + if (! missingCompilePackages.isEmpty()) { + getLog().info("Packages unavailable runtime are referenced from compile scoped jars " + + "(annotations can usually be ignored): " + missingCompilePackages); + } + } + + private void logOverlappingPackages(PackageTally projectPackages, + Set<String> exportedPackagesFromProvidedDeps) { + Set<String> overlappingProjectPackages = Sets.intersection(projectPackages.definedPackages(), exportedPackagesFromProvidedDeps); + if (! overlappingProjectPackages.isEmpty()) { + getLog().warn("Project classes use the following packages that are already defined in provided scoped dependencies: " + + overlappingProjectPackages); + } + } + + /* + * This mostly detects packages re-exported via composite bundles like jdisc_core and container-disc. + * An artifact can only be represented once, either in compile or provided scope. So if the project + * adds an artifact in compile scope that we deploy as a pre-installed bundle, we won't see the same + * artifact as provided via container-dev and hence can't detect the duplicate packages. + */ + private void logUnnecessaryPackages(PackageTally compileJarsPackages, + Set<String> exportedPackagesFromProvidedDeps) { + Set<String> unnecessaryPackages = Sets.intersection(compileJarsPackages.definedPackages(), exportedPackagesFromProvidedDeps); + if (! unnecessaryPackages.isEmpty()) { + getLog().info("Compile scoped jars contain the following packages that are most likely " + + "available from jdisc runtime: " + unnecessaryPackages); } } private static void warnIfPackagesDefinedOverlapsGlobalPackages(Set<String> internalPackages, List<String> globalPackages) throws MojoExecutionException { Set<String> overlap = Sets.intersection(internalPackages, new HashSet<>(globalPackages)); - if (overlap.isEmpty() == false) { + if (! overlap.isEmpty()) { throw new MojoExecutionException( "The following packages are both global and included in the bundle:\n " + String.join("\n ", overlap)); } @@ -173,7 +242,7 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { Pair.of("WebInfUrl", webInfUrl), // Pair.of("Import-Package", importPackage), // Pair.of("Export-Package", exportPackage))) { - if (element.getValue() != null && element.getValue().isEmpty() == false) { + if (element.getValue() != null && ! element.getValue().isEmpty()) { ret.put(element.getKey(), element.getValue()); } } @@ -232,7 +301,7 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { } private void warnOnUnsupportedArtifacts(Collection<Artifact> nonJarArtifacts) { - List<Artifact> unsupportedArtifacts = nonJarArtifacts.stream().filter(a -> "pom".equals(a.getType()) == false) + List<Artifact> unsupportedArtifacts = nonJarArtifacts.stream().filter(a -> ! a.getType().equals("pom")) .collect(Collectors.toList()); unsupportedArtifacts.forEach(artifact -> getLog() @@ -240,7 +309,7 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { artifact.getId(), artifact.getType()))); } - private PackageTally analyzeProjectClasses() { + private PackageTally getProjectClassesTally() { File outputDirectory = new File(project.getBuild().getOutputDirectory()); List<ClassFileMetaData> analyzedClasses = allDescendantFiles(outputDirectory).filter(file -> file.getName().endsWith(".class")) @@ -258,7 +327,7 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { List<ClassFileMetaData> analyzedClasses = new ArrayList<>(); for (Enumeration<JarEntry> entries = jarFile.entries(); entries.hasMoreElements();) { JarEntry entry = entries.nextElement(); - if (entry.isDirectory() == false && entry.getName().endsWith(".class")) { + if (! entry.isDirectory() && entry.getName().endsWith(".class")) { analyzedClasses.add(analyzeClass(jarFile, entry)); } } @@ -305,10 +374,7 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { } private static Optional<String> emptyToNone(String str) { - return Optional.ofNullable(str).map(String::trim).filter(s -> s.isEmpty() == false); + return Optional.ofNullable(str).map(String::trim).filter(s -> ! s.isEmpty()); } - private static boolean isClassToAnalyze(String name) { - return name.endsWith(".class") && name.endsWith("module-info.class") == false; - } } diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ImportPackages.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ImportPackages.java index b58248ec4a6..a3031cb7ad7 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ImportPackages.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ImportPackages.java @@ -74,8 +74,9 @@ public class ImportPackages { } } - public static Map<String, Import> calculateImports(Set<String> referencedPackages, Set<String> implementedPackages, - Map<String, ExportPackages.Export> exportedPackages) { + public static Map<String, Import> calculateImports(Set<String> referencedPackages, + Set<String> implementedPackages, + Map<String, ExportPackages.Export> exportedPackages) { Map<String, Import> ret = new HashMap<>(); for (String undefinedPackage : Sets.difference(referencedPackages, implementedPackages)) { ExportPackages.Export export = exportedPackages.get(undefinedPackage); diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/util/JdkPackages.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/util/JdkPackages.java new file mode 100644 index 00000000000..0786272bc70 --- /dev/null +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/util/JdkPackages.java @@ -0,0 +1,39 @@ +package com.yahoo.container.plugin.util; + +import java.net.URL; + +/** + * @author gjoranv + */ +public class JdkPackages { + + /** + * Returns a boolean indicating (via best effort) if the given package is part of the JDK. + */ + public static boolean isJdkPackage(String pkg) { + return hasJdkExclusivePrefix(pkg) + || isResourceInPlatformClassLoader(pkg); // TODO: must be a class, not a package, due to module encapsulation + } + + private static boolean isResourceInPlatformClassLoader(String klass) { + String klassAsPath = klass.replaceAll("\\.", "/") + ".class"; + URL resource = getPlatformClassLoader().getResource(klassAsPath); + return !(resource == null); + } + + private static ClassLoader getPlatformClassLoader() { + ClassLoader platform = JdkPackages.class.getClassLoader().getParent(); + + // Will fail upon changes in classloader hierarchy between JDK versions + assert (platform.getName().equals("platform")); + + return platform; + } + + private static boolean hasJdkExclusivePrefix(String pkg) { + return pkg.startsWith("java.") + || pkg.startsWith("sun."); + } + +} + diff --git a/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java b/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java index 8cccf0598ab..a09604b842b 100644 --- a/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java +++ b/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java @@ -11,8 +11,10 @@ import org.junit.rules.ExpectedException; import java.io.File; import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static com.yahoo.container.plugin.classanalysis.TestUtilities.throwableMessage; import static org.hamcrest.CoreMatchers.containsString; @@ -28,6 +30,7 @@ import static org.junit.Assert.assertThat; */ public class AnalyzeBundleTest { private final List<ExportPackages.Export> exports; + private final Set<String> exportedPackageNames; private final Map<String, ExportPackages.Export> exportsByPackageName; File jarDir = new File("src/test/resources/jar"); @@ -37,6 +40,7 @@ public class AnalyzeBundleTest { File simple = new File(jarDir, "simple1.jar"); PublicPackages pp = AnalyzeBundle.publicPackagesAggregated(Arrays.asList(notOsgi, simple)); this.exports = pp.exports; + this.exportedPackageNames = pp.exportedPackageNames(); this.exportsByPackageName = ExportPackages.exportsByPackageName(exports); } @@ -55,6 +59,11 @@ public class AnalyzeBundleTest { assertThat(exportsByPackageName.keySet(), hasItem("com.yahoo.sample.exported.package")); } + @Test + public void exported_class_names_can_be_retrieved() { + assertThat(exportedPackageNames, is(new HashSet<>(exports.get(0).getPackageNames()))); + } + @Rule public ExpectedException exception = ExpectedException.none(); diff --git a/bundle-plugin/src/test/java/com/yahoo/container/plugin/classanalysis/PackageTallyTest.java b/bundle-plugin/src/test/java/com/yahoo/container/plugin/classanalysis/PackageTallyTest.java new file mode 100644 index 00000000000..e80562c2c13 --- /dev/null +++ b/bundle-plugin/src/test/java/com/yahoo/container/plugin/classanalysis/PackageTallyTest.java @@ -0,0 +1,24 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.plugin.classanalysis; + +import org.junit.Test; + +import java.util.Set; + +import static java.util.Collections.emptyMap; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +/** + * @author gjoranv + */ +public class PackageTallyTest { + + @Test + public void referenced_packages_missing_from_are_detected() { + PackageTally tally = new PackageTally(emptyMap(), Set.of("p1", "java.util", "com.yahoo.api.annotations", "missing")); + Set<String> missingPackages = tally.referencedPackagesMissingFrom(Set.of("p1")); + assertThat(missingPackages, is(Set.of("missing"))); + } + +} diff --git a/bundle-plugin/src/test/java/com/yahoo/container/plugin/osgi/ImportPackageTest.java b/bundle-plugin/src/test/java/com/yahoo/container/plugin/osgi/ImportPackageTest.java index 23960323a31..b76151c790e 100644 --- a/bundle-plugin/src/test/java/com/yahoo/container/plugin/osgi/ImportPackageTest.java +++ b/bundle-plugin/src/test/java/com/yahoo/container/plugin/osgi/ImportPackageTest.java @@ -109,8 +109,9 @@ public class ImportPackageTest { is("com.yahoo.exported;version=\"[1.2.3,2)\"")); } - private static Set<Import> calculateImports(Set<String> referencedPackages, Set<String> implementedPackages, - Map<String, Export> exportedPackages) { + private static Set<Import> calculateImports(Set<String> referencedPackages, + Set<String> implementedPackages, + Map<String, Export> exportedPackages) { return new HashSet<>(ImportPackages.calculateImports(referencedPackages, implementedPackages, exportedPackages).values()); } diff --git a/config-provisioning/abi-spec.json b/config-provisioning/abi-spec.json index 64114389751..33c02811318 100644 --- a/config-provisioning/abi-spec.json +++ b/config-provisioning/abi-spec.json @@ -737,25 +737,6 @@ ], "fields": [] }, - "com.yahoo.config.provision.RotationName": { - "superClass": "java.lang.Object", - "interfaces": [ - "java.lang.Comparable" - ], - "attributes": [ - "public" - ], - "methods": [ - "public java.lang.String value()", - "public boolean equals(java.lang.Object)", - "public int hashCode()", - "public int compareTo(com.yahoo.config.provision.RotationName)", - "public java.lang.String toString()", - "public static com.yahoo.config.provision.RotationName from(java.lang.String)", - "public bridge synthetic int compareTo(java.lang.Object)" - ], - "fields": [] - }, "com.yahoo.config.provision.SystemName": { "superClass": "java.lang.Enum", "interfaces": [], diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/RotationName.java b/config-provisioning/src/main/java/com/yahoo/config/provision/RotationName.java deleted file mode 100644 index fb6d9dc09e6..00000000000 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/RotationName.java +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.config.provision; - -import java.util.Objects; - -/** - * Represents a rotation name for a container cluster. Typically created from the rotation element in services.xml. - * - * @author mpolden - */ -// TODO(mpolden): Remove this once all usages have been replaced -public class RotationName implements Comparable<RotationName> { - - private final String name; - - private RotationName(String name) { - this.name = requireNonBlank(name, "name must be non-empty"); - } - - public String value() { - return name; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - RotationName that = (RotationName) o; - return name.equals(that.name); - } - - @Override - public int hashCode() { - return Objects.hash(name); - } - - @Override - public int compareTo(RotationName o) { - return name.compareTo(o.name); - } - - @Override - public String toString() { - return "rotation '" + name + "'"; - } - - public static RotationName from(String name) { - return new RotationName(name); - } - - private static String requireNonBlank(String s, String message) { - if (s == null || s.isBlank()) { - throw new IllegalArgumentException(message); - } - return s; - } - -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdater.java b/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdater.java index 408bf44e733..b2813be5456 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdater.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdater.java @@ -97,7 +97,7 @@ public class ZKMetricUpdater extends TimerTask { buffer.clear(); } while (nread >= 0); - return Optional.of(baos.toString("UTF-8")); + return Optional.of(baos.toString(StandardCharsets.UTF_8)); } catch (IOException | InterruptedException | ExecutionException | TimeoutException e) { log.warning("Failure in retrieving monitoring data: (" + e.getClass().getName() + ") " + e.getMessage()); return Optional.empty(); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java index 7fd29368ab3..2c6d7de8b0c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java @@ -36,7 +36,7 @@ import java.util.Optional; // TODO: Separate the "application store" and "session" aspects - the latter belongs in the HTTP layer -bratseth public class LocalSession extends Session implements Comparable<LocalSession> { - private final ApplicationPackage applicationPackage; + protected final ApplicationPackage applicationPackage; private final TenantApplications applicationRepo; private final SessionPreparer sessionPreparer; private final SessionContext sessionContext; @@ -47,7 +47,6 @@ public class LocalSession extends Session implements Comparable<LocalSession> { * * @param sessionId The session id for this session. */ - // TODO tenant in SessionContext? public LocalSession(TenantName tenant, long sessionId, SessionPreparer sessionPreparer, SessionContext sessionContext) { super(tenant, sessionId, sessionContext.getSessionZooKeeperClient()); this.serverDB = sessionContext.getServerDBSessionDir(); @@ -211,7 +210,7 @@ public class LocalSession extends Session implements Comparable<LocalSession> { private final String pathToDelete; - public DeleteOperation(String pathToDelete) { + DeleteOperation(String pathToDelete) { this.pathToDelete = pathToDelete; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java index e0727effeda..c37f4aa8401 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java @@ -3,9 +3,7 @@ package com.yahoo.vespa.config.server.session; import com.google.common.collect.HashMultiset; import com.google.common.collect.Multiset; -import com.yahoo.concurrent.InThreadExecutorService; import com.yahoo.concurrent.StripedExecutor; -import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.log.LogLevel; @@ -20,7 +18,6 @@ import com.yahoo.vespa.config.server.zookeeper.ConfigCurator; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.InMemoryFlagSource; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.cache.ChildData; import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; @@ -33,8 +30,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.Executor; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -68,7 +63,6 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> { ReloadHandler reloadHandler, TenantName tenantName, TenantApplications applicationRepo) { - this.curator = registry.getCurator(); this.sessionsPath = TenantRepository.getSessionsPath(tenantName); this.applicationRepo = applicationRepo; @@ -85,20 +79,6 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> { this.directoryCache.start(); } - // For testing only - public RemoteSessionRepo(TenantName tenantName) { - this.curator = null; - this.remoteSessionFactory = null; - this.reloadHandler = null; - this.tenantName = tenantName; - this.sessionsPath = TenantRepository.getSessionsPath(tenantName); - this.metrics = null; - this.directoryCache = null; - this.applicationRepo = null; - this.flagSource = new InMemoryFlagSource(); - this.zkWatcherExecutor = Runnable::run; - } - public List<Long> getSessions() { return getSessionList(curator.getChildren(sessionsPath)); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java index ad2472add89..2e09d830783 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java @@ -5,7 +5,6 @@ import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.concurrent.StripedExecutor; -import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.log.LogLevel; @@ -285,7 +284,7 @@ public class TenantRepository { tenants.get(name).delete(); } - public synchronized void closeTenant(TenantName name) { + private synchronized void closeTenant(TenantName name) { Tenant tenant = tenants.remove(name); if (tenant == null) throw new IllegalArgumentException("Closing '" + name + "' failed, tenant does not exist"); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java index e4ff8702ff1..6c144fe2f43 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java @@ -45,7 +45,6 @@ public class InjectedGlobalComponentRegistryTest { private SessionPreparer sessionPreparer; private ConfigserverConfig configserverConfig; private RpcServer rpcServer; - private SuperModelGenerationCounter generationCounter; private ConfigDefinitionRepo defRepo; private PermanentApplicationPackage permanentApplicationPackage; private HostRegistries hostRegistries; @@ -68,8 +67,10 @@ public class InjectedGlobalComponentRegistryTest { .configDefinitionsDir(temporaryFolder.newFolder("configdefinitions").getAbsolutePath())); sessionPreparer = new SessionTest.MockSessionPreparer(); rpcServer = new RpcServer(configserverConfig, null, Metrics.createTestMetrics(), - new HostRegistries(), new ConfigRequestHostLivenessTracker(), new FileServer(temporaryFolder.newFolder("filereferences")), new NoopRpcAuthorizer(), new RpcRequestHandlerProvider()); - generationCounter = new SuperModelGenerationCounter(curator); + new HostRegistries(), new ConfigRequestHostLivenessTracker(), + new FileServer(temporaryFolder.newFolder("filereferences")), + new NoopRpcAuthorizer(), new RpcRequestHandlerProvider()); + SuperModelGenerationCounter generationCounter = new SuperModelGenerationCounter(curator); defRepo = new StaticConfigDefinitionRepo(); permanentApplicationPackage = new PermanentApplicationPackage(configserverConfig); hostRegistries = new HostRegistries(); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/MemoryGenerationCounter.java b/configserver/src/test/java/com/yahoo/vespa/config/server/MemoryGenerationCounter.java index b83a46cb066..2df28f81e9e 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/MemoryGenerationCounter.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/MemoryGenerationCounter.java @@ -5,10 +5,10 @@ import com.yahoo.vespa.config.GenerationCounter; /** * @author Ulf Lilleengen - * @since 5. */ public class MemoryGenerationCounter implements GenerationCounter { - long value; + private long value; + @Override public long increment() { return ++value; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/MiscTestCase.java b/configserver/src/test/java/com/yahoo/vespa/config/server/MiscTestCase.java index 88e602eed7c..f8777f4c477 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/MiscTestCase.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/MiscTestCase.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.config.server; import static org.junit.Assert.*; import java.io.*; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.ArrayList; import org.junit.Test; @@ -32,7 +33,7 @@ public class MiscTestCase { private static List<String> file2lines(File file) throws IOException { List<String> lines = new ArrayList<>(); - LineNumberReader in = new LineNumberReader(new InputStreamReader(new FileInputStream(file), "UTF-8")); + LineNumberReader in = new LineNumberReader(new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8)); String line; while ((line = in.readLine()) != null) { lines.add(line); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/MockReloadHandler.java b/configserver/src/test/java/com/yahoo/vespa/config/server/MockReloadHandler.java index 99d0c6ea4da..6e9299c1a61 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/MockReloadHandler.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/MockReloadHandler.java @@ -8,16 +8,13 @@ import java.util.Set; /** * @author Ulf Lilleengen - * @since 5.1.24 */ public class MockReloadHandler implements ReloadHandler { - public ApplicationSet current = null; public volatile ApplicationId lastRemoved = null; @Override public void reloadConfig(ApplicationSet application) { - this.current = application; } @Override diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelFactoryRegistryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelFactoryRegistryTest.java index 986e9f5603d..2fd87161fd0 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelFactoryRegistryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelFactoryRegistryTest.java @@ -67,7 +67,7 @@ public class ModelFactoryRegistryTest { @Test(expected = UnknownVespaVersionException.class) public void testThatUnknownVersionGivesError() { - ModelFactoryRegistry registry = new ModelFactoryRegistry(Arrays.asList(new TestFactory(new Version(1, 2, 3)))); + ModelFactoryRegistry registry = new ModelFactoryRegistry(List.of(new TestFactory(new Version(1, 2, 3)))); registry.getFactory(new Version(3, 2, 1)); } @@ -75,7 +75,7 @@ public class ModelFactoryRegistryTest { private final Version version; - public TestFactory(Version version) { + TestFactory(Version version) { this.version = version; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/PortRangeAllocator.java b/configserver/src/test/java/com/yahoo/vespa/config/server/PortRangeAllocator.java index 8424dccbedc..359c8562869 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/PortRangeAllocator.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/PortRangeAllocator.java @@ -13,7 +13,6 @@ import java.util.Stack; * Allocates port ranges for all configserver tests. * * @author Ulf Lilleengen - * @since 5.1.26 */ public class PortRangeAllocator { private final static PortRange portRange = new PortRange(); @@ -33,7 +32,7 @@ public class PortRangeAllocator { private static final int first = 18651; private static final int last = 18899; // see: factory/doc/port-ranges - public PortRange() { + PortRange() { freePorts.addAll(ContiguousSet.create(Range.closed(first, last), DiscreteDomain.integers())); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelControllerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelControllerTest.java index c6f6be5fbab..b7486dc7951 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelControllerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelControllerTest.java @@ -54,7 +54,7 @@ public class SuperModelControllerTest { File testApp = new File("src/test/resources/deploy/app"); ApplicationId app = ApplicationId.from(TenantName.from("a"), ApplicationName.from("foo"), InstanceName.defaultName()); - models.put(app, new ApplicationInfo(app, 4l, new VespaModel(FilesApplicationPackage.fromFile(testApp)))); + models.put(app, new ApplicationInfo(app, 4L, new VespaModel(FilesApplicationPackage.fromFile(testApp)))); SuperModel superModel = new SuperModel(models); handler = new SuperModelController(new SuperModelConfigProvider(superModel, Zone.defaultZone(), new InMemoryFlagSource()), new TestConfigDefinitionRepo(), 2, new UncompressedConfigResponseFactory()); } @@ -94,9 +94,9 @@ public class SuperModelControllerTest { ApplicationId simple = applicationId("mysimpleapp", t1); ApplicationId advanced = applicationId("myadvancedapp", t1); ApplicationId tooAdvanced = applicationId("minetooadvancedapp", t2); - models.put(simple, createApplicationInfo(testApp1, simple, 4l)); - models.put(advanced, createApplicationInfo(testApp2, advanced, 4l)); - models.put(tooAdvanced, createApplicationInfo(testApp3, tooAdvanced, 4l)); + models.put(simple, createApplicationInfo(testApp1, simple, 4L)); + models.put(advanced, createApplicationInfo(testApp2, advanced, 4L)); + models.put(tooAdvanced, createApplicationInfo(testApp3, tooAdvanced, 4L)); SuperModel superModel = new SuperModel(models); SuperModelController han = new SuperModelController(new SuperModelConfigProvider(superModel, Zone.defaultZone(), new InMemoryFlagSource()), new TestConfigDefinitionRepo(), 2, new UncompressedConfigResponseFactory()); @@ -122,9 +122,9 @@ public class SuperModelControllerTest { ApplicationId simple = applicationId("mysimpleapp", t1); ApplicationId advanced = applicationId("myadvancedapp", t1); ApplicationId tooAdvanced = applicationId("minetooadvancedapp", t2); - models.put(simple, createApplicationInfo(testApp1, simple, 4l)); - models.put(advanced, createApplicationInfo(testApp2, advanced, 4l)); - models.put(tooAdvanced, createApplicationInfo(testApp3, tooAdvanced, 4l)); + models.put(simple, createApplicationInfo(testApp1, simple, 4L)); + models.put(advanced, createApplicationInfo(testApp2, advanced, 4L)); + models.put(tooAdvanced, createApplicationInfo(testApp3, tooAdvanced, 4L)); SuperModel superModel = new SuperModel(models); SuperModelController han = new SuperModelController(new SuperModelConfigProvider(superModel, Zone.defaultZone(), new InMemoryFlagSource()), new TestConfigDefinitionRepo(), 2, new UncompressedConfigResponseFactory()); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelRequestHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelRequestHandlerTest.java index f7b900c8f02..dd2c3e07131 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelRequestHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelRequestHandlerTest.java @@ -57,14 +57,14 @@ public class SuperModelRequestHandlerTest { assertNotNull(controller.getHandler()); long gen = counter.get(); - controller.reloadConfig(createApp(foo, 3l)); + controller.reloadConfig(createApp(foo, 3L)); assertNotNull(controller.getHandler()); assertThat(controller.getHandler().getGeneration(), is(gen + 1)); - controller.reloadConfig(createApp(foo, 4l)); + controller.reloadConfig(createApp(foo, 4L)); assertThat(controller.getHandler().getGeneration(), is(gen + 2)); // Test that a new app is used when there already exist an application with the same id - assertThat(controller.getHandler().getSuperModel().applicationModels().get(foo).getGeneration(), is(4l)); - controller.reloadConfig(createApp(bar, 2l)); + assertThat(controller.getHandler().getSuperModel().applicationModels().get(foo).getGeneration(), is(4L)); + controller.reloadConfig(createApp(bar, 2L)); assertThat(controller.getHandler().getGeneration(), is(gen + 3)); } @@ -75,9 +75,9 @@ public class SuperModelRequestHandlerTest { ApplicationId baz = applicationId("b", "baz"); long gen = counter.get(); - controller.reloadConfig(createApp(foo, 3l)); - controller.reloadConfig(createApp(bar, 30l)); - controller.reloadConfig(createApp(baz, 9l)); + controller.reloadConfig(createApp(foo, 3L)); + controller.reloadConfig(createApp(bar, 30L)); + controller.reloadConfig(createApp(baz, 9L)); assertThat(controller.getHandler().getGeneration(), is(gen + 3)); assertThat(controller.getHandler().getSuperModel().applicationModels().size(), is(3)); assertEquals(Arrays.asList(foo, bar, baz), new ArrayList<>(controller.getHandler().getSuperModel().applicationModels().keySet())); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/TimeoutBudgetTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/TimeoutBudgetTest.java index 070201a4369..db9906b9e02 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/TimeoutBudgetTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/TimeoutBudgetTest.java @@ -24,16 +24,16 @@ public class TimeoutBudgetTest { ManualClock clock = new ManualClock(); TimeoutBudget budget = new TimeoutBudget(clock, Duration.ofMillis(7)); - assertThat(budget.timeLeft().toMillis(), is(7l)); + assertThat(budget.timeLeft().toMillis(), is(7L)); clock.advance(Duration.ofMillis(1)); - assertThat(budget.timeLeft().toMillis(), is(6l)); + assertThat(budget.timeLeft().toMillis(), is(6L)); clock.advance(Duration.ofMillis(5)); - assertThat(budget.timeLeft().toMillis(), is(1l)); - assertThat(budget.timeLeft().toMillis(), is(1l)); + assertThat(budget.timeLeft().toMillis(), is(1L)); + assertThat(budget.timeLeft().toMillis(), is(1L)); clock.advance(Duration.ofMillis(1)); - assertThat(budget.timeLeft().toMillis(), is(0l)); + assertThat(budget.timeLeft().toMillis(), is(0L)); clock.advance(Duration.ofMillis(5)); - assertThat(budget.timeLeft().toMillis(), is(0l)); + assertThat(budget.timeLeft().toMillis(), is(0L)); clock.advance(Duration.ofMillis(1)); assertThat(budget.timesUsed(), is("[0 ms, 1 ms, 5 ms, 0 ms, 1 ms, 5 ms, total: 13 ms]")); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationMapperTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationMapperTest.java index 284bb716a6e..a562f89c7ec 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationMapperTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationMapperTest.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.config.server.application; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import java.util.Optional; import com.yahoo.config.provision.ApplicationId; @@ -18,22 +19,21 @@ import static org.junit.Assert.assertEquals; public class ApplicationMapperTest { - ApplicationId appId; - ApplicationMapper applicationMapper; - ArrayList<Version> vespaVersions = new ArrayList<>(); - ArrayList<Application> applications = new ArrayList<>(); + private ApplicationId appId; + private ApplicationMapper applicationMapper; + private ArrayList<Version> vespaVersions = new ArrayList<>(); + private ArrayList<Application> applications = new ArrayList<>(); @Before public void setUp() { applicationMapper = new ApplicationMapper(); appId = new ApplicationId.Builder() - .tenant("test").applicationName("test").instanceName("test").build(); - vespaVersions.add(Version.fromString("1.2.3")); - vespaVersions.add(Version.fromString("1.2.4")); - vespaVersions.add(Version.fromString("1.2.5")); - applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(0), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); - applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(1), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); - applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(2), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); + .tenant("test") + .applicationName("test") + .instanceName("test") + .build(); + vespaVersions.addAll(List.of(Version.fromString("1.2.3"), Version.fromString("1.2.4"), Version.fromString("1.2.5"))); + applications.addAll(List.of(createApplication(vespaVersions.get(0)), createApplication(vespaVersions.get(1)), createApplication(vespaVersions.get(2)))); } @Test @@ -53,7 +53,6 @@ public class ApplicationMapperTest { @Test (expected = VersionDoesNotExistException.class) public void testGetForVersionThrows() { applicationMapper.register(appId, ApplicationSet.fromList(Arrays.asList(applications.get(0), applications.get(2)))); - applicationMapper.getForVersion(appId, Optional.of(vespaVersions.get(1)), Instant.now()); } @@ -62,9 +61,16 @@ public class ApplicationMapperTest { applicationMapper.register(appId, ApplicationSet.fromSingle(applications.get(0))); applicationMapper.getForVersion(new ApplicationId.Builder() - .tenant("different").applicationName("different").instanceName("different").build(), + .tenant("different") + .applicationName("different") + .instanceName("different") + .build(), Optional.of(vespaVersions.get(1)), Instant.now()); } + private Application createApplication(Version version) { + return new Application(new ModelStub(), null, 0, false, version, MetricUpdater.createTestUpdater(), ApplicationId.defaultId()); + } + } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationSetTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationSetTest.java index 8ee6a82bd1d..cf1e00674cb 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationSetTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationSetTest.java @@ -20,18 +20,14 @@ import static org.junit.Assert.assertEquals; */ public class ApplicationSetTest { - ApplicationSet applicationSet; - List<Version> vespaVersions = new ArrayList<>(); - List<Application> applications = new ArrayList<>(); + private ApplicationSet applicationSet; + private List<Version> vespaVersions = new ArrayList<>(); + private List<Application> applications = new ArrayList<>(); @Before public void setUp() { - vespaVersions.add(Version.fromString("1.2.3")); - vespaVersions.add(Version.fromString("1.2.4")); - vespaVersions.add(Version.fromString("1.2.5")); - applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(0), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); - applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(1), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); - applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(2), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); + vespaVersions.addAll(List.of(Version.fromString("1.2.3"), Version.fromString("1.2.4"), Version.fromString("1.2.5"))); + applications.addAll(List.of(createApplication(vespaVersions.get(0)), createApplication(vespaVersions.get(1)), createApplication(vespaVersions.get(2)))); } @Test @@ -54,4 +50,7 @@ public class ApplicationSetTest { applicationSet.getForVersionOrLatest(Optional.of(vespaVersions.get(1)), Instant.now()); } + private Application createApplication(Version version) { + return new Application(new ModelStub(), null, 0, false, version, MetricUpdater.createTestUpdater(), ApplicationId.defaultId()); + } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationTest.java index cc30ef09878..405fff3e190 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationTest.java @@ -55,7 +55,7 @@ public class ApplicationTest { ServerCache cache = new ServerCache(); Version vespaVersion = new Version(1, 2, 3); Application app = new Application(new ModelStub(), cache, 1337L, false, vespaVersion, MetricUpdater.createTestUpdater(), appId); - assertThat(app.getApplicationGeneration(), is(1337l)); + assertThat(app.getApplicationGeneration(), is(1337L)); assertNotNull(app.getModel()); assertThat(app.getCache(), is(cache)); assertThat(app.getId().application().value(), is("foobar")); @@ -107,7 +107,7 @@ public class ApplicationTest { @Test public void require_that_known_config_defs_are_found() { - handler.resolveConfig(createSimpleConfigRequest(emptySchema)); + handler.resolveConfig(createSimpleConfigRequest()); } @Test @@ -119,7 +119,7 @@ public class ApplicationTest { @Test public void require_that_non_existent_fields_in_schema_is_skipped() { // Ask for config without schema and check that we get correct default value back - List<String> payload = handler.resolveConfig(createSimpleConfigRequest(emptySchema)).getLegacyPayload(); + List<String> payload = handler.resolveConfig(createSimpleConfigRequest()).getLegacyPayload(); assertThat(payload.get(0), is("boolval false")); // Ask for config with wrong schema String[] schema = new String[1]; @@ -138,19 +138,18 @@ public class ApplicationTest { assertTrue(response == cached_response); } - private static GetConfigRequest createRequest(String name, String namespace, String defMd5, String[] schema, String configId) { + private static GetConfigRequest createRequest(String name, String namespace, String defMd5, String[] schema) { Request request = JRTClientConfigRequestV3. - createWithParams(new ConfigKey<>(name, configId, namespace, defMd5, null), DefContent.fromArray(schema), + createWithParams(new ConfigKey<>(name, "admin/model", namespace, defMd5, null), DefContent.fromArray(schema), "fromHost", "", 0, 100, Trace.createDummy(), CompressionType.UNCOMPRESSED, Optional.empty()).getRequest(); return JRTServerConfigRequestV3.createFromRequest(request); } - private static GetConfigRequest createRequest(String name, String namespace, String defMd5, String[] schema) { - return createRequest(name, namespace, defMd5, schema, "admin/model"); - } - - private static GetConfigRequest createSimpleConfigRequest(String[] schema) { - return createRequest(SimpletypesConfig.CONFIG_DEF_NAME, SimpletypesConfig.CONFIG_DEF_NAMESPACE, SimpletypesConfig.CONFIG_DEF_MD5, schema); + private static GetConfigRequest createSimpleConfigRequest() { + return createRequest(SimpletypesConfig.CONFIG_DEF_NAME, + SimpletypesConfig.CONFIG_DEF_NAMESPACE, + SimpletypesConfig.CONFIG_DEF_MD5, + ApplicationTest.emptySchema); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java index 496da2cf809..016192e3281 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.config.server.application; import com.google.common.collect.ImmutableList; import com.google.common.io.ByteStreams; -import com.yahoo.vespa.config.server.application.CompressedApplicationInputStream; import org.apache.commons.compress.archivers.ArchiveOutputStream; import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; @@ -22,6 +21,7 @@ import java.util.zip.GZIPOutputStream; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -30,13 +30,13 @@ import static org.junit.Assert.assertTrue; */ public class CompressedApplicationInputStreamTest { - static void writeFileToTar(ArchiveOutputStream taos, File file) throws IOException { + private static void writeFileToTar(ArchiveOutputStream taos, File file) throws IOException { taos.putArchiveEntry(taos.createArchiveEntry(file, file.getName())); ByteStreams.copy(new FileInputStream(file), taos); taos.closeArchiveEntry(); } - public static File createArchiveFile(ArchiveOutputStream taos, File outFile) throws IOException { + private static File createArchiveFile(ArchiveOutputStream taos, File outFile) throws IOException { File app = new File("src/test/resources/deploy/validapp"); writeFileToTar(taos, new File(app, "services.xml")); writeFileToTar(taos, new File(app, "hosts.xml")); @@ -51,14 +51,15 @@ public class CompressedApplicationInputStreamTest { return createArchiveFile(archiveOutputStream, outFile); } - public static File createZipFile() throws IOException { + private static File createZipFile() throws IOException { File outFile = File.createTempFile("testapp", ".tar.gz"); ArchiveOutputStream archiveOutputStream = new ZipArchiveOutputStream(new FileOutputStream(outFile)); return createArchiveFile(archiveOutputStream, outFile); } - void assertTestApp(File outApp) { + private void assertTestApp(File outApp) { String [] files = outApp.list(); + assertNotNull(files); assertThat(files.length, is(3)); assertThat(Arrays.asList(files), containsInAnyOrder(ImmutableList.of(is("hosts.xml"), is("services.xml"), is("deployment.xml")))); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java index 871182d75d9..800be763092 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java @@ -184,12 +184,10 @@ public class ConfigConvergenceCheckerTest { .withBody("response too slow"))); HttpResponse response = checker.checkService(application, hostAndPort(service), requestUrl, Duration.ofMillis(1)); // Message contained in a SocketTimeoutException may differ across platforms, so we do a partial match of the response here - assertResponse((responseBody) -> { - assertTrue("Response matches", responseBody.startsWith( - "{\"url\":\"" + requestUrl.toString() + "\",\"host\":\"" + hostAndPort(requestUrl) + - "\",\"wantedGeneration\":3,\"error\":\"java.net.SocketTimeoutException") && - responseBody.endsWith("\"}")); - }, 404, response); + assertResponse((responseBody) -> assertTrue("Response matches", responseBody.startsWith( + "{\"url\":\"" + requestUrl.toString() + "\",\"host\":\"" + hostAndPort(requestUrl) + + "\",\"wantedGeneration\":3,\"error\":\"java.net.SocketTimeoutException") && + responseBody.endsWith("\"}")), 404, response); } private URI testServer() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/FileDistributionStatusTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/FileDistributionStatusTest.java index 0c27066dd6b..89392d799ba 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/FileDistributionStatusTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/FileDistributionStatusTest.java @@ -170,7 +170,7 @@ public class FileDistributionStatusTest { appId); } - HttpResponse getStatus(FileDistributionStatus fileDistributionStatus, Application application) { + private HttpResponse getStatus(FileDistributionStatus fileDistributionStatus, Application application) { return fileDistributionStatus.status(application, timeout); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java index 433a63f4ad2..0da96f9f01d 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java @@ -45,6 +45,7 @@ public class MockModel implements Model { return new HostInfo(hostname, Arrays.asList(container, serviceNoStatePort)); } + // TODO: Move to caller static MockModel createClusterController(String hostname, int statePort) { ServiceInfo container = createServiceInfo( hostname, diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java index aa157366a60..66d113afbe6 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java @@ -4,7 +4,6 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.orchestrator.Host; import com.yahoo.vespa.orchestrator.Orchestrator; -import com.yahoo.vespa.orchestrator.model.NodeGroup; import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus; import com.yahoo.vespa.orchestrator.status.HostStatus; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java index e3335dded4c..33932a678b7 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java @@ -69,11 +69,11 @@ public class TenantApplicationsTest { TenantApplications repo = createZKAppRepo(); ApplicationId myapp = createApplicationId("myapp"); repo.createApplication(myapp); - repo.createPutTransaction(myapp, 3l).commit(); + repo.createPutTransaction(myapp, 3).commit(); String path = TenantRepository.getApplicationsPath(tenantName).append(myapp.serializedForm()).getAbsolute(); assertNotNull(curatorFramework.checkExists().forPath(path)); assertThat(Utf8.toString(curatorFramework.getData().forPath(path)), is("3")); - repo.createPutTransaction(myapp, 5l).commit(); + repo.createPutTransaction(myapp, 5).commit(); assertNotNull(curatorFramework.checkExists().forPath(path)); assertThat(Utf8.toString(curatorFramework.getData().forPath(path)), is("5")); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java index 4a23d76b76e..2566b1029a8 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java @@ -11,7 +11,6 @@ import java.util.List; /** * @author geirst - * @since 5.44 */ public class ConfigChangeActionsBuilder { @@ -25,15 +24,15 @@ public class ConfigChangeActionsBuilder { public ConfigChangeActionsBuilder restart(String message, String clusterName, String clusterType, String serviceType, String serviceName) { actions.add(new MockRestartAction(message, - Arrays.asList(createService(clusterName, clusterType, serviceType, serviceName)))); + List.of(createService(clusterName, clusterType, serviceType, serviceName)))); return this; } - public ConfigChangeActionsBuilder refeed(String name, boolean allowed, String message, String documentType, String clusterName, String serviceName) { + ConfigChangeActionsBuilder refeed(String name, boolean allowed, String message, String documentType, String clusterName, String serviceName) { actions.add(new MockRefeedAction(name, allowed, message, - Arrays.asList(createService(clusterName, "myclustertype", "myservicetype", serviceName)), documentType)); + List.of(createService(clusterName, "myclustertype", "myservicetype", serviceName)), documentType)); return this; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockConfigChangeAction.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockConfigChangeAction.java index 64e3aceee8d..639e4a6fee3 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockConfigChangeAction.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockConfigChangeAction.java @@ -15,7 +15,7 @@ public abstract class MockConfigChangeAction implements ConfigChangeAction { private final String message; private final List<ServiceInfo> services; - protected MockConfigChangeAction(String message, List<ServiceInfo> services) { + MockConfigChangeAction(String message, List<ServiceInfo> services) { this.message = message; this.services = services; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsTest.java index 6f21682981a..7235b8905c5 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RefeedActionsTest.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.configchange; +import com.yahoo.config.model.api.ServiceInfo; import org.junit.Test; import java.util.List; @@ -13,7 +14,6 @@ import static com.yahoo.vespa.config.server.configchange.Utils.*; /** * @author geirst - * @since 5.44 */ public class RefeedActionsTest { @@ -21,7 +21,7 @@ public class RefeedActionsTest { StringBuilder builder = new StringBuilder(); builder.append(entry.getDocumentType() + "." + entry.getClusterName() + ":"); builder.append(entry.getServices().stream(). - map(service -> service.getServiceName()). + map(ServiceInfo::getServiceName). sorted(). collect(Collectors.joining(",", "[", "]"))); builder.append(entry.getMessages().stream(). diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RestartActionsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RestartActionsTest.java index 4c937061733..ee0180802af 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RestartActionsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RestartActionsTest.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.configchange; +import com.yahoo.config.model.api.ServiceInfo; import org.junit.Test; import java.util.List; @@ -13,7 +14,6 @@ import static com.yahoo.vespa.config.server.configchange.Utils.*; /** * @author geirst - * @since 5.44 */ public class RestartActionsTest { @@ -21,7 +21,7 @@ public class RestartActionsTest { StringBuilder builder = new StringBuilder(); builder.append(entry.getClusterType() + "." + entry.getClusterName() + "." + entry.getServiceType() + ":"); builder.append(entry.getServices().stream(). - map(service -> service.getServiceName()). + map(ServiceInfo::getServiceName). sorted(). collect(Collectors.joining(",", "[", "]"))); builder.append(entry.getMessages().stream(). diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployHandlerLoggerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployHandlerLoggerTest.java index d5092adb90b..d321edefe67 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployHandlerLoggerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployHandlerLoggerTest.java @@ -8,7 +8,6 @@ import com.yahoo.slime.Cursor; import com.yahoo.slime.JsonFormat; import com.yahoo.slime.Slime; -import com.yahoo.vespa.config.server.deploy.DeployHandlerLogger; import org.junit.Test; import java.io.ByteArrayOutputStream; @@ -19,7 +18,6 @@ import static org.junit.Assert.assertTrue; /** * @author Ulf Lilleengen - * @since 5.1 */ public class DeployHandlerLoggerTest { @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java index b363c749212..74d3fd4ec74 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java @@ -366,7 +366,7 @@ public class HostedDeployTest { @Override public ModelCreateResult createAndValidateModel(ModelContext modelContext, ValidationParameters validationParameters) { ModelCreateResult result = super.createAndValidateModel(modelContext, validationParameters); - return new ModelCreateResult(result.getModel(), Arrays.asList(action)); + return new ModelCreateResult(result.getModel(), List.of(action)); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java index da387eb569a..967e2321b95 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java @@ -13,8 +13,6 @@ import java.util.Optional; */ public class MockDeployer implements com.yahoo.config.provision.Deployer { - public ApplicationId lastDeployed; - @Override public Optional<Deployment> deployFromLocalActive(ApplicationId application) { return deployFromLocalActive(application, Duration.ofSeconds(60)); @@ -27,7 +25,6 @@ public class MockDeployer implements com.yahoo.config.provision.Deployer { @Override public Optional<Deployment> deployFromLocalActive(ApplicationId application, Duration timeout) { - lastDeployed = application; return Optional.empty(); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/RedeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/RedeployTest.java index adf3ba19fe3..49327f984b7 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/RedeployTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/RedeployTest.java @@ -9,7 +9,6 @@ import com.yahoo.component.Version; import org.junit.Test; import java.time.Clock; -import java.time.Instant; import java.util.ArrayList; import java.util.List; import java.util.Optional; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java index 918670d71f2..daf981611e4 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java @@ -80,7 +80,7 @@ public class ZooKeeperClientTest { } @Test - public void testInitZooKeeper() throws IOException { + public void testInitZooKeeper() { ConfigCurator zk = ConfigCurator.create(new MockCurator()); BaseDeployLogger logger = new BaseDeployLogger(); long generation = 1L; @@ -88,8 +88,8 @@ public class ZooKeeperClientTest { zooKeeperClient.setupZooKeeper(); String appPath = "/"; assertThat(zk.getChildren(appPath).size(), is(1)); - assertTrue(zk.exists("/" + String.valueOf(generation))); - String currentAppPath = appPath + String.valueOf(generation); + assertTrue(zk.exists("/" + generation)); + String currentAppPath = appPath + generation; assertTrue(zk.exists(currentAppPath, ConfigCurator.DEFCONFIGS_ZK_SUBPATH.replaceFirst("/", ""))); assertThat(zk.getChildren(currentAppPath).size(), is(4)); } @@ -140,9 +140,9 @@ public class ZooKeeperClientTest { assertTrue(metaData.isInternalRedeploy()); assertThat(metaData.getDeployedByUser(), is("foo")); assertThat(metaData.getDeployPath(), is("/bar/baz")); - assertThat(metaData.getDeployTimestamp(), is(1345l)); - assertThat(metaData.getGeneration(), is(3l)); - assertThat(metaData.getPreviousActiveGeneration(), is(2l)); + assertThat(metaData.getDeployTimestamp(), is(1345L)); + assertThat(metaData.getGeneration(), is(3L)); + assertThat(metaData.getPreviousActiveGeneration(), is(2L)); } @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployerTest.java index d00c0b8dd32..4825ccc1328 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployerTest.java @@ -8,7 +8,6 @@ import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.component.Version; import com.yahoo.io.IOUtils; import com.yahoo.path.Path; -import com.yahoo.prelude.semantics.parser.ParseException; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.config.server.zookeeper.ConfigCurator; import org.junit.Rule; @@ -25,7 +24,6 @@ import static org.junit.Assert.fail; /** * @author Ulf Lilleengen - * @since 5.1 */ public class ZooKeeperDeployerTest { @@ -34,7 +32,7 @@ public class ZooKeeperDeployerTest { private static final String defFile = "test2.def"; @Test - public void require_that_deployer_is_initialized() throws IOException, ParseException { + public void require_that_deployer_is_initialized() throws IOException { ConfigCurator zkfacade = ConfigCurator.create(new MockCurator()); File serverdbDir = folder.newFolder("serverdb"); File defsDir = new File(serverdbDir, "serverdefs"); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/host/HostRegistryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/host/HostRegistryTest.java index b2335165105..63dfb1d01bd 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/host/HostRegistryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/host/HostRegistryTest.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.config.server.host; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -20,12 +19,12 @@ public class HostRegistryTest { public void old_hosts_are_removed() { HostRegistry<String> reg = new HostRegistry<>(); assertNull(reg.getKeyForHost("foo.com")); - reg.update("fookey", Arrays.asList("foo.com", "bar.com", "baz.com")); + reg.update("fookey", List.of("foo.com", "bar.com", "baz.com")); assertGetKey(reg, "foo.com", "fookey"); assertGetKey(reg, "bar.com", "fookey"); assertGetKey(reg, "baz.com", "fookey"); assertThat(reg.getAllHosts().size(), is(3)); - reg.update("fookey", Arrays.asList("bar.com", "baz.com")); + reg.update("fookey", List.of("bar.com", "baz.com")); assertNull(reg.getKeyForHost("foo.com")); assertGetKey(reg, "bar.com", "fookey"); assertGetKey(reg, "baz.com", "fookey"); @@ -41,8 +40,8 @@ public class HostRegistryTest { @Test public void multiple_keys_are_handled() { HostRegistry<String> reg = new HostRegistry<>(); - reg.update("fookey", Arrays.asList("foo.com", "bar.com")); - reg.update("barkey", Arrays.asList("baz.com", "quux.com")); + reg.update("fookey", List.of("foo.com", "bar.com")); + reg.update("barkey", List.of("baz.com", "quux.com")); assertGetKey(reg, "foo.com", "fookey"); assertGetKey(reg, "bar.com", "fookey"); assertGetKey(reg, "baz.com", "barkey"); @@ -52,22 +51,22 @@ public class HostRegistryTest { @Test(expected = IllegalArgumentException.class) public void keys_cannot_overlap() { HostRegistry<String> reg = new HostRegistry<>(); - reg.update("fookey", Arrays.asList("foo.com", "bar.com")); - reg.update("barkey", Arrays.asList("bar.com", "baz.com")); + reg.update("fookey", List.of("foo.com", "bar.com")); + reg.update("barkey", List.of("bar.com", "baz.com")); } @Test public void all_hosts_are_returned() { HostRegistry<String> reg = new HostRegistry<>(); - reg.update("fookey", Arrays.asList("foo.com", "bar.com")); - reg.update("barkey", Arrays.asList("baz.com", "quux.com")); + reg.update("fookey", List.of("foo.com", "bar.com")); + reg.update("barkey", List.of("baz.com", "quux.com")); assertThat(reg.getAllHosts().size(), is(4)); } @Test public void ensure_that_collection_is_copied() { HostRegistry<String> reg = new HostRegistry<>(); - List<String> hosts = new ArrayList<>(Arrays.asList("foo.com", "bar.com", "baz.com")); + List<String> hosts = new ArrayList<>(List.of("foo.com", "bar.com", "baz.com")); reg.update("fookey", hosts); assertThat(reg.getHostsForKey("fookey").size(), is(3)); hosts.remove(2); @@ -77,10 +76,10 @@ public class HostRegistryTest { @Test public void ensure_that_underlying_hosts_do_not_change() { HostRegistry<String> reg = new HostRegistry<>(); - reg.update("fookey", new ArrayList<>(Arrays.asList("foo.com", "bar.com", "baz.com"))); + reg.update("fookey", List.of("foo.com", "bar.com", "baz.com")); Collection<String> hosts = reg.getAllHosts(); assertThat(hosts.size(), is(3)); - reg.update("fookey", new ArrayList<>(Arrays.asList("foo.com"))); + reg.update("fookey", List.of("foo.com")); assertThat(hosts.size(), is(3)); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/ContentHandlerTestBase.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/ContentHandlerTestBase.java index 3415facd714..20e52263350 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/ContentHandlerTestBase.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/ContentHandlerTestBase.java @@ -11,10 +11,8 @@ import static org.junit.Assert.assertThat; import java.io.IOException; import java.util.Arrays; import java.util.Collection; -import javax.annotation.Nullable; import org.junit.Test; -import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.collect.Collections2; import com.yahoo.container.jdisc.HttpResponse; @@ -37,7 +35,7 @@ public abstract class ContentHandlerTestBase extends SessionHandlerTest { } @Test - public void require_that_nonexistant_file_returns_not_found() throws IOException { + public void require_that_nonexistant_file_returns_not_found() { HttpResponse response = doRequest(HttpRequest.Method.GET, "/test2.txt"); assertNotNull(response); assertThat(response.getStatus(), is(NOT_FOUND)); @@ -88,12 +86,7 @@ public abstract class ContentHandlerTestBase extends SessionHandlerTest { protected abstract HttpResponse doRequest(HttpRequest.Method method, String path); private String generateResultArray(String... files) { - Collection<String> output = Collections2.transform(Arrays.asList(files), new Function<String, String>() { - @Override - public String apply(@Nullable String input) { - return "\"" + baseUrl + input + "\""; - } - }); + Collection<String> output = Collections2.transform(Arrays.asList(files), input -> "\"" + baseUrl + input + "\""); StringBuilder sb = new StringBuilder(); sb.append("["); sb.append(Joiner.on(",").join(output)); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpConfigRequestTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpConfigRequestTest.java index 62ec451107d..d28aa804c6f 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpConfigRequestTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpConfigRequestTest.java @@ -1,8 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.http; -import java.io.IOException; - import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.vespa.config.ConfigKey; @@ -17,7 +15,6 @@ import static org.junit.Assert.assertTrue; /** * @author Ulf Lilleengen - * @since 5.1 */ public class HttpConfigRequestTest { @Test @@ -39,7 +36,7 @@ public class HttpConfigRequestTest { } @Test - public void require_that_request_can_be_created_with_advanced_uri() throws IOException { + public void require_that_request_can_be_created_with_advanced_uri() { HttpConfigRequest.createFromRequestV1(HttpRequest.createTestRequest( "http://example.yahoo.com:19071/config/v1/vespa.config.cloud.sentinel/host-01.example.yahoo.com", GET)); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpErrorResponseTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpErrorResponseTest.java index 54a3db3d94d..56391127b62 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpErrorResponseTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpErrorResponseTest.java @@ -12,7 +12,6 @@ import static com.yahoo.jdisc.http.HttpResponse.Status.*; /** * @author Ulf Lilleengen - * @since 5.1 */ public class HttpErrorResponseTest { @Test @@ -29,7 +28,7 @@ public class HttpErrorResponseTest { } @Test - public void testThatHttpErrorResponseHasJsonContentType() throws IOException { + public void testThatHttpErrorResponseHasJsonContentType() { HttpErrorResponse response = HttpErrorResponse.badRequest("Error doing something"); assertThat(response.getContentType(), is("application/json")); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java index a1fbbb57ce2..3ae98c1b8f2 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java @@ -6,7 +6,6 @@ import com.yahoo.config.codegen.DefParser; import com.yahoo.config.codegen.InnerCNode; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.container.logging.AccessLog; import com.yahoo.text.StringUtilities; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.ConfigPayload; @@ -20,17 +19,14 @@ import java.io.IOException; import java.io.StringReader; import java.util.Collections; import java.util.HashSet; -import java.util.concurrent.Executor; import static org.hamcrest.core.Is.is; import static org.junit.Assert.*; import static com.yahoo.jdisc.http.HttpRequest.Method.GET; import static com.yahoo.jdisc.http.HttpResponse.Status.*; - /** * @author Ulf Lilleengen - * @since 5.1 */ public class HttpGetConfigHandlerTest { private static final String configUri = "http://yahoo.com:8080/config/v1/foo.bar/myid"; @@ -41,12 +37,10 @@ public class HttpGetConfigHandlerTest { @Before public void setUp() { mockRequestHandler = new MockRequestHandler(); - mockRequestHandler.setAllConfigs(new HashSet<ConfigKey<?>>() {{ + mockRequestHandler.setAllConfigs(new HashSet<>() {{ add(new ConfigKey<>("bar", "myid", "foo")); - }} ); - handler = new HttpGetConfigHandler( - HttpGetConfigHandler.testOnlyContext(), - mockRequestHandler); + }} ); + handler = new HttpGetConfigHandler(HttpGetConfigHandler.testOnlyContext(), mockRequestHandler); } @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpHandlerTest.java index e8d4373842d..e2bd8a120e0 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpHandlerTest.java @@ -46,7 +46,7 @@ public class HttpHandlerTest { private static class HttpTestHandler extends HttpHandler { private RuntimeException exception; - public HttpTestHandler(RuntimeException exception) { + HttpTestHandler(RuntimeException exception) { super(HttpHandler.testOnlyContext()); this.exception = exception; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java index 25b9e66ceb3..9113978d58b 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.config.server.http; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.container.logging.AccessLog; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.server.rpc.MockRequestHandler; import com.yahoo.vespa.config.server.http.HttpListConfigsHandler.ListConfigsResponse; @@ -13,7 +12,6 @@ import org.junit.Test; import java.io.IOException; import java.util.*; -import java.util.concurrent.Executor; import static org.hamcrest.core.Is.is; import static org.junit.Assert.*; @@ -23,7 +21,6 @@ import static com.yahoo.jdisc.http.HttpRequest.Method.GET; /** * @author Ulf Lilleengen - * @since 5.1 */ public class HttpListConfigsHandlerTest { @@ -34,9 +31,9 @@ public class HttpListConfigsHandlerTest { @Before public void setUp() { mockRequestHandler = new MockRequestHandler(); - mockRequestHandler.setAllConfigs(new HashSet<ConfigKey<?>>() {{ + mockRequestHandler.setAllConfigs(new HashSet<>() {{ add(new ConfigKey<>("bar", "conf/id/", "foo")); - }} ); + }} ); HttpListConfigsHandler.Context ctx = HttpListConfigsHandler.testOnlyContext(); handler = new HttpListConfigsHandler(ctx, mockRequestHandler); namedHandler = new HttpListNamedConfigsHandler(ctx, mockRequestHandler); @@ -51,14 +48,14 @@ public class HttpListConfigsHandlerTest { @Test public void require_that_named_handler_can_be_created() throws IOException { HttpRequest req = HttpRequest.createTestRequest("http://foo.com:8080/config/v1/foo.bar/conf/id/", GET); - req.getJDiscRequest().parameters().put("http.path", Arrays.asList("foo.bar")); + req.getJDiscRequest().parameters().put("http.path", List.of("foo.bar")); HttpResponse response = namedHandler.handle(req); assertThat(SessionHandlerTest.getRenderedString(response), is("{\"children\":[],\"configs\":[]}")); } @Test public void require_child_listings_correct() { - Set<ConfigKey<?>> keys = new LinkedHashSet<ConfigKey<?>>() {{ + Set<ConfigKey<?>> keys = new LinkedHashSet<>() {{ add(new ConfigKey<>("name1", "id/1", "ns1")); add(new ConfigKey<>("name1", "id/1", "ns1")); add(new ConfigKey<>("name1", "id/2", "ns1")); @@ -74,7 +71,7 @@ public class HttpListConfigsHandlerTest { @Test public void require_url_building_and_mimetype_correct() { - HttpListConfigsHandler.ListConfigsResponse resp = new ListConfigsResponse(new HashSet<ConfigKey<?>>(), null, "http://foo.com/config/v1/", true); + HttpListConfigsHandler.ListConfigsResponse resp = new ListConfigsResponse(new HashSet<>(), null, "http://foo.com/config/v1/", true); assertEquals(resp.toUrl(new ConfigKey<>("myconfig", "my/id", "mynamespace"), true), "http://foo.com/config/v1/mynamespace.myconfig/my/id"); assertEquals(resp.toUrl(new ConfigKey<>("myconfig", "my/id", "mynamespace"), false), "http://foo.com/config/v1/mynamespace.myconfig/my/id/"); assertEquals(resp.getContentType(), "application/json"); @@ -96,7 +93,7 @@ public class HttpListConfigsHandlerTest { @Test public void require_correct_error_response_on_no_model() throws IOException { - mockRequestHandler.setAllConfigs(new HashSet<ConfigKey<?>>()); + mockRequestHandler.setAllConfigs(new HashSet<>()); HttpResponse response = namedHandler.handle(HttpRequest.createTestRequest("http://yahoo.com:8080/config/v1/foo.bar/myid/", GET)); HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(response, NOT_FOUND, HttpErrorResponse.errorCodes.NOT_FOUND, diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java index 0381af57cc3..9a326a18dd5 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java @@ -6,7 +6,6 @@ import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.application.provider.FilesApplicationPackage; -import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterSpec; @@ -33,14 +32,13 @@ import com.yahoo.vespa.config.server.session.PrepareParams; import com.yahoo.vespa.config.server.session.Session; import com.yahoo.vespa.config.server.session.SessionContext; import com.yahoo.vespa.config.server.session.SessionFactory; -import com.yahoo.vespa.config.server.session.SessionPreparer; -import com.yahoo.vespa.config.server.session.SessionTest; import com.yahoo.vespa.flags.InMemoryFlagSource; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.time.Instant; import java.util.Collection; import java.util.List; @@ -80,16 +78,13 @@ public class SessionHandlerTest { public static String getRenderedString(HttpResponse response) throws IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); response.render(baos); - return baos.toString("UTF-8"); + return baos.toString(StandardCharsets.UTF_8); } public static class MockSession extends LocalSession { - private final InMemoryFlagSource flagSource; public boolean doVerboseLogging = false; public Session.Status status; - private final SessionPreparer preparer; - private final ApplicationPackage app; private ConfigChangeActions actions = new ConfigChangeActions(); private long createTime = System.currentTimeMillis() / 1000; private ApplicationId applicationId; @@ -99,10 +94,7 @@ public class SessionHandlerTest { } private MockSession(long id, ApplicationPackage app, InMemoryFlagSource flagSource) { - super(TenantName.defaultName(), id, null, new SessionContext(null, new MockSessionZKClient(MockApplicationPackage.createEmpty()), null, null, new HostRegistry<>(), flagSource)); - this.app = app; - this.preparer = new SessionTest.MockSessionPreparer(); - this.flagSource = flagSource; + super(TenantName.defaultName(), id, null, new SessionContext(app, new MockSessionZKClient(app), null, null, new HostRegistry<>(), flagSource)); } public MockSession(long sessionId, ApplicationPackage applicationPackage, long createTime) { @@ -140,31 +132,17 @@ public class SessionHandlerTest { @Override public Transaction createDeactivateTransaction() { - return new DummyTransaction().add((DummyTransaction.RunnableOperation) () -> { - status = Status.DEACTIVATE; - }); + return new DummyTransaction().add((DummyTransaction.RunnableOperation) () -> status = Status.DEACTIVATE); } @Override public Transaction createActivateTransaction() { - return new DummyTransaction().add((DummyTransaction.RunnableOperation) () -> { - status = Status.ACTIVATE; - }); + return new DummyTransaction().add((DummyTransaction.RunnableOperation) () -> status = Status.ACTIVATE); } @Override public ApplicationFile getApplicationFile(Path relativePath, Mode mode) { - if (mode == Mode.WRITE) { - status = Status.NEW; - } - if (preparer == null) { - return null; - } - ApplicationPackage pkg = app; - if (pkg == null) { - return null; - } - return pkg.getFile(relativePath); + return this.applicationPackage.getFile(relativePath); } @Override @@ -205,8 +183,7 @@ public class SessionHandlerTest { public File applicationPackage; @Override - public LocalSession createSession(File applicationDirectory, ApplicationId applicationId, - TimeoutBudget timeoutBudget) { + public LocalSession createSession(File applicationDirectory, ApplicationId applicationId, TimeoutBudget timeoutBudget) { createCalled = true; if (doThrow) { throw new RuntimeException("foo"); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java index c6a8e1f2f9d..b7f55aa0670 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java @@ -53,17 +53,17 @@ public class ApplicationContentHandlerTest extends ContentHandlerTestBase { tenantRepository.addTenant(TenantBuilder.create(componentRegistry, tenantName1)); tenantRepository.addTenant(TenantBuilder.create(componentRegistry, tenantName2)); - session2 = new MockSession(2l, FilesApplicationPackage.fromFile(new File("src/test/apps/content"))); + session2 = new MockSession(2, FilesApplicationPackage.fromFile(new File("src/test/apps/content"))); Tenant tenant1 = tenantRepository.getTenant(tenantName1); tenant1.getLocalSessionRepo().addSession(session2); tenant1.getApplicationRepo().createApplication(idTenant1); - tenant1.getApplicationRepo().createPutTransaction(idTenant1, 2l).commit(); + tenant1.getApplicationRepo().createPutTransaction(idTenant1, 2).commit(); - MockSession session3 = new MockSession(3l, FilesApplicationPackage.fromFile(new File("src/test/apps/content2"))); + MockSession session3 = new MockSession(3, FilesApplicationPackage.fromFile(new File("src/test/apps/content2"))); Tenant tenant2 = tenantRepository.getTenant(tenantName2); tenant2.getLocalSessionRepo().addSession(session3); tenant2.getApplicationRepo().createApplication(idTenant2); - tenant2.getApplicationRepo().createPutTransaction(idTenant2, 3l).commit(); + tenant2.getApplicationRepo().createPutTransaction(idTenant2, 3).commit(); handler = new ApplicationHandler(ApplicationHandler.testOnlyContext(), Zone.defaultZone(), diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java index bc583c64206..fb09aa99039 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java @@ -44,9 +44,9 @@ public class HttpGetConfigHandlerTest { @Before public void setUp() { mockRequestHandler = new MockRequestHandler(); - mockRequestHandler.setAllConfigs(new HashSet<ConfigKey<?>>() {{ + mockRequestHandler.setAllConfigs(new HashSet<>() {{ add(new ConfigKey<>("bar", "myid", "foo")); - }} ); + }} ); TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder().build(); TenantRepository tenantRepository = new TenantRepository(componentRegistry, false); tenantRepository.addTenant(TenantBuilder.create(componentRegistry, tenant).withRequestHandler(mockRequestHandler)); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpListConfigsHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpListConfigsHandlerTest.java index 750ad1c9fc0..7789c5d88db 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpListConfigsHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpListConfigsHandlerTest.java @@ -41,9 +41,9 @@ public class HttpListConfigsHandlerTest { @Before public void setUp() { mockRequestHandler = new MockRequestHandler(); - mockRequestHandler.setAllConfigs(new HashSet<ConfigKey<?>>() {{ + mockRequestHandler.setAllConfigs(new HashSet<>() {{ add(new ConfigKey<>("bar", "conf/id", "foo")); - }} ); + }} ); TenantName tenantName = TenantName.from("mytenant"); TenantRepository tenantRepository = new TenantRepository(componentRegistry, false); TenantBuilder tenantBuilder = TenantBuilder.create(componentRegistry, tenantName) @@ -73,7 +73,7 @@ public class HttpListConfigsHandlerTest { @Test public void require_that_named_handler_can_be_created() throws IOException { HttpRequest req = HttpRequest.createTestRequest("http://foo.com:8080/config/v2/tenant/mytenant/application/myapplication/foo.bar/conf/id/", GET); - req.getJDiscRequest().parameters().put("http.path", Arrays.asList("foo.bar")); + req.getJDiscRequest().parameters().put("http.path", List.of("foo.bar")); HttpResponse response = namedHandler.handle(req); assertThat(SessionHandlerTest.getRenderedString(response), is("{\"children\":[],\"configs\":[]}")); } @@ -81,14 +81,14 @@ public class HttpListConfigsHandlerTest { @Test public void require_that_named_handler_can_be_created_from_full_appid() throws IOException { HttpRequest req = HttpRequest.createTestRequest("http://foo.com:8080/config/v2/tenant/mytenant/application/myapplication/environment/prod/region/myregion/instance/myinstance/foo.bar/conf/id/", GET); - req.getJDiscRequest().parameters().put("http.path", Arrays.asList("foo.bar")); + req.getJDiscRequest().parameters().put("http.path", List.of("foo.bar")); HttpResponse response = namedHandler.handle(req); assertThat(SessionHandlerTest.getRenderedString(response), is("{\"children\":[],\"configs\":[]}")); } @Test public void require_child_listings_correct() { - Set<ConfigKey<?>> keys = new LinkedHashSet<ConfigKey<?>>() {{ + Set<ConfigKey<?>> keys = new LinkedHashSet<>() {{ add(new ConfigKey<>("name1", "id/1", "ns1")); add(new ConfigKey<>("name1", "id/1", "ns1")); add(new ConfigKey<>("name1", "id/2", "ns1")); @@ -104,7 +104,7 @@ public class HttpListConfigsHandlerTest { @Test public void require_url_building_and_mimetype_correct() { - ListConfigsResponse resp = new ListConfigsResponse(new HashSet<ConfigKey<?>>(), null, "http://foo.com/config/v2/tenant/mytenant/application/mya/", true); + ListConfigsResponse resp = new ListConfigsResponse(new HashSet<>(), null, "http://foo.com/config/v2/tenant/mytenant/application/mya/", true); assertEquals(resp.toUrl(new ConfigKey<>("myconfig", "my/id", "mynamespace"), true), "http://foo.com/config/v2/tenant/mytenant/application/mya/mynamespace.myconfig/my/id"); assertEquals(resp.toUrl(new ConfigKey<>("myconfig", "my/id", "mynamespace"), false), "http://foo.com/config/v2/tenant/mytenant/application/mya/mynamespace.myconfig/my/id/"); assertEquals(resp.toUrl(new ConfigKey<>("myconfig", "", "mynamespace"), false), "http://foo.com/config/v2/tenant/mytenant/application/mya/mynamespace.myconfig"); @@ -128,7 +128,7 @@ public class HttpListConfigsHandlerTest { @Test public void require_correct_error_response_on_no_model() throws IOException { - mockRequestHandler.setAllConfigs(new HashSet<ConfigKey<?>>()); + mockRequestHandler.setAllConfigs(new HashSet<>()); HttpResponse response = namedHandler.handle(HttpRequest.createTestRequest("http://yahoo.com:8080/config/v2/tenant/mytenant/application/myapplication/foo.bar/myid/", GET)); HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(response, NOT_FOUND, HttpErrorResponse.errorCodes.NOT_FOUND, @@ -137,7 +137,7 @@ public class HttpListConfigsHandlerTest { @Test public void require_correct_configid_parent() { - assertEquals(ListConfigsResponse.parentConfigId(null), null); + assertNull(ListConfigsResponse.parentConfigId(null)); assertEquals(ListConfigsResponse.parentConfigId("foo"), ""); assertEquals(ListConfigsResponse.parentConfigId(""), ""); assertEquals(ListConfigsResponse.parentConfigId("/"), ""); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java index 7fa417ea287..52ecc5e2bae 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java @@ -36,7 +36,6 @@ import com.yahoo.vespa.config.server.session.LocalSession; import com.yahoo.vespa.config.server.session.LocalSessionRepo; import com.yahoo.vespa.config.server.session.MockSessionZKClient; import com.yahoo.vespa.config.server.session.RemoteSession; -import com.yahoo.vespa.config.server.session.RemoteSessionRepo; import com.yahoo.vespa.config.server.session.Session; import com.yahoo.vespa.config.server.session.SessionContext; import com.yahoo.vespa.config.server.session.SessionTest; @@ -57,7 +56,6 @@ import org.junit.rules.TemporaryFolder; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; -import java.time.Clock; import java.util.Collections; import java.util.Optional; @@ -81,7 +79,6 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { private final InMemoryFlagSource flagSource = new InMemoryFlagSource(); private Curator curator; - private RemoteSessionRepo remoteSessionRepo; private LocalSessionRepo localRepo; private TenantApplications applicationRepo; private MockProvisioner hostProvisioner; @@ -95,7 +92,6 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { @Before public void setup() { - remoteSessionRepo = new RemoteSessionRepo(tenantName); curator = new MockCurator(); modelFactory = new VespaModelFactory(new NullConfigModelRegistry()); componentRegistry = new TestComponentRegistry.Builder() @@ -110,7 +106,6 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { TenantBuilder tenantBuilder = TenantBuilder.create(componentRegistry, tenantName) .withSessionFactory(new MockSessionFactory()) .withLocalSessionRepo(localRepo) - .withRemoteSessionRepo(remoteSessionRepo) .withApplicationRepo(applicationRepo); tenantRepository.addTenant(tenantBuilder); handler = createHandler(); @@ -118,15 +113,15 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { @Test public void testThatPreviousSessionIsDeactivated() throws Exception { - RemoteSession firstSession = activateAndAssertOK(90l, 0l); - activateAndAssertOK(91l, 90l); + RemoteSession firstSession = activateAndAssertOK(90, 0); + activateAndAssertOK(91, 90); assertThat(firstSession.getStatus(), Is.is(Session.Status.DEACTIVATE)); } @Test public void testForceActivationWithActivationInBetween() throws Exception { - activateAndAssertOK(90l, 0l); - activateAndAssertOK(92l, 89l, "?force=true"); + activateAndAssertOK(90, 0); + activateAndAssertOK(92, 89, "?force=true"); } @Test @@ -138,9 +133,9 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { @Test public void testActivationWithBarrierTimeout() throws Exception { // Needed so we can test that previous active session is still active after a failed activation - activateAndAssertOK(90l, 0l); + activateAndAssertOK(90, 0); ((MockCurator) curator).timeoutBarrierOnEnter(true); - ActivateRequest activateRequest = new ActivateRequest(91l, 90l, "", Clock.systemUTC()).invoke(); + ActivateRequest activateRequest = new ActivateRequest(91, 90, "").invoke(); HttpResponse actResponse = activateRequest.getActResponse(); assertThat(actResponse.getStatus(), Is.is(INTERNAL_SERVER_ERROR)); } @@ -153,7 +148,7 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { activateAndAssertOK(sessionId, 1); sessionId++; - ActivateRequest activateRequest = new ActivateRequest(sessionId, 1, "", componentRegistry.getClock()).invoke(); + ActivateRequest activateRequest = new ActivateRequest(sessionId, 1, "").invoke(); HttpResponse actResponse = activateRequest.getActResponse(); String message = getRenderedString(actResponse); assertThat(message, actResponse.getStatus(), Is.is(CONFLICT)); @@ -164,7 +159,7 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { @Test public void testAlreadyActivatedSession() throws Exception { activateAndAssertOK(1, 0); - HttpResponse response = handler.handle(SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.ACTIVE, 1l)); + HttpResponse response = handler.handle(SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.ACTIVE, 1L)); String message = getRenderedString(response); assertThat(message, response.getStatus(), Is.is(BAD_REQUEST)); assertThat(message, containsString("Session 1 is already active")); @@ -177,8 +172,8 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { @Test public void testActivationWithActivationInBetween() throws Exception { - activateAndAssertOK(90l, 0l); - activateAndAssertError(92l, 89l, componentRegistry.getClock(), + activateAndAssertOK(90, 0); + activateAndAssertError(92, 89, Response.Status.CONFLICT, HttpErrorResponse.errorCodes.ACTIVATION_CONFLICT, "tenant:" + tenantName + " app:default:default Cannot activate session 92 because the currently active session (90) has changed since session 92 was created (was 89 at creation time)"); } @@ -186,9 +181,9 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { @Test public void testActivationOfUnpreparedSession() throws Exception { // Needed so we can test that previous active session is still active after a failed activation - RemoteSession firstSession = activateAndAssertOK(90l, 0l); + RemoteSession firstSession = activateAndAssertOK(90, 0); long sessionId = 91L; - ActivateRequest activateRequest = new ActivateRequest(sessionId, 0l, Session.Status.NEW, "", componentRegistry.getClock()).invoke(); + ActivateRequest activateRequest = new ActivateRequest(sessionId, 0, Session.Status.NEW, "").invoke(); HttpResponse actResponse = activateRequest.getActResponse(); RemoteSession session = activateRequest.getSession(); assertThat(actResponse.getStatus(), is(Response.Status.BAD_REQUEST)); @@ -209,7 +204,7 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { public void require_that_handler_gives_error_when_provisioner_activated_fails() throws Exception { hostProvisioner = new FailingMockProvisioner(); hostProvisioner.activated = false; - activateAndAssertError(1, 0, componentRegistry.getClock(), BAD_REQUEST, HttpErrorResponse.errorCodes.BAD_REQUEST, "Cannot activate application"); + activateAndAssertError(1, 0, BAD_REQUEST, HttpErrorResponse.errorCodes.BAD_REQUEST, "Cannot activate application"); assertFalse(hostProvisioner.activated); } @@ -219,9 +214,7 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { TenantRepository.getSessionsPath(tenantName).append(String.valueOf(sessionId))); zkC.write(Collections.singletonMap(modelFactory.version(), new MockFileRegistry())); zkC.write(AllocatedHosts.withHosts(Collections.emptySet())); - RemoteSession session = new RemoteSession(tenantName, sessionId, componentRegistry, zkClient); - remoteSessionRepo.addSession(session); - return session; + return new RemoteSession(tenantName, sessionId, componentRegistry, zkClient); } private void addLocalSession(long sessionId, DeployData deployData, SessionZooKeeperClient zkc) throws IOException { @@ -235,7 +228,7 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { } private ActivateRequest activateAndAssertOKPut(long sessionId, long previousSessionId, String subPath) throws Exception { - ActivateRequest activateRequest = new ActivateRequest(sessionId, previousSessionId, subPath, componentRegistry.getClock()); + ActivateRequest activateRequest = new ActivateRequest(sessionId, previousSessionId, subPath); activateRequest.invoke(); HttpResponse actResponse = activateRequest.getActResponse(); String message = getRenderedString(actResponse); @@ -246,9 +239,9 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { return activateRequest; } - private void activateAndAssertErrorPut(long sessionId, long previousSessionId, Clock clock, + private void activateAndAssertErrorPut(long sessionId, long previousSessionId, int statusCode, HttpErrorResponse.errorCodes errorCode, String expectedError) throws Exception { - ActivateRequest activateRequest = new ActivateRequest(sessionId, previousSessionId, "", clock); + ActivateRequest activateRequest = new ActivateRequest(sessionId, previousSessionId, ""); activateRequest.invoke(); HttpResponse actResponse = activateRequest.getActResponse(); RemoteSession session = activateRequest.getSession(); @@ -275,24 +268,22 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { private DeployData deployData; private ApplicationMetaData metaData; private String subPath; - private Clock clock; - ActivateRequest(long sessionId, long previousSessionId, String subPath, Clock clock) { - this(sessionId, previousSessionId, Session.Status.PREPARE, subPath, clock); + ActivateRequest(long sessionId, long previousSessionId, String subPath) { + this(sessionId, previousSessionId, Session.Status.PREPARE, subPath); } - ActivateRequest(long sessionId, long previousSessionId, Session.Status initialStatus, String subPath, Clock clock) { + ActivateRequest(long sessionId, long previousSessionId, Session.Status initialStatus, String subPath) { this.sessionId = sessionId; this.initialStatus = initialStatus; this.deployData = new DeployData("foo", "bar", appName, - 0l, + 0L, false, sessionId, previousSessionId); this.subPath = subPath; - this.clock = clock; } public RemoteSession getSession() { @@ -352,9 +343,9 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { assertThat(hostProvisioner.lastHosts.size(), is(1)); } - private void activateAndAssertError(long sessionId, long previousSessionId, Clock clock, int statusCode, HttpErrorResponse.errorCodes errorCode, String expectedError) throws Exception { + private void activateAndAssertError(long sessionId, long previousSessionId, int statusCode, HttpErrorResponse.errorCodes errorCode, String expectedError) throws Exception { hostProvisioner.activated = false; - activateAndAssertErrorPut(sessionId, previousSessionId, clock, statusCode, errorCode, expectedError); + activateAndAssertErrorPut(sessionId, previousSessionId, statusCode, errorCode, expectedError); assertFalse(hostProvisioner.activated); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java index 42b3fadc0de..d4dd17ca280 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java @@ -24,7 +24,6 @@ import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.time.Clock; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertNotNull; @@ -37,7 +36,6 @@ public class SessionContentHandlerTest extends ContentHandlerTestBase { private static final TenantName tenant = TenantName.from("contenttest"); private final TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder().build(); - private final Clock clock = componentRegistry.getClock(); private TenantRepository tenantRepository; private SessionContentHandler handler = null; @@ -59,10 +57,11 @@ public class SessionContentHandlerTest extends ContentHandlerTestBase { assertMkdir("/bar/brask/"); assertMkdir("/bar/brask/bram/"); assertMkdir("/brask/og/bram/"); - }// TODO: Enable when we have a predictable way of checking request body existence. + } @Test @Ignore + // TODO: Enable when we have a predictable way of checking request body existence. public void require_that_mkdir_with_body_is_illegal(){ HttpResponse response = put("/foobio/", "foo"); assertNotNull(response); @@ -70,15 +69,15 @@ public class SessionContentHandlerTest extends ContentHandlerTestBase { } @Test - public void require_that_nonexistant_session_returns_not_found() { - HttpResponse response = doRequest(HttpRequest.Method.GET, "/test.txt", 2l); + public void require_that_nonexistent_session_returns_not_found() { + HttpResponse response = doRequest(HttpRequest.Method.GET, "/test.txt", 2); assertNotNull(response); assertThat(response.getStatus(), is(Response.Status.NOT_FOUND)); } protected HttpResponse put(String path, String content) { ByteArrayInputStream data = new ByteArrayInputStream(Utf8.toBytes(content)); - return doRequest(HttpRequest.Method.PUT, path, data); + return doRequest(HttpRequest.Method.PUT, path, 1, data); } @Test @@ -95,7 +94,7 @@ public class SessionContentHandlerTest extends ContentHandlerTestBase { } @Test - public void require_that_nonexistant_file_returs_not_found_when_deleted() throws IOException { + public void require_that_nonexistent_file_returns_not_found_when_deleted() throws IOException { assertDeleteFile(Response.Status.NOT_FOUND, "/test2.txt", "{\"error-code\":\"NOT_FOUND\",\"message\":\"Session 1 does not contain a file 'test2.txt'\"}"); } @@ -152,17 +151,13 @@ public class SessionContentHandlerTest extends ContentHandlerTestBase { } protected HttpResponse doRequest(HttpRequest.Method method, String path) { - return doRequest(method, path, 1l); + return doRequest(method, path, 1); } private HttpResponse doRequest(HttpRequest.Method method, String path, long sessionId) { return handler.handle(SessionHandlerTest.createTestRequest(pathPrefix, method, Cmd.CONTENT, sessionId, path)); } - private HttpResponse doRequest(HttpRequest.Method method, String path, InputStream data) { - return doRequest(method, path, 1l, data); - } - private HttpResponse doRequest(HttpRequest.Method method, String path, long sessionId, InputStream data) { return handler.handle(SessionHandlerTest.createTestRequest(pathPrefix, method, Cmd.CONTENT, sessionId, path, data)); } @@ -173,7 +168,7 @@ public class SessionContentHandlerTest extends ContentHandlerTestBase { new ApplicationRepository(tenantRepository, new SessionHandlerTest.MockProvisioner(), new OrchestratorMock(), - clock), + componentRegistry.getClock()), tenantRepository); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java index 06e1c36b3f2..6d2c2d35ab9 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java @@ -26,7 +26,6 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; -import java.time.Clock; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -51,7 +50,6 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { private static final HashMap<String, String> postHeaders = new HashMap<>(); private final TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder().build(); - private final Clock clock = componentRegistry.getClock(); private String pathPrefix = "/application/v2/session/"; private String createdMessage = " created.\""; @@ -179,13 +177,12 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { public void require_that_session_is_stored_in_repo() throws IOException { File outFile = CompressedApplicationInputStreamTest.createTarFile(); createHandler().handle(post(outFile)); - assertNotNull(localSessionRepo.getSession(0l)); + assertNotNull(localSessionRepo.getSession(0)); } - @Test public void require_that_application_urls_can_be_given_as_from_parameter() throws Exception { - localSessionRepo.addSession(new SessionHandlerTest.MockSession(2l, FilesApplicationPackage.fromFile(testApp))); + localSessionRepo.addSession(new SessionHandlerTest.MockSession(2, FilesApplicationPackage.fromFile(testApp))); ApplicationId fooId = new ApplicationId.Builder() .tenant(tenant) .applicationName("foo") @@ -194,7 +191,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { applicationRepo.createApplication(fooId); applicationRepo.createPutTransaction(fooId, 2).commit(); assertFromParameter("3", "http://myhost:40555/application/v2/tenant/" + tenant + "/application/foo/environment/test/region/baz/instance/quux"); - localSessionRepo.addSession(new SessionHandlerTest.MockSession(5l, FilesApplicationPackage.fromFile(testApp))); + localSessionRepo.addSession(new SessionHandlerTest.MockSession(5, FilesApplicationPackage.fromFile(testApp))); ApplicationId bioId = new ApplicationId.Builder() .tenant(tenant) .applicationName("foobio") @@ -221,7 +218,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { new ApplicationRepository(tenantRepository, new SessionHandlerTest.MockProvisioner(), new OrchestratorMock(), - clock), + componentRegistry.getClock()), tenantRepository, componentRegistry.getConfigserverConfig()); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java index ec64ef2e3ba..e201ce107bd 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java @@ -46,9 +46,10 @@ import java.util.Collections; import java.util.List; import java.util.Optional; -import static com.yahoo.jdisc.Response.Status.*; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; +import static com.yahoo.jdisc.Response.Status.METHOD_NOT_ALLOWED; import static com.yahoo.jdisc.Response.Status.NOT_FOUND; +import static com.yahoo.jdisc.Response.Status.OK; import static com.yahoo.vespa.config.server.http.HandlerTest.assertHttpStatusCodeErrorCodeAndMessage; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; @@ -70,7 +71,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { private String preparedMessage = " prepared.\"}"; private String tenantMessage = ""; - private RemoteSessionRepo remoteSessionRepo; private TenantRepository tenantRepository; @Before @@ -80,11 +80,9 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { preparedMessage = " for tenant '" + tenant + "' prepared.\""; tenantMessage = ",\"tenant\":\"" + tenant + "\""; tenantRepository = new TenantRepository(componentRegistry, false); - remoteSessionRepo = new RemoteSessionRepo(tenant); TenantBuilder tenantBuilder = TenantBuilder.create(componentRegistry, tenant) .withSessionFactory(new MockSessionFactory()) .withLocalSessionRepo(localRepo) - .withRemoteSessionRepo(remoteSessionRepo) .withApplicationRepo(TenantApplications.create(componentRegistry, new MockReloadHandler(), tenant)); tenantRepository.addTenant(tenantBuilder); } @@ -149,21 +147,8 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { assertThat(SessionHandlerTest.getRenderedString(response), containsString("debuglog")); } - /** - * A mock remote session repo based on contents of local repo. Only works when there is just one session in local repo - */ - // TODO: Fix this mess - private SessionZooKeeperClient fromLocalSessionRepo(LocalSessionRepo localRepo) { - SessionZooKeeperClient zooKeeperClient = null; - for (LocalSession ls : localRepo.listSessions()) { - zooKeeperClient = new MockSessionZKClient(curator, tenant, ls.getSessionId()); - if (ls.getStatus()!=null) zooKeeperClient.writeStatus(ls.getStatus()); - RemoteSession remSess = new RemoteSession(tenant, ls.getSessionId(), - new TestComponentRegistry.Builder().curator(curator).build(), - zooKeeperClient); - remoteSessionRepo.addSession(remSess); - } - return zooKeeperClient; + private SessionZooKeeperClient createSessionZooKeeperClient(LocalSession session) { + return new MockSessionZKClient(curator, tenant, session.getSessionId()); } @Test @@ -173,7 +158,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { SessionHandler sessHandler = createHandler(); sessHandler.handle(SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.PREPARED, 1L)); session.setStatus(Session.Status.PREPARE); - SessionZooKeeperClient zooKeeperClient = fromLocalSessionRepo(localRepo); + SessionZooKeeperClient zooKeeperClient = createSessionZooKeeperClient(session); zooKeeperClient.writeStatus(Session.Status.PREPARE); HttpResponse getResponse = sessHandler.handle( SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.GET, Cmd.PREPARED, 1L)); @@ -187,7 +172,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { localRepo.addSession(session); SessionHandler sessHandler = createHandler(); session.setStatus(Session.Status.NEW); - SessionZooKeeperClient zooKeeperClient = fromLocalSessionRepo(localRepo); + SessionZooKeeperClient zooKeeperClient = createSessionZooKeeperClient(session); zooKeeperClient.writeStatus(Session.Status.NEW); HttpResponse getResponse = sessHandler.handle( SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.GET, Cmd.PREPARED, 1L)); @@ -247,7 +232,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { LocalSessionRepo localRepoDefault = new LocalSessionRepo(defaultTenant, componentRegistry); TenantBuilder defaultTenantBuilder = TenantBuilder.create(componentRegistry, defaultTenant) .withLocalSessionRepo(localRepoDefault) - .withRemoteSessionRepo(new RemoteSessionRepo(defaultTenant)) .withSessionFactory(new MockSessionFactory()); tenantRepository.addTenant(defaultTenantBuilder); final SessionHandler handler = createHandler(); @@ -324,7 +308,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } @Test - public void test_out_of_capacity_response() throws InterruptedException, IOException { + public void test_out_of_capacity_response() throws IOException { String message = "Internal error"; SessionThrowingException session = new SessionThrowingException(new OutOfCapacityException(message)); localRepo.addSession(session); @@ -337,7 +321,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } @Test - public void test_that_nullpointerexception_gives_internal_server_error() throws InterruptedException, IOException { + public void test_that_nullpointerexception_gives_internal_server_error() throws IOException { String message = "No nodes available"; SessionThrowingException session = new SessionThrowingException(new NullPointerException(message)); localRepo.addSession(session); @@ -350,7 +334,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } @Test - public void test_application_lock_failure() throws InterruptedException, IOException { + public void test_application_lock_failure() throws IOException { String message = "Timed out after waiting PT1M to acquire lock '/provision/v1/locks/foo/bar/default'"; SessionThrowingException session = new SessionThrowingException(new ApplicationLockException(new UncheckedTimeoutException(message))); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/TenantHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/TenantHandlerTest.java index 6effa3359b1..9d843a1f2c6 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/TenantHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/TenantHandlerTest.java @@ -6,6 +6,7 @@ import static org.junit.Assert.*; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.time.Clock; import com.yahoo.config.provision.ApplicationId; @@ -139,7 +140,7 @@ public class TenantHandlerTest { private void assertResponseEquals(SessionResponse response, String payload) throws IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); response.render(baos); - assertEquals(baos.toString("UTF-8"), payload); + assertEquals(baos.toString(StandardCharsets.UTF_8), payload); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java index 659baf5a184..c2489a1d3e9 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java @@ -10,8 +10,6 @@ import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; -import java.time.Clock; - class MaintainerTester { private final Curator curator; @@ -25,7 +23,7 @@ class MaintainerTester { applicationRepository = new ApplicationRepository(tenantRepository, new SessionHandlerTest.MockProvisioner(), new OrchestratorMock(), - Clock.systemUTC()); + componentRegistry.getClock()); } Curator curator() { return curator; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java index 34a4074c7cf..9b678b9b37f 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java @@ -28,7 +28,6 @@ import org.xml.sax.SAXException; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -212,7 +211,7 @@ public class LbServicesProducerTest { private ApplicationInfo createApplication(ApplicationId appId, DeployState.Builder deploystateBuilder) throws IOException, SAXException { return new ApplicationInfo( appId, - 3l, + 3, createVespaModel(createApplicationPackage( appId.tenant() + "." + appId.application() + ".yahoo.com", appId.tenant().value() + "." + appId.application().value() + "2.yahoo.com"), deploystateBuilder)); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/model/RoutingProducerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/model/RoutingProducerTest.java index 7ce2c39c6a4..5a34dd4c912 100755 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/model/RoutingProducerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/model/RoutingProducerTest.java @@ -66,7 +66,7 @@ public class RoutingProducerTest { private ApplicationInfo createApplication(ApplicationId appId, DeployState.Builder deploystateBuilder) throws IOException, SAXException { return new ApplicationInfo( appId, - 3l, + 3L, createVespaModel( createApplicationPackage( appId.tenant() + "." + appId.application() + ".yahoo.com", diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdaterTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdaterTest.java index 28c6dddd5a7..ed089109759 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdaterTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdaterTest.java @@ -62,8 +62,7 @@ public class ZKMetricUpdaterTest { private ZKMetricUpdater buildUpdater() { ZookeeperServerConfig zkServerConfig = new ZookeeperServerConfig( new ZookeeperServerConfig.Builder().clientPort(serverPort).myid(12345)); - ZKMetricUpdater updater = new ZKMetricUpdater(zkServerConfig, 0, -1); - return updater; + return new ZKMetricUpdater(zkServerConfig, 0, -1); } private void setupTcpServer(Supplier<String> reportProvider) throws IOException { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java index 819078d296b..87459228d0d 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java @@ -17,11 +17,13 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import java.io.IOException; +import java.time.Duration; import java.util.Collections; import java.util.List; import java.util.Optional; import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -52,7 +54,7 @@ public class DelayedConfigResponseTest { assertThat(responses.size(), is(2)); assertTrue(req.isDelayedResponse()); List<DelayedConfigResponses.DelayedConfigResponse> it = responses.allDelayedResponses(); - assertTrue(!it.isEmpty()); + assertFalse(it.isEmpty()); } @Test @@ -74,7 +76,7 @@ public class DelayedConfigResponseTest { assertThat(responses.toString(), is("DelayedConfigResponses. Average Size=0")); JRTServerConfigRequest req = createRequest("foo", "md5", "myid", "mymd5", 3, 100, "bar"); responses.delayResponse(req, GetConfigContext.testContext(ApplicationId.defaultId())); - rpc.waitUntilSet(5000); + rpc.waitUntilSet(Duration.ofSeconds(5)); assertThat(rpc.latestRequest, is(req)); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java index 5fa51e1c404..07f6e9cf222 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java @@ -15,6 +15,8 @@ import com.yahoo.vespa.config.server.rpc.security.NoopRpcAuthorizer; import com.yahoo.vespa.config.server.tenant.MockTenantProvider; import java.io.File; +import java.time.Duration; +import java.time.Instant; import java.util.Optional; import java.util.concurrent.CompletionService; @@ -67,10 +69,9 @@ public class MockRpc extends RpcServer { return new ConfigserverConfig(b); } - public boolean waitUntilSet(int timeout) { - long start = System.currentTimeMillis(); - long end = start + timeout; - while (start < end) { + boolean waitUntilSet(Duration timeout) { + Instant end = Instant.now().plus(timeout); + while (Instant.now().isBefore(end)) { if (latestRequest != null) return true; try { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionRepoTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionRepoTest.java index 23fde190297..9232109a89b 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionRepoTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionRepoTest.java @@ -74,42 +74,42 @@ public class LocalSessionRepoTest { @Test public void require_that_sessions_can_be_loaded_from_disk() { - assertNotNull(repo.getSession(1l)); - assertNotNull(repo.getSession(2l)); - assertNotNull(repo.getSession(3l)); - assertNull(repo.getSession(4l)); + assertNotNull(repo.getSession(1L)); + assertNotNull(repo.getSession(2L)); + assertNotNull(repo.getSession(3L)); + assertNull(repo.getSession(4L)); } @Test public void require_that_old_sessions_are_purged() { clock.advance(Duration.ofSeconds(1)); - assertNotNull(repo.getSession(1l)); - assertNotNull(repo.getSession(2l)); - assertNotNull(repo.getSession(3l)); + assertNotNull(repo.getSession(1L)); + assertNotNull(repo.getSession(2L)); + assertNotNull(repo.getSession(3L)); clock.advance(Duration.ofSeconds(1)); - assertNotNull(repo.getSession(1l)); - assertNotNull(repo.getSession(2l)); - assertNotNull(repo.getSession(3l)); + assertNotNull(repo.getSession(1L)); + assertNotNull(repo.getSession(2L)); + assertNotNull(repo.getSession(3L)); clock.advance(Duration.ofSeconds(1)); - addSession(4l, 6); - assertNotNull(repo.getSession(1l)); - assertNotNull(repo.getSession(2l)); - assertNotNull(repo.getSession(3l)); - assertNotNull(repo.getSession(4l)); + addSession(4L, 6); + assertNotNull(repo.getSession(1L)); + assertNotNull(repo.getSession(2L)); + assertNotNull(repo.getSession(3L)); + assertNotNull(repo.getSession(4L)); clock.advance(Duration.ofSeconds(1)); - addSession(5l, 10); + addSession(5L, 10); repo.purgeOldSessions(); - assertNull(repo.getSession(1l)); - assertNull(repo.getSession(2l)); - assertNull(repo.getSession(3l)); + assertNull(repo.getSession(1L)); + assertNull(repo.getSession(2L)); + assertNull(repo.getSession(3L)); } @Test public void require_that_all_sessions_are_deleted() { repo.close(); - assertNull(repo.getSession(1l)); - assertNull(repo.getSession(2l)); - assertNull(repo.getSession(3l)); + assertNull(repo.getSession(1L)); + assertNull(repo.getSession(2L)); + assertNull(repo.getSession(3L)); } private void addSession(long sessionId, long createTime) { @@ -119,10 +119,10 @@ public class LocalSessionRepoTest { @Test public void require_that_sessions_belong_to_a_tenant() { // tenant is "default" - assertNotNull(repo.getSession(1l)); - assertNotNull(repo.getSession(2l)); - assertNotNull(repo.getSession(3l)); - assertNull(repo.getSession(4l)); + assertNotNull(repo.getSession(1L)); + assertNotNull(repo.getSession(2L)); + assertNotNull(repo.getSession(3L)); + assertNull(repo.getSession(4L)); // tenant is "newTenant" try { @@ -130,12 +130,12 @@ public class LocalSessionRepoTest { } catch (Exception e) { fail(); } - assertNull(repo.getSession(1l)); + assertNull(repo.getSession(1L)); - repo.addSession(new SessionHandlerTest.MockSession(1l, FilesApplicationPackage.fromFile(testApp))); - repo.addSession(new SessionHandlerTest.MockSession(2l, FilesApplicationPackage.fromFile(testApp))); - assertNotNull(repo.getSession(1l)); - assertNotNull(repo.getSession(2l)); - assertNull(repo.getSession(3l)); + repo.addSession(new SessionHandlerTest.MockSession(1L, FilesApplicationPackage.fromFile(testApp))); + repo.addSession(new SessionHandlerTest.MockSession(2L, FilesApplicationPackage.fromFile(testApp))); + assertNotNull(repo.getSession(1L)); + assertNotNull(repo.getSession(2L)); + assertNull(repo.getSession(3L)); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java index 96caff9b3a7..b357f712004 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java @@ -65,17 +65,17 @@ public class LocalSessionTest { @Test public void require_that_session_is_initialized() throws Exception { LocalSession session = createSession(TenantName.defaultName(), 2); - assertThat(session.getSessionId(), is(2l)); + assertThat(session.getSessionId(), is(2L)); session = createSession(TenantName.defaultName(), Long.MAX_VALUE); assertThat(session.getSessionId(), is(Long.MAX_VALUE)); - assertThat(session.getActiveSessionAtCreate(), is(0l)); + assertThat(session.getActiveSessionAtCreate(), is(0L)); } @Test public void require_that_session_status_is_updated() throws Exception { LocalSession session = createSession(TenantName.defaultName(), 3); assertThat(session.getStatus(), is(Session.Status.NEW)); - doPrepare(session, Instant.now()); + doPrepare(session); assertThat(session.getStatus(), is(Session.Status.PREPARE)); session.createActivateTransaction().commit(); assertThat(session.getStatus(), is(Session.Status.ACTIVATE)); @@ -84,7 +84,7 @@ public class LocalSessionTest { @Test public void require_that_marking_session_modified_changes_status_to_new() throws Exception { LocalSession session = createSession(TenantName.defaultName(), 3); - doPrepare(session, Instant.now()); + doPrepare(session); assertThat(session.getStatus(), is(Session.Status.PREPARE)); session.getApplicationFile(Path.createRoot(), LocalSession.Mode.READ); assertThat(session.getStatus(), is(Session.Status.PREPARE)); @@ -97,7 +97,7 @@ public class LocalSessionTest { SessionTest.MockSessionPreparer preparer = new SessionTest.MockSessionPreparer(); LocalSession session = createSession(TenantName.defaultName(), 3, preparer); assertFalse(preparer.isPrepared); - doPrepare(session, Instant.now()); + doPrepare(session); assertTrue(preparer.isPrepared); assertThat(session.getStatus(), is(Session.Status.PREPARE)); } @@ -148,7 +148,7 @@ public class LocalSessionTest { NetworkPorts ports = new NetworkPorts(list); AllocatedHosts input = AllocatedHosts.withHosts(Collections.singleton( - new HostSpec("myhost", Collections.<String>emptyList(), + new HostSpec("myhost", Collections.emptyList(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(ports)))); @@ -156,7 +156,7 @@ public class LocalSessionTest { ApplicationId origId = new ApplicationId.Builder() .tenant("tenant") .applicationName("foo").instanceName("quux").build(); - doPrepare(session, new PrepareParams.Builder().applicationId(origId).build(), Instant.now()); + doPrepare(session, new PrepareParams.Builder().applicationId(origId).build()); AllocatedHosts info = session.getAllocatedHosts(); assertNotNull(info); assertThat(info.getHosts().size(), is(1)); @@ -169,7 +169,7 @@ public class LocalSessionTest { @Test public void require_that_application_metadata_is_correct() throws Exception { LocalSession session = createSession(TenantName.defaultName(), 3); - doPrepare(session, new PrepareParams.Builder().build(), Instant.now()); + doPrepare(session, new PrepareParams.Builder().build()); assertThat(session.getMetaData().toString(), is("n/a, n/a, 0, 0, , 0")); } @@ -179,7 +179,7 @@ public class LocalSessionTest { } private LocalSession createSession(TenantName tenant, long sessionId, SessionTest.MockSessionPreparer preparer) throws Exception { - return createSession(tenant, sessionId, preparer, Optional.<AllocatedHosts>empty()); + return createSession(tenant, sessionId, preparer, Optional.empty()); } private LocalSession createSession(TenantName tenant, long sessionId, SessionTest.MockSessionPreparer preparer, Optional<AllocatedHosts> allocatedHosts) throws Exception { @@ -204,16 +204,16 @@ public class LocalSessionTest { flagSource)); } - private void doPrepare(LocalSession session, Instant now) { - doPrepare(session, new PrepareParams.Builder().build(), now); + private void doPrepare(LocalSession session) { + doPrepare(session, new PrepareParams.Builder().build()); } - private void doPrepare(LocalSession session, PrepareParams params, Instant now) { - session.prepare(getLogger(false), params, Optional.empty(), tenantPath, now); + private void doPrepare(LocalSession session, PrepareParams params) { + session.prepare(getLogger(), params, Optional.empty(), tenantPath, Instant.now()); } - DeployHandlerLogger getLogger(boolean verbose) { - return new DeployHandlerLogger(new Slime().get(), verbose, + private DeployHandlerLogger getLogger() { + return new DeployHandlerLogger(new Slime().get(), false, new ApplicationId.Builder().tenant("testtenant").applicationName("testapp").build()); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/MockSessionZKClient.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/MockSessionZKClient.java index e22185ae69b..b0e184398e5 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/MockSessionZKClient.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/MockSessionZKClient.java @@ -5,9 +5,6 @@ import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.TenantName; -import com.yahoo.transaction.Transaction; -import com.yahoo.path.Path; -import com.yahoo.vespa.config.server.session.Session.Status; import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; @@ -23,7 +20,6 @@ public class MockSessionZKClient extends SessionZooKeeperClient { private ApplicationPackage app; private Optional<AllocatedHosts> info = Optional.empty(); - private Status sessionStatus; public MockSessionZKClient(Curator curator, TenantName tenantName, long sessionId) { this(curator, tenantName, sessionId, (ApplicationPackage) null); @@ -34,7 +30,7 @@ public class MockSessionZKClient extends SessionZooKeeperClient { this.info = allocatedHosts; } - public MockSessionZKClient(Curator curator, TenantName tenantName, long sessionId, ApplicationPackage application) { + MockSessionZKClient(Curator curator, TenantName tenantName, long sessionId, ApplicationPackage application) { super(curator, TenantRepository.getSessionsPath(tenantName).append(String.valueOf(sessionId))); this.app = application; curator.create(TenantRepository.getSessionsPath(tenantName).append(String.valueOf(sessionId))); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionRepoTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionRepoTest.java index 83183a27666..9110671b494 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionRepoTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionRepoTest.java @@ -49,8 +49,8 @@ public class RemoteSessionRepoTest { this.remoteSessionRepo = tenant.getRemoteSessionRepo(); curator.create(TenantRepository.getTenantPath(tenantName).append("/applications")); curator.create(TenantRepository.getSessionsPath(tenantName)); - createSession(1l, false); - createSession(2l, false); + createSession(1L, false); + createSession(2L, false); } private void createSession(long sessionId, boolean wait) { @@ -69,14 +69,14 @@ public class RemoteSessionRepoTest { @Test public void testInitialize() { - assertSessionExists(1l); - assertSessionExists(2l); + assertSessionExists(1L); + assertSessionExists(2L); } @Test public void testCreateSession() { - createSession(3l, true); - assertSessionExists(3l); + createSession(3L, true); + assertSessionExists(3L); } @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java index c89c2f23873..99ef1831744 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java @@ -62,7 +62,7 @@ public class RemoteSessionTest { public void require_that_session_is_initialized() { Clock clock = Clock.systemUTC(); Session session = createSession(2, clock); - assertThat(session.getSessionId(), is(2l)); + assertThat(session.getSessionId(), is(2L)); session = createSession(Long.MAX_VALUE, clock); assertThat(session.getSessionId(), is(Long.MAX_VALUE)); } @@ -73,14 +73,14 @@ public class RemoteSessionTest { session.loadPrepared(); ApplicationSet applicationSet = session.ensureApplicationLoaded(); assertNotNull(applicationSet); - assertThat(applicationSet.getApplicationGeneration(), is(3l)); + assertThat(applicationSet.getApplicationGeneration(), is(3L)); assertThat(applicationSet.getForVersionOrLatest(Optional.empty(), Instant.now()).getId().application().value(), is("foo")); assertNotNull(applicationSet.getForVersionOrLatest(Optional.empty(), Instant.now()).getModel()); session.deactivate(); applicationSet = session.ensureApplicationLoaded(); assertNotNull(applicationSet); - assertThat(applicationSet.getApplicationGeneration(), is(3l)); + assertThat(applicationSet.getApplicationGeneration(), is(3L)); assertThat(applicationSet.getForVersionOrLatest(Optional.empty(), Instant.now()).getId().application().value(), is("foo")); assertNotNull(applicationSet.getForVersionOrLatest(Optional.empty(), Instant.now()).getModel()); } @@ -224,9 +224,7 @@ public class RemoteSessionTest { .curator(curator) .clock(clock) .modelFactoryRegistry(new ModelFactoryRegistry(modelFactories)); - if (permanentApplicationPackage.isPresent()) - registryBuilder.permanentApplicationPackage(permanentApplicationPackage.get()); - + permanentApplicationPackage.ifPresent(registryBuilder::permanentApplicationPackage); return new RemoteSession(tenantName, sessionId, registryBuilder.build(), zkc); } @@ -234,12 +232,12 @@ public class RemoteSessionTest { private class MockModelFactory implements ModelFactory { /** Throw a RuntimeException on load - this is handled gracefully during model building */ - public boolean throwOnLoad = false; + boolean throwOnLoad = false; /** Throw an Error on load - this is useful to propagate this condition all the way to the test */ - public boolean throwErrorOnLoad = false; + boolean throwErrorOnLoad = false; - public ModelContext modelContext; + ModelContext modelContext; public Version vespaVersion = new Version(1, 2, 3); /** The validation overrides of this, or null if none */ @@ -247,9 +245,9 @@ public class RemoteSessionTest { private Clock clock = Clock.fixed(LocalDate.parse("2000-01-01", DateTimeFormatter.ISO_DATE).atStartOfDay().atZone(ZoneOffset.UTC).toInstant(), ZoneOffset.UTC); - public MockModelFactory() { this(null); } + MockModelFactory() { this(null); } - public MockModelFactory(String validationOverrides) { + MockModelFactory(String validationOverrides) { this.validationOverrides = validationOverrides; } @@ -271,7 +269,7 @@ public class RemoteSessionTest { return loadModel(); } - public Model loadModel() { + Model loadModel() { try { ApplicationPackage application = new MockApplicationPackage.Builder().withEmptyHosts().withEmptyServices().withValidationOverrides(validationOverrides).build(); DeployState deployState = new DeployState.Builder().applicationPackage(application).now(clock.instant()).build(); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepoTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepoTest.java index 01cb90721f3..b8a3d0bc401 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepoTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepoTest.java @@ -12,7 +12,6 @@ import static org.junit.Assert.assertThat; /** * @author hmusum - * @since 5.1.14 */ public class SessionRepoTest { @Test @@ -20,7 +19,7 @@ public class SessionRepoTest { SessionRepo<TestSession> sessionRepo = new SessionRepo<>(); assertNull(sessionRepo.getSession(1L)); sessionRepo.addSession(new TestSession(1)); - assertThat(sessionRepo.getSession(1L).getSessionId(), is(1l)); + assertThat(sessionRepo.getSession(1L).getSessionId(), is(1L)); } @Test(expected = IllegalArgumentException.class) diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClientTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClientTest.java index 522a21a47b3..27e04bd7422 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClientTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClientTest.java @@ -92,18 +92,18 @@ public class SessionZooKeeperClientTest { public void require_that_create_time_can_be_written_and_read() { SessionZooKeeperClient zkc = createSessionZKClient("3"); curator.delete(Path.fromString("3")); - assertThat(zkc.readCreateTime(), is(0l)); - zkc.createNewSession(123456l, TimeUnit.SECONDS); - assertThat(zkc.readCreateTime(), is(123456l)); + assertThat(zkc.readCreateTime(), is(0L)); + zkc.createNewSession(123456L, TimeUnit.SECONDS); + assertThat(zkc.readCreateTime(), is(123456L)); } @Test public void require_that_create_time_has_correct_unit() { SessionZooKeeperClient zkc = createSessionZKClient("3"); curator.delete(Path.fromString("3")); - assertThat(zkc.readCreateTime(), is(0l)); + assertThat(zkc.readCreateTime(), is(0L)); zkc.createNewSession(60, TimeUnit.MINUTES); - assertThat(zkc.readCreateTime(), is(3600l)); + assertThat(zkc.readCreateTime(), is(3600L)); } private void assertApplicationIdParse(String sessionId, String idString, String expectedIdString) { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java index 36bb7a926b5..ff3d9449448 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java @@ -27,7 +27,6 @@ import com.yahoo.vespa.config.server.TestComponentRegistry; import com.yahoo.vespa.config.server.application.Application; import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.deploy.ZooKeeperDeployer; -import com.yahoo.vespa.config.server.host.HostRegistries; import com.yahoo.vespa.config.server.model.TestModelFactory; import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry; import com.yahoo.vespa.config.server.monitoring.MetricUpdater; @@ -135,24 +134,24 @@ public class TenantRequestHandlerTest { new TestModelFactory(new Version(3, 2, 1)))); } - public <T extends ConfigInstance> T resolve(Class<T> clazz, - TenantRequestHandler tenantRequestHandler, - ApplicationId appId, - Version vespaVersion, - String configId) { + private <T extends ConfigInstance> T resolve(Class<T> clazz, + TenantRequestHandler tenantRequestHandler, + ApplicationId appId, + Version vespaVersion, + String configId) { ConfigResponse response = getConfigResponse(clazz, tenantRequestHandler, appId, vespaVersion, configId); return ConfigPayload.fromUtf8Array(response.getPayload()).toInstance(clazz, configId); } - public <T extends ConfigInstance> ConfigResponse getConfigResponse(Class<T> clazz, - TenantRequestHandler tenantRequestHandler, - ApplicationId appId, - Version vespaVersion, - String configId) { + private <T extends ConfigInstance> ConfigResponse getConfigResponse(Class<T> clazz, + TenantRequestHandler tenantRequestHandler, + ApplicationId appId, + Version vespaVersion, + String configId) { return tenantRequestHandler.resolveConfig(appId, new GetConfigRequest() { @Override public ConfigKey<T> getConfigKey() { - return new ConfigKey<T>(clazz, configId); + return new ConfigKey<>(clazz, configId); } @Override @@ -183,7 +182,7 @@ public class TenantRequestHandlerTest { // Using only payload list for this simple test SimpletypesConfig config = resolve(SimpletypesConfig.class, server, defaultApp(), vespaVersion, ""); assertThat(config.intval(), is(1337)); - assertThat(server.getApplicationGeneration(applicationId, Optional.of(vespaVersion)), is(1l)); + assertThat(server.getApplicationGeneration(applicationId, Optional.of(vespaVersion)), is(1L)); server.reloadConfig(reloadConfig(1L)); ConfigResponse configResponse = getConfigResponse(SimpletypesConfig.class, server, defaultApp(), vespaVersion, ""); @@ -191,7 +190,7 @@ public class TenantRequestHandlerTest { config = resolve(SimpletypesConfig.class, server, defaultApp(), vespaVersion, ""); assertThat(config.intval(), is(1337)); assertThat(listener.reloaded.get(), is(2)); - assertThat(server.getApplicationGeneration(applicationId, Optional.of(vespaVersion)), is(1l)); + assertThat(server.getApplicationGeneration(applicationId, Optional.of(vespaVersion)), is(1L)); assertThat(listener.tenantHosts.size(), is(1)); assertThat(server.resolveApplicationId("mytesthost"), is(applicationId)); @@ -204,7 +203,7 @@ public class TenantRequestHandlerTest { config = resolve(SimpletypesConfig.class, server, defaultApp(), vespaVersion,""); assertThat(config.intval(), is(1330)); assertThat(listener.reloaded.get(), is(1)); - assertThat(server.getApplicationGeneration(applicationId, Optional.of(vespaVersion)), is(2l)); + assertThat(server.getApplicationGeneration(applicationId, Optional.of(vespaVersion)), is(2L)); } @Test @@ -284,7 +283,7 @@ public class TenantRequestHandlerTest { } public static class MockReloadListener implements ReloadListener { - public AtomicInteger reloaded = new AtomicInteger(0); + AtomicInteger reloaded = new AtomicInteger(0); AtomicInteger removed = new AtomicInteger(0); Map<String, Collection<String>> tenantHosts = new LinkedHashMap<>(); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantTest.java index e140dae3650..5b8f5299c01 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantTest.java @@ -6,7 +6,6 @@ import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.config.server.MockReloadHandler; import com.yahoo.vespa.config.server.TestComponentRegistry; import com.yahoo.vespa.config.server.application.TenantApplications; -import com.yahoo.vespa.curator.mock.MockCurator; import org.junit.Before; import org.junit.Test; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/InitializedCounterTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/InitializedCounterTest.java index 888cbb7a68b..f745e023126 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/InitializedCounterTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/InitializedCounterTest.java @@ -20,7 +20,7 @@ public class InitializedCounterTest { configCurator.createNode("/sessions/2"); InitializedCounter counter = new InitializedCounter(configCurator, "/counter", "/sessions"); - assertThat(counter.counter.get(), is(2l)); + assertThat(counter.counter.get(), is(2L)); } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java index 9f686570da1..0ad15e9cabe 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java @@ -1,15 +1,12 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.configserver; -import com.google.common.collect.ImmutableSortedSet; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RotationName; import java.util.Objects; import java.util.Optional; -import java.util.Set; /** * Represents an exclusive load balancer, assigned to an application's cluster. @@ -23,16 +20,14 @@ public class LoadBalancer { private final ClusterSpec.Id cluster; private final HostName hostname; private final Optional<String> dnsZone; - private final Set<RotationName> rotations; public LoadBalancer(String id, ApplicationId application, ClusterSpec.Id cluster, HostName hostname, - Optional<String> dnsZone, Set<RotationName> rotations) { + Optional<String> dnsZone) { this.id = Objects.requireNonNull(id, "id must be non-null"); this.application = Objects.requireNonNull(application, "application must be non-null"); this.cluster = Objects.requireNonNull(cluster, "cluster must be non-null"); this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null"); this.dnsZone = Objects.requireNonNull(dnsZone, "dnsZone must be non-null"); - this.rotations = ImmutableSortedSet.copyOf(Objects.requireNonNull(rotations, "rotations must be non-null")); } public String id() { @@ -55,8 +50,4 @@ public class LoadBalancer { return dnsZone; } - public Set<RotationName> rotations() { - return rotations; - } - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 7406795d0e3..65ef270ffd5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -470,7 +470,7 @@ public class ApplicationController { } finally { // Even if prepare fails, a load balancer may have been provisioned. Always refresh routing policies so that // any DNS updates can be propagated as early as possible. - routingPolicies.refresh(application, zone); + routingPolicies.refresh(application, applicationPackage.deploymentSpec(), zone); } } @@ -781,7 +781,7 @@ public class ApplicationController { } catch (NotFoundException ignored) { // ok; already gone } finally { - routingPolicies.refresh(application.get().id(), zone); + routingPolicies.refresh(application.get().id(), application.get().deploymentSpec(), zone); } return application.withoutDeploymentIn(zone); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java index d23ae913889..020203c6548 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java @@ -1,8 +1,8 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; +import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.RotationName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; @@ -72,14 +72,13 @@ public class RoutingPolicies { * Refresh routing policies for application in given zone. This is idempotent and changes will only be performed if * load balancers for given application have changed. */ - public void refresh(ApplicationId application, ZoneId zone) { - // TODO: Use this to decide how apply routing policies for shared routing layer + public void refresh(ApplicationId application, DeploymentSpec deploymentSpec, ZoneId zone) { if (!controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) return; var lbs = new LoadBalancers(application, zone, controller.applications().configServer() .getLoadBalancers(application, zone)); try (var lock = db.lockRoutingPolicies()) { - removeObsoleteEndpointsFromDns(lbs, lock); - storePoliciesOf(lbs, lock); + removeObsoleteEndpointsFromDns(lbs, deploymentSpec, lock); + storePoliciesOf(lbs, deploymentSpec, lock); removeObsoletePolicies(lbs, lock); registerEndpointsInDns(lbs, lock); } @@ -105,10 +104,10 @@ public class RoutingPolicies { } /** Store routing policies for given route */ - private void storePoliciesOf(LoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) { + private void storePoliciesOf(LoadBalancers loadBalancers, DeploymentSpec spec, @SuppressWarnings("unused") Lock lock) { Set<RoutingPolicy> policies = new LinkedHashSet<>(get(loadBalancers.application)); for (LoadBalancer loadBalancer : loadBalancers.list) { - RoutingPolicy policy = createPolicy(loadBalancers.application, loadBalancers.zone, loadBalancer); + RoutingPolicy policy = createPolicy(loadBalancers.application, spec, loadBalancers.zone, loadBalancer); if (!policies.add(policy)) { policies.remove(policy); policies.add(policy); @@ -118,17 +117,14 @@ public class RoutingPolicies { } /** Create a policy for given load balancer and register a CNAME for it */ - private RoutingPolicy createPolicy(ApplicationId application, ZoneId zone, LoadBalancer loadBalancer) { - // TODO(mpolden): Remove rotations from LoadBalancer. Use endpoints from deployment spec instead - Set<EndpointId> endpoints = loadBalancer.rotations().stream() - .map(RotationName::value) - .map(EndpointId::of) - .collect(Collectors.toSet()); - RoutingPolicy routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone, - loadBalancer.hostname(), loadBalancer.dnsZone(), - endpoints); - RecordName name = RecordName.from(routingPolicy.endpointIn(controller.system()).dnsName()); - RecordData data = RecordData.fqdn(loadBalancer.hostname().value()); + private RoutingPolicy createPolicy(ApplicationId application, DeploymentSpec deploymentSpec, ZoneId zone, + LoadBalancer loadBalancer) { + var endpoints = endpointIdsOf(loadBalancer, zone, deploymentSpec); + var routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone, + loadBalancer.hostname(), loadBalancer.dnsZone(), + endpoints); + var name = RecordName.from(routingPolicy.endpointIn(controller.system()).dnsName()); + var data = RecordData.fqdn(loadBalancer.hostname().value()); controller.nameServiceForwarder().createCname(name, data, Priority.normal); return routingPolicy; } @@ -152,10 +148,10 @@ public class RoutingPolicies { } /** Remove unreferenced global endpoints for given route from DNS */ - private void removeObsoleteEndpointsFromDns(LoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) { + private void removeObsoleteEndpointsFromDns(LoadBalancers loadBalancers, DeploymentSpec deploymentSpec, @SuppressWarnings("unused") Lock lock) { var zonePolicies = get(loadBalancers.application, loadBalancers.zone); var removalCandidates = routingTableFrom(zonePolicies).keySet(); - var activeRoutingIds = routingIdsFrom(loadBalancers.list); + var activeRoutingIds = routingIdsFrom(loadBalancers, deploymentSpec); removalCandidates.removeAll(activeRoutingIds); for (var id : removalCandidates) { Endpoint endpoint = RoutingPolicy.endpointOf(id.application(), id.endpointId(), controller.system()); @@ -164,11 +160,11 @@ public class RoutingPolicies { } /** Compute routing IDs from given load balancers */ - private static Set<RoutingId> routingIdsFrom(List<LoadBalancer> loadBalancers) { + private static Set<RoutingId> routingIdsFrom(LoadBalancers loadBalancers, DeploymentSpec spec) { Set<RoutingId> routingIds = new LinkedHashSet<>(); - for (var loadBalancer : loadBalancers) { - for (var rotation : loadBalancer.rotations()) { - routingIds.add(new RoutingId(loadBalancer.application(), EndpointId.of(rotation.value()))); + for (var loadBalancer : loadBalancers.list) { + for (var endpointId : endpointIdsOf(loadBalancer, loadBalancers.zone, spec)) { + routingIds.add(new RoutingId(loadBalancer.application(), endpointId)); } } return Collections.unmodifiableSet(routingIds); @@ -187,6 +183,16 @@ public class RoutingPolicies { return routingTable; } + /** Compute all endpoint IDs of given load balancer */ + private static Set<EndpointId> endpointIdsOf(LoadBalancer loadBalancer, ZoneId zone, DeploymentSpec spec) { + return spec.endpoints().stream() + .filter(endpoint -> endpoint.containerId().equals(loadBalancer.cluster().value())) + .filter(endpoint -> endpoint.regions().contains(zone.region())) + .map(com.yahoo.config.application.api.Endpoint::endpointId) + .map(EndpointId::of) + .collect(Collectors.toSet()); + } + /** Load balancers for a particular deployment */ private static class LoadBalancers { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java index 600fca4f45e..e6387ee3e0c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java @@ -3,19 +3,18 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RotationName; -import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; +import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; +import com.yahoo.vespa.hosted.controller.deployment.BuildJob; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import org.junit.Test; @@ -26,7 +25,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.function.Supplier; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; @@ -54,59 +52,88 @@ public class RoutingPoliciesTest { @Test public void maintains_global_routing_policies() { - int buildNumber = 42; + long buildNumber = BuildJob.defaultBuildNumber; int clustersPerZone = 2; - // Cluster 0 is member of 2 global rotations - Map<Integer, Set<RotationName>> rotations = Map.of(0, Set.of(RotationName.from("r0"), RotationName.from("r1"))); - provisionLoadBalancers(clustersPerZone, rotations, app1.id(), zone1, zone2); + int numberOfDeployments = 2; + var applicationPackage = new ApplicationPackageBuilder() + .region(zone1.region()) + .region(zone2.region()) + .endpoint("r0", "c0") + .endpoint("r1", "c0", "us-west-1") + .endpoint("r2", "c1") + .build(); + provisionLoadBalancers(clustersPerZone, app1.id(), zone1, zone2); - // Creates alias records for cluster0 + // Creates alias records tester.deployCompletely(app1, applicationPackage, ++buildNumber); - Supplier<List<Record>> records1 = () -> tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from("r0.app1.tenant1.global.vespa.oath.cloud")); - Supplier<List<Record>> records2 = () -> tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from("r1.app1.tenant1.global.vespa.oath.cloud")); - assertEquals(2, records1.get().size()); - assertEquals(records1.get().size(), records2.get().size()); - assertEquals("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1", records1.get().get(0).data().asString()); - assertEquals("lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1", records1.get().get(1).data().asString()); - assertEquals("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1", records2.get().get(0).data().asString()); - assertEquals("lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1", records2.get().get(1).data().asString()); - assertEquals(2, tester.controller().applications().routingPolicies().get(app1.id()).iterator().next() - .rotationEndpointsIn(SystemName.main).asList().size()); + var endpoint1 = "r0.app1.tenant1.global.vespa.oath.cloud"; + var endpoint2 = "r1.app1.tenant1.global.vespa.oath.cloud"; + var endpoint3 = "r2.app1.tenant1.global.vespa.oath.cloud"; + + assertEquals(endpoint1 + " points to c0 in all regions", + List.of("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1", + "lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1"), + aliasDataOf(endpoint1)); + assertEquals(endpoint2 + " points to c0 us-west-1", + List.of("lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1"), + aliasDataOf(endpoint2)); + assertEquals(endpoint3 + " points to c1 in all regions", + List.of("lb-1--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1", + "lb-1--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1"), + aliasDataOf(endpoint3)); + assertEquals("Routing policy count is equal to cluster count", + numberOfDeployments * clustersPerZone, + tester.controller().applications().routingPolicies().get(app1.id()).size()); // Applications gains a new deployment - ApplicationPackage updatedApplicationPackage = new ApplicationPackageBuilder() - .environment(Environment.prod) + ApplicationPackage applicationPackage2 = new ApplicationPackageBuilder() + .region(zone1.region()) + .region(zone2.region()) + .region(zone3.region()) + .endpoint("r0", "c0") + .endpoint("r1", "c0", "us-west-1") + .endpoint("r2", "c1") + .build(); + numberOfDeployments++; + provisionLoadBalancers(clustersPerZone, app1.id(), zone3); + tester.deployCompletely(app1, applicationPackage2, ++buildNumber); + + // Endpoint is updated to contain cluster in new deployment + assertEquals(endpoint1 + " points to c0 in all regions", + List.of("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1", + "lb-0--tenant1:app1:default--prod.us-east-3/dns-zone-1/prod.us-east-3", + "lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1"), + aliasDataOf(endpoint1)); + + // Another application is deployed with a single cluster and global endpoint + var endpoint4 = "r0.app2.tenant1.global.vespa.oath.cloud"; + provisionLoadBalancers(1, app2.id(), zone1, zone2); + var applicationPackage3 = new ApplicationPackageBuilder() + .region(zone1.region()) + .region(zone2.region()) + .endpoint("r0", "c0") + .build(); + tester.deployCompletely(app2, applicationPackage3); + assertEquals(endpoint4 + " points to c0 in all regions", + List.of("lb-0--tenant1:app2:default--prod.us-central-1/dns-zone-1/prod.us-central-1", + "lb-0--tenant1:app2:default--prod.us-west-1/dns-zone-1/prod.us-west-1"), + aliasDataOf(endpoint4)); + + // All endpoints for app1 are removed + ApplicationPackage applicationPackage4 = new ApplicationPackageBuilder() .region(zone1.region()) .region(zone2.region()) .region(zone3.region()) .build(); - int numberOfDeployments = 3; - provisionLoadBalancers(clustersPerZone, rotations, app1.id(), zone3); - tester.deployCompletely(app1, updatedApplicationPackage, ++buildNumber); - - // Cluster in new deployment is added to the rotation - assertEquals(numberOfDeployments, records1.get().size()); - assertEquals("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1", records1.get().get(0).data().asString()); - assertEquals("lb-0--tenant1:app1:default--prod.us-east-3/dns-zone-1/prod.us-east-3", records1.get().get(1).data().asString()); - assertEquals("lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1", records1.get().get(2).data().asString()); - - // Another application is deployed - Supplier<List<Record>> records3 = () -> tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from("r0.app2.tenant1.global.vespa.oath.cloud")); - provisionLoadBalancers(1, Map.of(0, Set.of(RotationName.from("r0"))), app2.id(), zone1, zone2); - tester.deployCompletely(app2, applicationPackage); - assertEquals(2, records3.get().size()); - assertEquals("lb-0--tenant1:app2:default--prod.us-central-1/dns-zone-1/prod.us-central-1", records3.get().get(0).data().asString()); - assertEquals("lb-0--tenant1:app2:default--prod.us-west-1/dns-zone-1/prod.us-west-1", records3.get().get(1).data().asString()); - - // All rotations for app1 are removed - provisionLoadBalancers(clustersPerZone, Map.of(), app1.id(), zone1, zone2, zone3); - tester.deployCompletely(app1, updatedApplicationPackage, ++buildNumber); - assertEquals(List.of(), records1.get()); + tester.deployCompletely(app1, applicationPackage4, ++buildNumber); + assertEquals("DNS records are removed", List.of(), aliasDataOf(endpoint1)); + assertEquals("DNS records are removed", List.of(), aliasDataOf(endpoint2)); + assertEquals("DNS records are removed", List.of(), aliasDataOf(endpoint3)); Set<RoutingPolicy> policies = tester.controller().curator().readRoutingPolicies(app1.id()); assertEquals(clustersPerZone * numberOfDeployments, policies.size()); assertTrue("Rotation membership is removed from all policies", policies.stream().allMatch(policy -> policy.endpoints().isEmpty())); - assertEquals("Rotations for " + app2 + " are not removed", 2, records3.get().size()); + assertEquals("Rotations for " + app2 + " are not removed", 2, aliasDataOf(endpoint4).size()); } @Test @@ -222,30 +249,30 @@ public class RoutingPoliciesTest { .collect(Collectors.toSet()); } - private void provisionLoadBalancers(int clustersPerZone, Map<Integer, Set<RotationName>> clusterRotations, ApplicationId application, ZoneId... zones) { - for (ZoneId zone : zones) { - tester.configServer().removeLoadBalancers(application, zone); - tester.configServer().addLoadBalancers(zone, createLoadBalancers(zone, application, clustersPerZone, clusterRotations)); - } + private List<String> aliasDataOf(String name) { + return tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from(name)).stream() + .map(Record::data) + .map(RecordData::asString) + .collect(Collectors.toList()); } private void provisionLoadBalancers(int clustersPerZone, ApplicationId application, ZoneId... zones) { - provisionLoadBalancers(clustersPerZone, Map.of(), application, zones); + for (ZoneId zone : zones) { + tester.configServer().removeLoadBalancers(application, zone); + tester.configServer().addLoadBalancers(zone, createLoadBalancers(zone, application, clustersPerZone)); + } } - private static List<LoadBalancer> createLoadBalancers(ZoneId zone, ApplicationId application, int count, - Map<Integer, Set<RotationName>> clusterRotations) { + private static List<LoadBalancer> createLoadBalancers(ZoneId zone, ApplicationId application, int count) { List<LoadBalancer> loadBalancers = new ArrayList<>(); for (int i = 0; i < count; i++) { - Set<RotationName> rotations = clusterRotations.getOrDefault(i, Collections.emptySet()); loadBalancers.add( new LoadBalancer("LB-" + i + "-Z-" + zone.value(), application, ClusterSpec.Id.from("c" + i), HostName.from("lb-" + i + "--" + application.serializedForm() + "--" + zone.value()), - Optional.of("dns-zone-1"), - rotations)); + Optional.of("dns-zone-1"))); } return loadBalancers; } diff --git a/http-utils/src/main/java/ai/vespa/util/http/VespaHttpClientBuilder.java b/http-utils/src/main/java/ai/vespa/util/http/VespaHttpClientBuilder.java index 5e7a9441fc8..5ac6786ae30 100644 --- a/http-utils/src/main/java/ai/vespa/util/http/VespaHttpClientBuilder.java +++ b/http-utils/src/main/java/ai/vespa/util/http/VespaHttpClientBuilder.java @@ -4,24 +4,28 @@ package ai.vespa.util.http; import com.yahoo.security.tls.MixedMode; import com.yahoo.security.tls.TlsContext; import com.yahoo.security.tls.TransportSecurityUtils; +import org.apache.http.HttpException; +import org.apache.http.HttpHost; import org.apache.http.HttpRequest; -import org.apache.http.HttpRequestInterceptor; -import org.apache.http.client.methods.HttpRequestBase; -import org.apache.http.client.utils.URIBuilder; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.config.Registry; import org.apache.http.config.RegistryBuilder; import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.conn.UnsupportedSchemeException; +import org.apache.http.conn.routing.HttpRoute; +import org.apache.http.conn.routing.HttpRoutePlanner; import org.apache.http.conn.socket.ConnectionSocketFactory; import org.apache.http.conn.socket.PlainConnectionSocketFactory; import org.apache.http.conn.ssl.NoopHostnameVerifier; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.conn.BasicHttpClientConnectionManager; +import org.apache.http.impl.conn.DefaultSchemePortResolver; import org.apache.http.protocol.HttpContext; import javax.net.ssl.SSLParameters; -import java.net.URI; -import java.net.URISyntaxException; +import java.net.InetAddress; import java.util.logging.Level; import java.util.logging.Logger; @@ -69,7 +73,7 @@ public class VespaHttpClientBuilder { private static HttpClientBuilder createBuilder(ConnectionManagerFactory connectionManagerFactory) { var builder = HttpClientBuilder.create(); addSslSocketFactory(builder, connectionManagerFactory); - addTlsAwareRequestInterceptor(builder); + addHttpsRewritingRoutePlanner(builder); return builder; } @@ -86,11 +90,10 @@ public class VespaHttpClientBuilder { }); } - private static void addTlsAwareRequestInterceptor(HttpClientBuilder builder) { + private static void addHttpsRewritingRoutePlanner(HttpClientBuilder builder) { if (TransportSecurityUtils.isTransportSecurityEnabled() && TransportSecurityUtils.getInsecureMixedMode() != MixedMode.PLAINTEXT_CLIENT_MIXED_SERVER) { - log.log(Level.FINE, "Adding request interceptor to client"); - builder.addInterceptorFirst(new HttpToHttpsRewritingRequestInterceptor()); + builder.setRoutePlanner(new HttpToHttpsRoutePlanner()); } } @@ -106,29 +109,32 @@ public class VespaHttpClientBuilder { .build(); } - static class HttpToHttpsRewritingRequestInterceptor implements HttpRequestInterceptor { + + /** + * Reroutes requests using 'http' to 'https'. + * Implementation inspired by {@link org.apache.http.impl.conn.DefaultRoutePlanner}, but without proxy support. + */ + static class HttpToHttpsRoutePlanner implements HttpRoutePlanner { + @Override - public void process(HttpRequest request, HttpContext context) { - if (request instanceof HttpRequestBase) { - HttpRequestBase httpUriRequest = (HttpRequestBase) request; - httpUriRequest.setURI(rewriteUri(httpUriRequest.getURI())); - } else { - log.log(Level.FINE, () -> "Not a HttpRequestBase - skipping URI rewriting: " + request.getClass().getName()); - } + public HttpRoute determineRoute(HttpHost host, HttpRequest request, HttpContext context) throws HttpException { + HttpClientContext clientContext = HttpClientContext.adapt(context); + RequestConfig config = clientContext.getRequestConfig(); + InetAddress local = config.getLocalAddress(); + + HttpHost target = resolveTarget(host); + boolean secure = target.getSchemeName().equalsIgnoreCase("https"); + return new HttpRoute(target, local, secure); } - private static URI rewriteUri(URI originalUri) { - if (!originalUri.getScheme().equals("http")) { - return originalUri; - } - int port = originalUri.getPort(); - int rewrittenPort = port != -1 ? port : 80; + private HttpHost resolveTarget(HttpHost host) throws HttpException { try { - URI rewrittenUri = new URIBuilder(originalUri).setScheme("https").setPort(rewrittenPort).build(); - log.log(Level.FINE, () -> String.format("Uri rewritten from '%s' to '%s'", originalUri, rewrittenUri)); - return rewrittenUri; - } catch (URISyntaxException e) { - throw new RuntimeException(e); + String originalScheme = host.getSchemeName(); + String scheme = originalScheme.equalsIgnoreCase("http") ? "https" : originalScheme; + int port = DefaultSchemePortResolver.INSTANCE.resolve(host); + return new HttpHost(host.getHostName(), port, scheme); + } catch (UnsupportedSchemeException e) { + throw new HttpException(e.getMessage(), e); } } } diff --git a/http-utils/src/test/java/ai/vespa/util/http/VespaHttpClientBuilderTest.java b/http-utils/src/test/java/ai/vespa/util/http/VespaHttpClientBuilderTest.java index 7ffd0e459b0..85ee0913c58 100644 --- a/http-utils/src/test/java/ai/vespa/util/http/VespaHttpClientBuilderTest.java +++ b/http-utils/src/test/java/ai/vespa/util/http/VespaHttpClientBuilderTest.java @@ -1,14 +1,15 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.util.http; -import ai.vespa.util.http.VespaHttpClientBuilder.HttpToHttpsRewritingRequestInterceptor; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.protocol.BasicHttpContext; +import org.apache.http.HttpException; +import org.apache.http.HttpHost; +import org.apache.http.HttpRequest; +import org.apache.http.client.protocol.HttpClientContext; +import org.apache.http.conn.routing.HttpRoute; import org.junit.Test; -import java.net.URI; - -import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; /** * @author bjorncs @@ -16,24 +17,25 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThat; public class VespaHttpClientBuilderTest { @Test - public void request_interceptor_modifies_scheme_of_requests() { - verifyProcessedUriMatchesExpectedOutput("http://dummyhostname:8080/a/path/to/resource?query=value", - "https://dummyhostname:8080/a/path/to/resource?query=value"); + public void route_planner_modifies_scheme_of_requests() throws HttpException { + verifyProcessedUriMatchesExpectedOutput("http://dummyhostname:8080", "https://dummyhostname:8080"); + } + + @Test + public void route_planer_handles_implicit_http_port() throws HttpException { + verifyProcessedUriMatchesExpectedOutput("http://dummyhostname", "https://dummyhostname:80"); } @Test - public void request_interceptor_add_handles_implicit_http_port() { - verifyProcessedUriMatchesExpectedOutput("http://dummyhostname/a/path/to/resource?query=value", - "https://dummyhostname:80/a/path/to/resource?query=value"); + public void route_planer_handles_https_port() throws HttpException { + verifyProcessedUriMatchesExpectedOutput("http://dummyhostname:443", "https://dummyhostname:443"); } - private static void verifyProcessedUriMatchesExpectedOutput(String inputUri, String expectedOutputUri) { - var interceptor = new HttpToHttpsRewritingRequestInterceptor(); - HttpGet request = new HttpGet(inputUri); - interceptor.process(request, new BasicHttpContext()); - URI modifiedUri = request.getURI(); - URI expectedUri = URI.create(expectedOutputUri); - assertThat(modifiedUri).isEqualTo(expectedUri); + private static void verifyProcessedUriMatchesExpectedOutput(String inputHostString, String expectedHostString) throws HttpException { + var routePlanner = new VespaHttpClientBuilder.HttpToHttpsRoutePlanner(); + HttpRoute newRoute = routePlanner.determineRoute(HttpHost.create(inputHostString), mock(HttpRequest.class), new HttpClientContext()); + HttpHost target = newRoute.getTargetHost(); + assertEquals(expectedHostString, target.toURI()); } }
\ No newline at end of file diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/HostCapacityResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/HostCapacityResponse.java index 9f5af52cc08..7b0eb38b628 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/HostCapacityResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/HostCapacityResponse.java @@ -42,14 +42,7 @@ public class HostCapacityResponse extends HttpResponse { } private List<Node> parseHostList(String hosts) { - ObjectMapper om = new ObjectMapper(); - String[] hostsArray; - try { - hostsArray = om.readValue(hosts, String[].class); - } catch (Exception e) { - throw new IllegalArgumentException(e.getMessage()); - } - List<String> hostNames = Arrays.asList(hostsArray); + List<String> hostNames = Arrays.asList(hosts.split(",")); try { return capacityChecker.nodesFromHostnames(hostNames); } catch (IllegalArgumentException e) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java index bfbf7775031..9f8f4a804d1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java @@ -77,10 +77,6 @@ public class LoadBalancersResponse extends HttpResponse { realObject.setString("ipAddress", real.ipAddress()); realObject.setLong("port", real.port()); }); - - // TODO(mpolden): The following fields preserves API compatibility. These can be removed once clients stop expecting them - lbObject.setArray("rotations"); - lbObject.setBool("inactive", lb.state() == LoadBalancer.State.inactive); }); new JsonFormat(true).encode(stream, slime); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java index e036124e489..b2f0998189d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java @@ -102,7 +102,7 @@ public class NodesApiHandler extends LoggingRequestHandler { if (path.equals( "/nodes/v2/command/")) return ResourcesResponse.fromStrings(request.getUri(), "restart", "reboot"); if (path.equals( "/nodes/v2/maintenance/")) return new JobsResponse(nodeRepository.jobControl()); if (path.equals( "/nodes/v2/upgrade/")) return new UpgradeResponse(nodeRepository.infrastructureVersions(), nodeRepository.osVersions(), nodeRepository.dockerImages()); - if (path.startsWith("/nodes/v2/capacity/")) return new HostCapacityResponse(nodeRepository, request); + if (path.startsWith("/nodes/v2/capacity")) return new HostCapacityResponse(nodeRepository, request); throw new NotFoundException("Nothing at path '" + path + "'"); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java index 35fa5adaeff..8452e8f93bb 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java @@ -823,17 +823,15 @@ public class RestApiTest { @Test public void test_capacity() throws Exception { assertFile(new Request("http://localhost:8080/nodes/v2/capacity/?json=true"), "capacity-zone.json"); + assertFile(new Request("http://localhost:8080/nodes/v2/capacity?json=true"), "capacity-zone.json"); List<String> hostsToRemove = List.of( - "%22dockerhost1.yahoo.com%22", - "%22dockerhost2.yahoo.com%22", - "%22dockerhost3.yahoo.com%22", - "%22dockerhost4.yahoo.com%22" + "dockerhost1.yahoo.com", + "dockerhost2.yahoo.com", + "dockerhost3.yahoo.com", + "dockerhost4.yahoo.com" ); - String requestUriTemplate = - "http://localhost:8080/nodes/v2/capacity/?json=true&hosts=[%s]" - .replaceAll("\\[", "%%5B") - .replaceAll("]", "%%5D"); + String requestUriTemplate = "http://localhost:8080/nodes/v2/capacity/?json=true&hosts=%s"; assertFile(new Request(String.format(requestUriTemplate, String.join(",", hostsToRemove.subList(0, 3)))), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers-single.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers-single.json index 67d2c3bfa4b..19e65c2fc25 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers-single.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers-single.json @@ -28,9 +28,7 @@ "ipAddress": "127.0.14.1", "port": 4080 } - ], - "rotations": [], - "inactive": false + ] } ] } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json index c9a45a9c3da..0b05a41af0a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json @@ -28,9 +28,7 @@ "ipAddress": "127.0.10.1", "port": 4080 } - ], - "rotations": [], - "inactive": false + ] }, { "id": "tenant4:application4:instance4:id4", @@ -60,9 +58,7 @@ "ipAddress": "127.0.14.1", "port": 4080 } - ], - "rotations": [], - "inactive": false + ] } ] } diff --git a/vespa-http-client/pom.xml b/vespa-http-client/pom.xml index 18dd34ee7be..96aa6defbe6 100644 --- a/vespa-http-client/pom.xml +++ b/vespa-http-client/pom.xml @@ -65,6 +65,22 @@ <artifactId>airline</artifactId> <version>0.6</version> </dependency> + <dependency> + <!-- Needed for Vespa TLS configuration. Standard jar artifact --> + <groupId>com.yahoo.vespa</groupId> + <artifactId>http-utils</artifactId> + <version>${project.version}</version> + <scope>compile</scope> + </dependency> + <dependency> + <!-- Needed for Vespa TLS configuration. --> + <!-- This artifact is packaged as an OSGi bundle - make sure to manually include or exclude transitive dependencies as necessary --> + <!-- Note: includes BouncyCastle to compile scope transitively. --> + <groupId>com.yahoo.vespa</groupId> + <artifactId>security-utils</artifactId> + <version>${project.version}</version> + <scope>compile</scope> + </dependency> <!-- Test dependencies --> <dependency> @@ -140,6 +156,17 @@ </goals> <configuration> <outputFile>target/${project.artifactId}-jar-with-dependencies.jar</outputFile> + <filters> + <filter> + <!-- Don't include signature files in uber jar (most likely from bouncycastle). --> + <artifact>*:*</artifact> + <excludes> + <exclude>META-INF/*.SF</exclude> + <exclude>META-INF/*.DSA</exclude> + <exclude>META-INF/*.RSA</exclude> + </excludes> + </filter> + </filters> <transformers> <transformer implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer"> <mainClass>com.yahoo.vespa.http.client.runner.Runner</mainClass> diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java index f503190864b..adf61b124ab 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java @@ -43,6 +43,16 @@ public final class ConnectionParams { private int traceLevel = 0; private int traceEveryXOperation = 0; private boolean printTraceToStdErr = true; + private boolean useTlsConfigFromEnvironment = false; + + /** + * Use TLS configuration through the standard Vespa environment variables. + * Setting this to 'true' will override any other TLS/HTTPS related configuration. + */ + public Builder setUseTlsConfigFromEnvironment(boolean useTlsConfigFromEnvironment) { + this.useTlsConfigFromEnvironment = useTlsConfigFromEnvironment; + return this; + } /** * Sets the SSLContext for the connection to the gateway when SSL is enabled for Endpoint. @@ -233,7 +243,8 @@ public final class ConnectionParams { dryRun, traceLevel, traceEveryXOperation, - printTraceToStdErr); + printTraceToStdErr, + useTlsConfigFromEnvironment); } public int getNumPersistentConnectionsPerEndpoint() { @@ -273,6 +284,10 @@ public final class ConnectionParams { public HostnameVerifier getHostnameVerifier() { return hostnameVerifier; } + + public boolean useTlsConfigFromEnvironment() { + return useTlsConfigFromEnvironment; + } } private final SSLContext sslContext; private final HostnameVerifier hostnameVerifier; @@ -288,6 +303,7 @@ public final class ConnectionParams { private final int traceLevel; private final int traceEveryXOperation; private final boolean printTraceToStdErr; + private final boolean useTlsConfigFromEnvironment; private ConnectionParams( SSLContext sslContext, @@ -303,9 +319,11 @@ public final class ConnectionParams { boolean dryRun, int traceLevel, int traceEveryXOperation, - boolean printTraceToStdErr) { + boolean printTraceToStdErr, + boolean useTlsConfigFromEnvironment) { this.sslContext = sslContext; this.hostnameVerifier = hostnameVerifier; + this.useTlsConfigFromEnvironment = useTlsConfigFromEnvironment; this.headers.putAll(headers); this.headerProviders.putAll(headerProviders); this.numPersistentConnectionsPerEndpoint = numPersistentConnectionsPerEndpoint; @@ -378,6 +396,10 @@ public final class ConnectionParams { return printTraceToStdErr; } + public boolean useTlsConfigFromEnvironment() { + return useTlsConfigFromEnvironment; + } + /** * A header provider that provides a header value. {@link #getHeaderValue()} is called each time a new HTTP request * is constructed by {@link com.yahoo.vespa.http.client.FeedClient}. diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java index 5289a7a562a..0b92427bcbe 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java @@ -17,14 +17,8 @@ import org.apache.http.StatusLine; import org.apache.http.client.HttpClient; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.HttpPost; -import org.apache.http.config.Registry; -import org.apache.http.config.RegistryBuilder; -import org.apache.http.conn.socket.ConnectionSocketFactory; -import org.apache.http.conn.socket.PlainConnectionSocketFactory; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.entity.InputStreamEntity; import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.message.BasicHeader; import java.io.ByteArrayInputStream; @@ -400,25 +394,24 @@ class ApacheGatewayConnection implements GatewayConnection { } public HttpClient createClient() { - HttpClientBuilder clientBuilder = HttpClientBuilder.create(); - if (useSsl && connectionParams.getSslContext() != null) { - Registry<ConnectionSocketFactory> socketFactoryRegistry = RegistryBuilder.<ConnectionSocketFactory>create() - .register("https", new SSLConnectionSocketFactory( - connectionParams.getSslContext(), connectionParams.getHostnameVerifier())) - .register("http", PlainConnectionSocketFactory.INSTANCE) - .build(); - PoolingHttpClientConnectionManager connMgr = new PoolingHttpClientConnectionManager(socketFactoryRegistry); - clientBuilder.setConnectionManager(connMgr); - + HttpClientBuilder clientBuilder; + if (connectionParams.useTlsConfigFromEnvironment()) { + clientBuilder = VespaTlsAwareClientBuilder.createHttpClientBuilder(); + } else { + clientBuilder = HttpClientBuilder.create(); + if (useSsl && connectionParams.getSslContext() != null) { + clientBuilder.setSslcontext(connectionParams.getSslContext()); + clientBuilder.setSSLHostnameVerifier(connectionParams.getHostnameVerifier()); + } } - clientBuilder.setUserAgent(String.format("vespa-http-client (%s)", Vtag.currentVersion)); - clientBuilder.setDefaultHeaders(Collections.singletonList(new BasicHeader(Headers.CLIENT_VERSION, Vtag.currentVersion))); clientBuilder.setMaxConnPerRoute(1); clientBuilder.setMaxConnTotal(1); + clientBuilder.setConnectionTimeToLive(15, TimeUnit.SECONDS); + clientBuilder.setUserAgent(String.format("vespa-http-client (%s)", Vtag.currentVersion)); + clientBuilder.setDefaultHeaders(Collections.singletonList(new BasicHeader(Headers.CLIENT_VERSION, Vtag.currentVersion))); clientBuilder.disableContentCompression(); // Try to disable the disabling to see if system tests become stable again. // clientBuilder.disableAutomaticRetries(); - clientBuilder.setConnectionTimeToLive(15, TimeUnit.SECONDS); { RequestConfig.Builder requestConfigBuilder = RequestConfig.custom(); requestConfigBuilder.setSocketTimeout(0); diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/VespaTlsAwareClientBuilder.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/VespaTlsAwareClientBuilder.java new file mode 100644 index 00000000000..be67e11963e --- /dev/null +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/VespaTlsAwareClientBuilder.java @@ -0,0 +1,26 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.http.client.core.communication; + +import org.apache.http.impl.client.HttpClientBuilder; + +/** + * A static factory for VespaHttpClientBuilder. + * The main purpose of this class is to avoid references to classes not compiled with JDK8. + * + * @author bjorncs + */ +// TODO Remove use of reflection once vespa-http-client only targets JDK11 +// The VespaTlsAwareClientBuilder class refers to classes in security-utils / http-utils that targets JDK11+. +class VespaTlsAwareClientBuilder { + + private VespaTlsAwareClientBuilder() {} + + static HttpClientBuilder createHttpClientBuilder() { + try { + Class<?> builderClass = Class.forName("ai.vespa.util.http.VespaHttpClientBuilder"); + return (HttpClientBuilder) builderClass.getMethod("create").invoke(null); + } catch (ReflectiveOperationException e) { + throw new RuntimeException(e); + } + } +} diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/Vtag.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/Vtag.java index f21a86dfc1b..f44a30208e7 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/Vtag.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/Vtag.java @@ -3,5 +3,5 @@ package com.yahoo.vespa.http.client.core.communication; class Vtag { - static final String currentVersion = "7.0.0"; + static final String currentVersion = "7.1.0"; } diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/CommandLineArguments.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/CommandLineArguments.java index 4e2c8f1509e..ea0b3f29509 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/CommandLineArguments.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/CommandLineArguments.java @@ -209,6 +209,10 @@ public class CommandLineArguments { description = "Add http header to every request. Header must have the format '<Name>: <Value>'. Use this parameter multiple times for multiple headers") private List<String> headers = new ArrayList<>(); + @Option(name = {"--vespaTls"}, + description = "BETA! Use Vespa TLS configuration from environment if available. Other HTTPS/TLS configuration will be ignored if this is set.") + private boolean useTlsConfigFromEnvironment = false; + private final List<Header> parsedHeaders = new ArrayList<>(); int getWhenVerboseEnabledPrintMessageForEveryXDocuments() { @@ -252,6 +256,7 @@ public class CommandLineArguments { .setTraceEveryXOperation(traceEveryXOperation) .setPrintTraceToStdErr(traceArg > 0) .setNumPersistentConnectionsPerEndpoint(numPersistentConnectionsPerEndpoint) + .setUseTlsConfigFromEnvironment(useTlsConfigFromEnvironment) .build() ) // Enable dynamic throttling. diff --git a/vespaclient-container-plugin/pom.xml b/vespaclient-container-plugin/pom.xml index 959fb687692..53e708601d7 100644 --- a/vespaclient-container-plugin/pom.xml +++ b/vespaclient-container-plugin/pom.xml @@ -55,6 +55,10 @@ <groupId>org.apache.httpcomponents</groupId> <artifactId>httpclient</artifactId> </exclusion> + <exclusion> + <groupId>org.apache.httpcomponents</groupId> + <artifactId>httpcore</artifactId> + </exclusion> </exclusions> </dependency> <dependency> |