Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code cleanup #559

Merged
merged 1 commit into from
Jan 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions cli/src/test/java/org/jboss/pnc/build/finder/cli/MainTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package org.jboss.pnc.build.finder.cli;

import static org.assertj.core.api.Assertions.assertThat;
import static org.jboss.pnc.build.finder.core.ChecksumType.md5;
import static org.jboss.pnc.build.finder.core.ChecksumType.sha256;

import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -139,7 +141,7 @@ void testParsing(@TempDir File folder) throws IOException {

Collection<ChecksumType> checksumType = parseResult.matchedOption("--checksum-type").getValue();

assertThat(checksumType).containsExactly(ChecksumType.sha256);
assertThat(checksumType).containsExactly(sha256);

List<Pattern> excludes = parseResult.matchedOption("--exclude").getValue();

Expand Down Expand Up @@ -193,7 +195,7 @@ void testConfig(@TempDir File folder, StdOut out) throws IOException {

Collection<ChecksumType> checksumType = parseResult.matchedOption("--checksum-type").getValue();

assertThat(checksumType).containsExactly(ChecksumType.sha256);
assertThat(checksumType).containsExactly(sha256);

Main.main(args);

Expand Down Expand Up @@ -242,7 +244,7 @@ void testDebug(@TempDir File folder, StdOut out) throws IOException {

Collection<ChecksumType> checksumType = parseResult.matchedOption("--checksum-type").getValue();

assertThat(checksumType).containsExactly(ChecksumType.md5);
assertThat(checksumType).containsExactly(md5);

assertThat((Boolean) parseResult.matchedOption("--debug").getValue()).isTrue();

Expand Down Expand Up @@ -298,7 +300,7 @@ void testQuiet(@TempDir File folder, StdOut out) throws IOException {

Collection<ChecksumType> checksumType = parseResult.matchedOption("--checksum-type").getValue();

assertThat(checksumType).containsExactly(ChecksumType.md5);
assertThat(checksumType).containsExactly(md5);

assertThat((Boolean) parseResult.matchedOption("--quiet").getValue()).isTrue();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,6 @@ public static BuildConfig load(String json) throws IOException {
return MAPPER.readValue(json, BuildConfig.class);
}

public static BuildConfig fromString(String json) throws IOException {
return load(json);
}

public static BuildConfig load(URL url) throws IOException {
return MAPPER.readValue(url, BuildConfig.class);
}
Expand Down Expand Up @@ -135,7 +131,7 @@ public static BuildConfig merge(BuildConfig config, URL url) throws IOException
return reader.readValue(url);
}

public static BuildConfig copy(BuildConfig baseConfig) throws IOException {
public static BuildConfig copy(BuildConfig baseConfig) {
return MAPPER.convertValue(baseConfig, BuildConfig.class);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
public class BuildStatistics {
private static final Logger LOGGER = LoggerFactory.getLogger(BuildStatistics.class);

private static final double ONE_HUNDRED_PERCENT = 100.00D;

private long numberOfBuilds;

private long numberOfArchives;
Expand Down Expand Up @@ -97,8 +99,9 @@ public BuildStatistics(Collection<KojiBuild> builds) {
}

if (!builds.isEmpty()) {
percentOfBuildsImported = ((double) numberOfImportedBuilds / (double) numberOfBuilds) * 100.00;
percentOfArchivesImported = ((double) numberOfImportedArchives / (double) numberOfArchives) * 100.00;
percentOfBuildsImported = ((double) numberOfImportedBuilds / (double) numberOfBuilds) * ONE_HUNDRED_PERCENT;
percentOfArchivesImported = ((double) numberOfImportedArchives / (double) numberOfArchives)
* ONE_HUNDRED_PERCENT;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ private Set<EnhancedArtifact> lookupArtifactsInPnc(ConcurrentHashMap<Checksum, C
}

/**
* A build produces multiple artifacts. This methods associates all the artifacts to the one PncBuild
* A build produces multiple artifacts. This method associates all the artifacts to the one PncBuild
*
* @param artifacts All found artifacts
* @return A map pncBuildId,pncBuild
Expand Down
7 changes: 5 additions & 2 deletions core/src/main/java/org/jboss/pnc/build/finder/core/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.math.RoundingMode;
import java.text.DecimalFormat;
import java.text.FieldPosition;
import java.time.Duration;
import java.util.Properties;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;
Expand All @@ -42,6 +43,8 @@ public final class Utils {

private static final Properties PROPERTIES;

public static final long TIMEOUT = Duration.ofSeconds(10L).toMillis();

static {
PROPERTIES = new Properties();

Expand All @@ -65,10 +68,10 @@ public static void shutdownAndAwaitTermination(ExecutorService pool) {
pool.shutdown();

try {
if (!pool.awaitTermination(10000L, TimeUnit.MILLISECONDS)) {
if (!pool.awaitTermination(TIMEOUT, TimeUnit.MILLISECONDS)) {
pool.shutdownNow();

if (!pool.awaitTermination(10000L, TimeUnit.MILLISECONDS)) {
if (!pool.awaitTermination(TIMEOUT, TimeUnit.MILLISECONDS)) {
LOGGER.error("Pool did not terminate");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,8 @@ public final class KojiClientSession extends KojiClient implements ClientSession

private final KojiClientHelper helper;

public KojiClientSession(
KojiConfig config,
PasswordManager passwordManager,
ExecutorService executorService,
com.codahale.metrics.MetricRegistry registry) throws KojiClientException {
public KojiClientSession(KojiConfig config, PasswordManager passwordManager, ExecutorService executorService)
throws KojiClientException {
super(config, passwordManager, executorService);
helper = new KojiClientHelper(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

import com.fasterxml.jackson.annotation.JsonIgnore;

// FIXME: To use current Infinispan cache, the fields have to be serializable
public class PncBuild {
private Build build;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

import org.jboss.pnc.build.finder.koji.KojiBuild;
import org.jboss.pnc.build.finder.pnc.PncBuild;
import org.jboss.pnc.dto.Artifact;
import org.jboss.pnc.dto.ArtifactRef;
import org.jboss.pnc.dto.Build;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -79,9 +79,9 @@ private PncUtils() {
}

// FIXME: Implement a better way of reading GAV from PNC as not all builds are pushed to Brew
private static void setMavenBuildInfoFromBuildRecord(Build record, KojiBuildInfo buildInfo) {
String executionRootName = getSafelyExecutionRootName(record);
String executionRootVersion = record.getAttributes().get(BUILD_BREW_VERSION);
private static void setMavenBuildInfoFromBuildRecord(Build build, KojiBuildInfo buildInfo) {
String executionRootName = getSafelyExecutionRootName(build);
String executionRootVersion = build.getAttributes().get(BUILD_BREW_VERSION);
String[] ga = executionRootName.split(":", 2);

if (ga.length >= 2) {
Expand Down Expand Up @@ -110,9 +110,9 @@ private static String getBrewBuildVersionOrZero(Build build) {
}
}

public static String getNVRFromBuildRecord(Build record) {
return getSafelyExecutionRootName(record).replace(':', '-') + "-"
+ record.getAttributes().get(BUILD_BREW_VERSION) + "-1";
public static String getNVRFromBuildRecord(Build build) {
return getSafelyExecutionRootName(build).replace(':', '-') + "-" + build.getAttributes().get(BUILD_BREW_VERSION)
+ "-1";
}

public static KojiBuild pncBuildToKojiBuild(PncBuild pncbuild) {
Expand Down Expand Up @@ -201,7 +201,7 @@ public static KojiBuild pncBuildToKojiBuild(PncBuild pncbuild) {
return kojiBuild;
}

public static KojiArchiveInfo artifactToKojiArchiveInfo(PncBuild pncbuild, Artifact artifact) {
public static KojiArchiveInfo artifactToKojiArchiveInfo(PncBuild pncbuild, ArtifactRef artifact) {
Build build = pncbuild.getBuild();
KojiArchiveInfo archiveInfo = new KojiArchiveInfo();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public MultiValuedMapProtobufWrapper(MultiValuedMap<K, V> map) {
/**
* This method is called for Protobuf to MultiValuedMapWrapper convertor.
*
* @param entries
* @param entries the entries
*/
@ProtoFactory
MultiValuedMapProtobufWrapper(Collection<MultiValuedMapProtobufEntry<K, V>> entries) {
Expand All @@ -58,16 +58,16 @@ Collection<MultiValuedMapProtobufEntry<K, V>> getEntries() {
return this.asMap()
.entrySet()
.stream()
.map(a -> new MultiValuedMapProtobufEntry<K, V>(a.getKey(), a.getValue()))
.map(a -> new MultiValuedMapProtobufEntry<>(a.getKey(), a.getValue()))
.collect(Collectors.toList());
}

/**
* POJO representing the key and list of values of MultiValuedMap for Protobuf.
*
* <p>
* The key and values are marshalled as a WrappedMessage. In theory, the WrappedMessage object should know how to
* marshall primitives, enums, and other POJO objects which can be marshalled by Protobuf (as long as they are in
* the `AutoProtoSchemaBuilder` list in the ProtobufSerializer interface.
* the `AutoProtoSchemaBuilder` list in the ProtobufSerializer interface).
*
* @param <K> Key type
* @param <V> Value type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.contentOf;
import static org.jboss.pnc.build.finder.core.ChecksumType.sha256;

import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -206,7 +207,7 @@ void testLoadFromClassPath() throws IOException {
"xml");
assertThat(bc2.getArchiveTypes()).containsExactly("jar");
assertThat(bc2.getChecksumOnly()).isTrue();
assertThat(bc2.getChecksumTypes()).containsExactly(ChecksumType.sha256);
assertThat(bc2.getChecksumTypes()).containsExactly(sha256);
assertThat(bc2.getKojiHubURL().toExternalForm()).isEqualTo("https://my.url.com/hub");
assertThat(bc2.getKojiWebURL().toExternalForm()).isEqualTo("https://my.url.com/web");

Expand All @@ -229,7 +230,7 @@ void testLoadFromClassPath() throws IOException {
"xml");
assertThat(merged.getArchiveTypes()).containsExactly("jar");
assertThat(merged.getChecksumOnly()).isTrue();
assertThat(merged.getChecksumTypes()).containsExactly(ChecksumType.sha256);
assertThat(merged.getChecksumTypes()).containsExactly(sha256);
assertThat(merged.getKojiHubURL().toExternalForm()).isEqualTo("https://my.url.com/hub");
assertThat(merged.getKojiWebURL().toExternalForm()).isEqualTo("https://my.url.com/web");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
package org.jboss.pnc.build.finder.core;

import static org.assertj.core.api.Assertions.assertThat;
import static org.jboss.pnc.build.finder.core.ChecksumType.md5;
import static org.jboss.pnc.build.finder.core.ChecksumType.sha1;
import static org.jboss.pnc.build.finder.core.ChecksumType.sha256;
import static org.junit.jupiter.params.provider.Arguments.arguments;

import java.io.File;
Expand Down Expand Up @@ -62,8 +65,7 @@ void testEmptyList() throws IOException {
map.forEach((key, value) -> assertThat(value.asMap()).isEmpty());
}

// XXX: Disabled for performance
@Disabled
@Disabled("Disabled for performance reasons")
@Test
void testSize(@TempDir File folder) throws IOException {
File test = new File(folder, "test-size");
Expand Down Expand Up @@ -126,7 +128,7 @@ void testLoadNestedZip() throws IOException {
DistributionAnalyzer da = new DistributionAnalyzer(target, config);
Map<ChecksumType, MultiValuedMap<String, LocalFile>> checksums = da.checksumFiles();

assertThat(checksums.get(ChecksumType.md5).size()).isEqualTo(25);
assertThat(checksums.get(md5).size()).isEqualTo(25);
}

@Test
Expand All @@ -137,7 +139,7 @@ void testLoadNestedWar() throws IOException {
DistributionAnalyzer da = new DistributionAnalyzer(target, config);
Map<ChecksumType, MultiValuedMap<String, LocalFile>> checksums = da.checksumFiles();

assertThat(checksums.get(ChecksumType.md5).size()).isEqualTo(7);
assertThat(checksums.get(md5).size()).isEqualTo(7);
}

@StdIo
Expand All @@ -149,7 +151,7 @@ void testLoadManPageZip(StdOut out) throws IOException {
DistributionAnalyzer da = new DistributionAnalyzer(target, config);
Map<ChecksumType, MultiValuedMap<String, LocalFile>> checksums = da.checksumFiles();

assertThat(checksums.get(ChecksumType.md5).size()).isEqualTo(4);
assertThat(checksums.get(md5).size()).isEqualTo(4);
assertThat(out.capturedLines()).anyMatch(line -> line.contains("Unable to process archive/compressed file"));
}

Expand All @@ -161,7 +163,7 @@ void testLoadNestedZipMultiThreaded() throws IOException {
DistributionAnalyzer da = new DistributionAnalyzer(target, config);
Map<ChecksumType, MultiValuedMap<String, LocalFile>> checksums = da.call();

assertThat(checksums.get(ChecksumType.md5).size()).isEqualTo(25);
assertThat(checksums.get(md5).size()).isEqualTo(25);
}

@Test
Expand All @@ -177,8 +179,7 @@ void testLoadNestedZipMultiThreadedMultipleChecksumTypes() throws IOException {
Map<ChecksumType, MultiValuedMap<String, LocalFile>> checksums = da.call();

assertThat(checksums.keySet()).hasSameSizeAs(config.getChecksumTypes());
assertThat(config.getChecksumTypes())
.containsExactlyInAnyOrder(ChecksumType.md5, ChecksumType.sha1, ChecksumType.sha256);
assertThat(config.getChecksumTypes()).containsExactlyInAnyOrder(md5, sha1, sha256);

Set<ChecksumType> checksumTypes = config.getChecksumTypes();

Expand Down Expand Up @@ -219,6 +220,6 @@ void testLoadNestedNoRecursion(String filename, int numChecksums) throws IOExcep
DistributionAnalyzer da = new DistributionAnalyzer(target, config);
Map<ChecksumType, MultiValuedMap<String, LocalFile>> checksums = da.checksumFiles();

assertThat(checksums.get(ChecksumType.md5).size()).isEqualTo(numChecksums);
assertThat(checksums.get(md5).size()).isEqualTo(numChecksums);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
package org.jboss.pnc.build.finder.core;

import static org.assertj.core.api.Assertions.assertThat;
import static org.jboss.pnc.constants.Attributes.BUILD_BREW_NAME;
import static org.jboss.pnc.constants.Attributes.BUILD_BREW_VERSION;
import static org.jboss.pnc.enums.BuildType.MVN;
import static org.mockito.Mockito.when;

import java.time.Instant;
Expand All @@ -36,14 +39,12 @@
import org.jboss.pnc.build.finder.pnc.client.StaticRemoteCollection;
import org.jboss.pnc.client.RemoteResourceException;
import org.jboss.pnc.client.RemoteResourceNotFoundException;
import org.jboss.pnc.constants.Attributes;
import org.jboss.pnc.dto.Artifact;
import org.jboss.pnc.dto.Build;
import org.jboss.pnc.dto.BuildConfigurationRevisionRef;
import org.jboss.pnc.dto.ProjectRef;
import org.jboss.pnc.dto.SCMRepository;
import org.jboss.pnc.dto.User;
import org.jboss.pnc.enums.BuildType;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
Expand Down Expand Up @@ -88,8 +89,8 @@ void testFindOneBuildInPnc() throws RemoteResourceException {
String buildId = "100";

Map<String, String> attributes = new HashMap<>(2);
attributes.put(Attributes.BUILD_BREW_NAME, "org.empty-empty");
attributes.put(Attributes.BUILD_BREW_VERSION, "1.0.0");
attributes.put(BUILD_BREW_NAME, "org.empty-empty");
attributes.put(BUILD_BREW_VERSION, "1.0.0");

Build build = Build.builder()
.id(buildId)
Expand All @@ -101,8 +102,7 @@ void testFindOneBuildInPnc() throws RemoteResourceException {
.scmRepository(SCMRepository.builder().internalUrl("http://repo.test/empty.git").build())
.scmRevision("master")
.project(ProjectRef.refBuilder().id("100").build())
.buildConfigRevision(
BuildConfigurationRevisionRef.refBuilder().id("100").buildType(BuildType.MVN).build())
.buildConfigRevision(BuildConfigurationRevisionRef.refBuilder().id("100").buildType(MVN).build())
.build();

Artifact artifact = Artifact.builder()
Expand Down
Loading