[ https://issues.apache.org/jira/browse/MINIFI-415?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16323016#comment-16323016 ]
ASF GitHub Bot commented on MINIFI-415: --------------------------------------- Github user jzonthemtn commented on a diff in the pull request: https://github.com/apache/nifi-minifi/pull/108#discussion_r161082784 --- Diff: minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-runtime/src/main/java/org/apache/nifi/minifi/FlowEnricher.java --- @@ -130,17 +127,13 @@ private void enrichComponent(EnrichingElementAdapter componentToEnrich, Map<Stri componentToEnrich.setBundleInformation(enrichingBundleCoordinate); componentToEnrich.setDependsUponBundleCoordinate(enrichingBundleDetails.getDependencyCoordinate()); } else { - - // mUltiple options + // multiple options final Set<String> componentToEnrichBundleVersions = componentToEnrichVersionToBundles.values().stream() .map(bundle -> bundle.getBundleDetails().getCoordinate().getVersion()).collect(Collectors.toSet()); - final String componentToEnrichId = componentToEnrich.getComponentId(); - String bundleVersion = componentToEnrichBundleVersions.stream().sorted().reduce((version, otherVersion) -> otherVersion).orElse(null); - if (bundleVersion != null) { - componentToEnrich.setBundleInformation(componentToEnrichVersionToBundles.get(bundleVersion).getBundleDetails().getCoordinate()); - } - logger.info("Enriching {} with bundle {}", new Object[]{}); - + final String bundleVersion = componentToEnrichBundleVersions.stream().sorted().reduce((version, otherVersion) -> otherVersion).orElse(null); + final BundleCoordinate enrichingCoordinate = componentToEnrichVersionToBundles.get(bundleVersion).getBundleDetails().getCoordinate(); --- End diff -- Is the null check on `bundleVersion` still necessary to prevent trying to get a map value by key `null`? > Bundle version number should not be compared as a simple String > --------------------------------------------------------------- > > Key: MINIFI-415 > URL: https://issues.apache.org/jira/browse/MINIFI-415 > Project: Apache NiFi MiNiFi > Issue Type: Bug > Components: Agent Configuration/Installation > Affects Versions: 0.3.0 > Reporter: Koji Kawamura > Assignee: Aldrin Piri > Priority: Minor > > MINIFI-408 added support for picking the latest bundle version automatically > if there are multiple versions for the same Nar. However, it compares version > as simple Strings and may not be able to pick the latest one semantically. > https://github.com/apache/nifi-minifi/pull/99/files#diff-c7d8398db8540d6e85f1a9207438ebddR138 > Following code shows the problematic inputs and possible solution. Current > implementation picks "1.0.9", and using Version class, it can pick "1.0.12". > {code} > class Test { > public static void main(String[] args) throws Exception { > Set<String> componentToEnrichBundleVersions = new HashSet<>(); > componentToEnrichBundleVersions.add("1.0.0"); > componentToEnrichBundleVersions.add("1.0.5"); > componentToEnrichBundleVersions.add("1.0.9"); > componentToEnrichBundleVersions.add("1.0.11-SNAPSHOT"); > componentToEnrichBundleVersions.add("1.0.12"); > // Current implementation > final String bundleVersion = > componentToEnrichBundleVersions.stream().sorted() > .reduce((version, otherVersion) -> otherVersion).orElse(null); > // Suggestion > final Version latestVersion = > componentToEnrichBundleVersions.stream().map(Version::fromString).sorted() > .reduce((version, otherVersion) -> otherVersion).orElse(null); > System.out.println(bundleVersion); > System.out.println(latestVersion.toVersionString()); > } > } > class Version implements Comparable<Version> { > private static Pattern P = > Pattern.compile("^([\\d]+)\\.([\\d]+)\\.([\\d]+)([^\\d]*)$"); > final int major; > final int minor; > final int patch; > final String opt; > public static Version fromString(String s) { > final Matcher matcher = P.matcher(s); > if (matcher.matches()) { > return new Version( > Integer.parseInt(matcher.group(1)), > Integer.parseInt(matcher.group(2)), > Integer.parseInt(matcher.group(3)), > matcher.group(4)); > } > throw new IllegalArgumentException("Unknown version pattern " + s); > } > private Version(int major, int minor, int patch, String opt) { > this.major = major; > this.minor = minor; > this.patch = patch; > this.opt = opt; > } > @Override > public int compareTo(@NotNull Version o) { > final int ma = new Integer(major).compareTo(o.major); > if (ma != 0) { > return ma; > } > final int mi = new Integer(minor).compareTo(o.minor); > if (mi != 0) { > return mi; > } > final int pa = new Integer(patch).compareTo(o.patch); > if (pa != 0) { > return pa; > } > final String o1 = opt != null ? opt : ""; > final String o2 = o.opt != null ? o.opt : ""; > final int op = o1.compareTo(o2); > return op; > } > public String toVersionString() { > return String.format("%d.%d.%d%s", major, minor, patch, opt); > } > } > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)