35 audit one liners [pr] (#36)

* PRComment_002 * PRComment_005 * PRComment_009 * PRComment_010 * PRComment_012 * PRComment_015 I might suggest logging this and the following error at the WARN or INFO level as this appears to default to a value when the input is not correct. * PRComment_016 An alternative syntax for map get is ids[key]. See https://groovy-lang.org/style-guide.html#_native_syntax_for_data_structures https://stackoverflow.com/questions/14196454/groovy-safely-find-a-key-in-a-map-and-return-its-value * PRComment_017 Typically, the fields and constructor would appear at the top of the class. These should also be camelCase. For reference: https://stackoverflow.com/a/12427127 * PRComment_018 A common naming convention for constants like this string would be upper snake case like INSTALL_PREFIX https://google.github.io/styleguide/javaguide.html#s5.2.4-constant-names * PRComment_021 > Should this be logged at the ERROR level? Not really. We can use this same codepath when creating a new package. * PRComment_027 The safe navigation operator seems unnecessary here due to the preceding !query check. * PRComment_030 I think this would be more readable to pass the closure directly * PRComment_015a, PRComment_021a .warning -> .warn * recast PRComment_010 QuiltObserverFactory.groovy: 37: [Static type checking] - Incompatible generic argument types. Cannot assign java.util.List <nextflow.quilt.QuiltObserver> to: java.util.Collection <TraceObserver> @ line 37, column 9. [new QuiltObserver()] * PRComment_031 The Groovy safe navigation operator is a nice way to express this: * PRComment_032. Groovy also has an exclusive range operator: beginIndex..<endIndex. * PRComment_033 You should be able to omit the package name here: * PRComment_036 to Groovy truthiness, empty string and null are both false * PRComment_038 @unroll is not needed here. * PRComment_040 @unroll is not needed here. * NoSuchFileException e PRComment_033, but it is non blocking.
quiltdata · Jan 23, 2023 · f22de03 · f22de03
1 parent 205d5ba
commit f22de03
Show file tree

Hide file tree

Showing 11 changed files with 32 additions and 48 deletions.
diff --git a/plugins/nf-quilt/src/main/nextflow/quilt/QuiltObserver.groovy b/plugins/nf-quilt/src/main/nextflow/quilt/QuiltObserver.groovy
@@ -45,35 +45,27 @@ class QuiltObserver implements TraceObserver {
     public static void writeString(String text, QuiltPackage pkg, String filename) {
         String dir = pkg.packageDest().toString()
         def path = Paths.get(dir, filename)
-        //log.debug "QuiltObserver.writeString[$path]: $text"
         Files.write(path, text.bytes)
     }
 
     private Session session
     private Map config
     private Map quilt_config
-    private Set<QuiltPackage> pkgs
+    private Set<QuiltPackage> pkgs = new HashSet<>()
 
     static String now(){
         def date = new Date()
         def sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss")
         return sdf.format(date)
     }
 
-    Set<QuiltPackage> ensurePkgs() {
-        if ( !this.pkgs ) {
-            this.pkgs = new HashSet<>()
-        }
-        this.pkgs
-    }
-
     @Override
     void onFlowCreate(Session session) {
         log.debug "`onFlowCreate` $this"
         this.session = session
         this.config = session.config
         this.quilt_config = session.config.navigate('quilt') as Map
-        ensurePkgs()
+        this.pkgs
     }
 
     @Override
@@ -82,7 +74,7 @@ class QuiltObserver implements TraceObserver {
         if( path instanceof QuiltPath ) {
             QuiltPath qPath = (QuiltPath)path
             QuiltPackage pkg = qPath.pkg()
-            ensurePkgs().add(pkg)
+            this.pkgs.add(pkg)
             log.debug "onFilePublish.QuiltPath[$qPath]: pkgs=${pkgs}"
         }
     }
@@ -147,7 +139,7 @@ ${meta['workflow']['stats']['processes']}
     static void printMap(Map map, String title) {
         log.info "\n\n\n# $title"
         map.each{
-            key, value -> print "\n## $key\n\n$value";
+            key, value -> log.info "\n## $key\n\n$value";
         }
     }
 

diff --git a/plugins/nf-quilt/src/main/nextflow/quilt/QuiltObserverFactory.groovy b/plugins/nf-quilt/src/main/nextflow/quilt/QuiltObserverFactory.groovy
@@ -34,8 +34,6 @@ class QuiltObserverFactory implements TraceObserverFactory {
     @Override
     Collection<TraceObserver> create(Session session) {
         log.debug "`create` ${this}"
-        final result = new ArrayList()
-        result.add( new QuiltObserver() )
-        return result
+        (Collection<TraceObserver>) [new QuiltObserver()]
     }
 }
diff --git a/plugins/nf-quilt/src/main/nextflow/quilt/QuiltOpts.groovy b/plugins/nf-quilt/src/main/nextflow/quilt/QuiltOpts.groovy
@@ -65,8 +65,7 @@ class QuiltOpts {
         //
         // If not using the default, the profile needs to be set in the nextflow config file.
 
-        final config = fromSession(session)
-        return config
+        return fromSession(session)
     }
 
 }
diff --git a/plugins/nf-quilt/src/main/nextflow/quilt/jep/QuiltID.groovy b/plugins/nf-quilt/src/main/nextflow/quilt/jep/QuiltID.groovy
@@ -24,41 +24,40 @@ import groovy.util.logging.Slf4j
 @CompileStatic
 class QuiltID {
     public static String[] DEFAULT_PACKAGE=["null","default"]
-
     private static final Map<String,QuiltID> ids = [:]
 
+    private final String bucket
+    private final String pkgPrefix
+    private final String pkgSuffix
+
     static public QuiltID Fetch(String bucket, String pkg_name) {
         if (!bucket) {
             log.error "null == QuiltID.Fetch($bucket, $pkg_name)"
             return null
         }
         if (!pkg_name || pkg_name.size()<QuiltParser.MIN_SIZE) {
             pkg_name = DEFAULT_PACKAGE.join(QuiltParser.SEP)
-            log.error "QuiltID.Fetch: setting missing package to $pkg_name"
+            log.warn "QuiltID.Fetch: setting missing package to $pkg_name"
         }
         String[] split = pkg_name.split(QuiltParser.SEP)
         if (split.size()<QuiltParser.MIN_SIZE || split[1].size()<QuiltParser.MIN_SIZE) {
             split += DEFAULT_PACKAGE[1] as String
-            log.error "QuiltID.Fetch: setting missing suffix to $split[1]"
+            log.warn "QuiltID.Fetch: setting missing suffix to $split[1]"
         }
         String key = "${bucket}/${split[0]}/${split[1]}"
-        def id = ids.get(key)
-        if (id) return id
-        ids[key] = new QuiltID(bucket, split[0], split[1])
+        if (!ids.containsKey(key)) {
+            ids[key] = new QuiltID(bucket, split[0], split[1])
+        }
         ids[key]
     }
 
-    private final String bucket
-    private final String pkg_prefix
-    private final String pkg_suffix
-
-    QuiltID(String bucket, String pkg_prefix, String pkg_suffix) {
+    QuiltID(String bucket, String pkgPrefix, String pkgSuffix) {
         this.bucket = bucket
-        this.pkg_prefix = pkg_prefix
-        this.pkg_suffix = pkg_suffix
+        this.pkgPrefix = pkgPrefix
+        this.pkgSuffix = pkgSuffix
     }
 
     String toString() {
-        "${bucket}.${pkg_prefix}.${pkg_suffix}"
+        "${bucket}.${pkgPrefix}.${pkgSuffix}"
     }
 }
diff --git a/plugins/nf-quilt/src/main/nextflow/quilt/jep/QuiltPackage.groovy b/plugins/nf-quilt/src/main/nextflow/quilt/jep/QuiltPackage.groovy
@@ -36,8 +36,8 @@ import java.lang.ProcessBuilder
 @CompileStatic
 class QuiltPackage {
     private static final Map<String,QuiltPackage> packages = [:]
-    private static final String installPrefix = "QuiltPackage"
-    public static final Path installRoot = Files.createTempDirectory(installPrefix)
+    private static final String INSTALL_PREFIX = "QuiltPackage"
+    public static final Path INSTALL_ROOT = Files.createTempDirectory(INSTALL_PREFIX)
 
     private final String bucket
     private final String pkg_name
@@ -56,7 +56,7 @@ class QuiltPackage {
             pkg.install()
         }
         catch (Exception e) {
-            log.debug "Package `${parsed.toUriString()}` does not yet exist"
+            log.warn "Package `${parsed.toUriString()}` does not yet exist"
         }
         return pkg
     }
@@ -87,7 +87,7 @@ class QuiltPackage {
         this.bucket = parsed.bucket()
         this.pkg_name = parsed.pkg_name()
         this.hash = parsed.hash()
-        this.folder = Paths.get(installRoot.toString(), this.toString())
+        this.folder = Paths.get(INSTALL_ROOT.toString(), this.toString())
         assert this.folder
         this.setup()
     }

diff --git a/plugins/nf-quilt/src/main/nextflow/quilt/jep/QuiltParser.groovy b/plugins/nf-quilt/src/main/nextflow/quilt/jep/QuiltParser.groovy
@@ -67,8 +67,8 @@ class QuiltParser {
     }
 
     static private Map<String,Object> parseQuery(String query) {
-        if (!query) return [:]
-        final queryParams = query?.split('&') // safe operator for urls without query params
+        if (!query) return [:] // skip for urls without query params
+        final queryParams = query.split('&') 
         queryParams.collectEntries { param -> param.split('=').collect { URLDecoder.decode(it) }}
     }
 
@@ -131,7 +131,7 @@ class QuiltParser {
 
     QuiltParser normalized() {
         boolean skip = false
-        def norm = { String x ->
+        String[] rnorms = paths.reverse().findAll { String x ->
             if (x == "..") {
                 skip = true
                 false
@@ -142,7 +142,7 @@ class QuiltParser {
                 true
             }
         }
-        String[] rnorms = paths.reverse().findAll(norm)
+
         log.debug("normalized: ${paths} -> ${rnorms}")
         String path2 = rnorms.reverse().join(SEP)
         log.debug("normalized: -> ${path2}")
@@ -158,7 +158,7 @@ class QuiltParser {
     }
 
     String bucket() {
-        bucket ? bucket.toLowerCase() : null
+        bucket?.toLowerCase()
     }
 
     String pkg_name() {
@@ -178,7 +178,7 @@ class QuiltParser {
     }
 
     String path(int beginIndex, int endIndex) {
-        String[] sub = paths[beginIndex..(endIndex-1)]
+        String[] sub = paths[beginIndex..<endIndex]
         sub.join(SEP)
     }
 
@@ -191,7 +191,7 @@ class QuiltParser {
     }
 
     String options(String key) {
-        options ? options.get(key) : null
+        options?.get(key)
     }
 
     String toPackageString() {

diff --git a/plugins/nf-quilt/src/main/nextflow/quilt/nio/QuiltFileSystem.groovy b/plugins/nf-quilt/src/main/nextflow/quilt/nio/QuiltFileSystem.groovy
@@ -119,7 +119,7 @@ public final class QuiltFileSystem extends FileSystem {
             BasicFileAttributes attrs = Files.readAttributes(installedPath, BasicFileAttributes)
             return new QuiltFileAttributes(path,path.toString(),attrs)
         }
-        catch (java.nio.file.NoSuchFileException e) {
+        catch (NoSuchFileException e) {
             log.debug "No attributes yet for: ${installedPath}"
         }
         return null

diff --git a/plugins/nf-quilt/src/main/nextflow/quilt/nio/QuiltPath.groovy b/plugins/nf-quilt/src/main/nextflow/quilt/nio/QuiltPath.groovy
@@ -95,8 +95,7 @@ public final class QuiltPath implements Path {
     @Override
     boolean isAbsolute() {
         log.debug "isAbsolute[${pkg_name()}] : ${parsed}"
-
-        filesystem && pkg_name() != "" && pkg_name() != null
+        filesystem && pkg_name()
     }
 
     boolean isJustPackage() {

diff --git a/plugins/nf-quilt/src/test/nextflow/quilt/jep/QuiltPackageTest.groovy b/plugins/nf-quilt/src/test/nextflow/quilt/jep/QuiltPackageTest.groovy
@@ -52,7 +52,6 @@ class QuiltPackageTest extends QuiltSpecification {
         pkg = qpath.pkg()
     }
 
-    @Unroll
     def 'should create unique Package for associated Paths' () {
         given:
         def pkgPath = qpath.getJustPackage()

diff --git a/plugins/nf-quilt/src/test/nextflow/quilt/nio/QuiltNioTest.groovy b/plugins/nf-quilt/src/test/nextflow/quilt/nio/QuiltNioTest.groovy
@@ -476,7 +476,6 @@ class QuiltNioTest extends QuiltSpecification {
         when:
         def p = Paths.get(new URI(null_url))
         def list = Files.newDirectoryStream(p).collect {
-             //log.debug "newDirectoryStream[$p]: $it"
              it.getFileName().toString()
         }
         then:

diff --git a/plugins/nf-quilt/src/test/nextflow/quilt/nio/QuiltPathFactoryTest.groovy b/plugins/nf-quilt/src/test/nextflow/quilt/nio/QuiltPathFactoryTest.groovy
@@ -32,7 +32,6 @@ class QuiltPathFactoryTest extends QuiltSpecification {
     static String pkg_url = 'quilt+s3://quilt-example#package=examples/hurdat@f8d1478d93'
     static String url = pkg_url + '&path=scripts/build.py'
 
-    @Unroll
     def 'should decompose Quilt URLs' () {
         given:
         def qpath = QuiltPathFactory.Parse(url)
-Original file line number
+Diff line change
@@ Expand Up / @@ -52,7 +52,6 @@ class QuiltPackageTest extends QuiltSpecification { @@
             pkg = qpath.pkg()
         }
-        @Unroll
         def 'should create unique Package for associated Paths' () {
             given:
             def pkgPath = qpath.getJustPackage()
@@ Expand Down @@