nifi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MINIFI-415) Bundle version number should not be compared as a simple String
Date Thu, 11 Jan 2018 22:34:00 GMT

    [ https://issues.apache.org/jira/browse/MINIFI-415?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16323113#comment-16323113
] 

ASF GitHub Bot commented on MINIFI-415:
---------------------------------------

Github user apiri commented on a diff in the pull request:

    https://github.com/apache/nifi-minifi/pull/108#discussion_r161095169
  
    --- 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 --
    
    Probably doesn't hurt but my thought when I looked at it again was that given that we
have already determined that there is a non-empty collection of possible candidates and we
are just selecting the greatest of these through the reduction, there will be a guaranteed
value.  What makes it awkward is the orElse that was left behind.  I suppose I could change
this to just #get() and the meaning would be a little clearer.  What do you think makes the
most sense?


> 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)

Mime
View raw message