[
https://issues.apache.org/jira/browse/MINIFI-415?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16323300#comment-16323300
]
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_r161116290
--- 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 --
As long as its guaranteed to not be `null` I think it is ok as is. The
`orElse` is what raised a flag for me.
> 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)
