CMR-5410: Remove use of reflection in many places in the CMR codebase. (

nasa#674)

* CMR-5410: Remove cyclic dependency between acl-lib and common-app-lib. Remove use of reflection in many places in the CMR codebase.

* CMR-5410: Back out changes to SQS namespace. Removed debug.

* CMR-5410: Allow reflection to fix failing unit tests which were using Long rather than strings which is what compojure uses for parameters.

acl-lib/src/cmr/acl/acl_fetcher.clj

@@ -5,7 +5,7 @@
   for callers without any"
   (:require
    [clojure.set :as set]
-   [cmr.common-app.cache.consistent-cache :as consistent-cache]
+   [cmr.transmit.cache.consistent-cache :as consistent-cache]
    [cmr.common.cache :as cache]
    [cmr.common.cache.single-thread-lookup-cache :as stl-cache]
    [cmr.common.config :refer [defconfig]]

common-app-lib/src/cmr/common_app/api/enabled.clj

@@ -7,9 +7,8 @@
   anything from writing on the NGAP side while we are transitioning."
   (:require
    [cheshire.core :as json]
-   [cmr.acl.core :as acl]
    [cmr.common.api.context :as context]
-   [cmr.common-app.cache.consistent-cache :as consistent-cache]
+   [cmr.transmit.cache.consistent-cache :as consistent-cache]
    [cmr.common.cache :as cache]
    [cmr.common.cache.in-memory-cache :as mem-cache]
    [cmr.common.cache.single-thread-lookup-cache :as stl-cache]

common-app-lib/src/cmr/common_app/api/request_context_user_augmenter.clj

@@ -10,7 +10,6 @@
   the system datastructure."
   (:require
    [cheshire.core :as json]
-   [cmr.acl.core :as acl]
    [cmr.common.cache :as cache]
    [cmr.common.cache.in-memory-cache :as mem-cache]
    [cmr.common.log :refer (info)]

common-app-lib/src/cmr/common_app/api/routes.clj

@@ -164,7 +164,7 @@
 
 (defn update-response-body-with-fn
   [response f]
-  (let [new-body (f (:body response))]
+  (let [^String new-body (f (:body response))]
     (-> response
         (assoc-in [:body] new-body)
         (assoc-in [:headers "Content-Length"] (str (count (.getBytes new-body "UTF-8"))))

common-app-lib/src/cmr/common_app/services/kms_fetcher.clj

@@ -14,8 +14,8 @@
          :providers [...]}"
   (:require
     [clojure.string :as str]
-    [cmr.common-app.cache.consistent-cache :as consistent-cache]
-    [cmr.common-app.cache.cubby-cache :as cubby-cache]
+    [cmr.transmit.cache.consistent-cache :as consistent-cache]
+    [cmr.transmit.cache.cubby-cache :as cubby-cache]
     [cmr.common-app.services.kms-lookup :as kms-lookup]
     [cmr.common.cache :as cache]
     [cmr.common.cache.deflating-cache :as deflating-cache]

common-app-lib/src/cmr/common_app/services/search.clj

@@ -7,7 +7,7 @@
    [cmr.common.config :as cfg :refer [defconfig]]
    [cmr.common.log :refer (debug info warn error)]
    [cmr.common.services.errors :as errors]
-   [cmr.common-app.cache.cubby-cache :as cubby-cache]
+   [cmr.transmit.cache.cubby-cache :as cubby-cache]
    [cmr.common.cache.in-memory-cache :as mem-cache]
    [cmr.common-app.services.search.query-validation :as qv]
    [cmr.common-app.services.search.query-execution :as qe]

common-app-lib/src/cmr/common_app/services/search/elastic_search_index.clj

@@ -12,7 +12,7 @@
    [cmr.common-app.services.search.results-model :as results]
    [cmr.common.concepts :as concepts]
    [cmr.common.lifecycle :as lifecycle]
-   [cmr.common.log :refer (debug info warn error)]
+   [cmr.common.log :refer [debug info warn error]]
    [cmr.common.services.errors :as errors]
    [cmr.common.util :as util]
    [cmr.elastic-utils.config :as es-config]
@@ -185,9 +185,9 @@
             (update-in [:took] + took-total)
             (update-in [:hits :hits] concat prev-items))
         ;; We need to keep searching subsequent pages
-        (recur (+ offset unlimited-page-size)
+        (recur (long (+ offset unlimited-page-size))
                (concat prev-items current-items)
-               (+ took-total (:took results)))))))
+               (long (+ took-total (:took results))))))))
 
 
 (defn execute-query

common-app-lib/src/cmr/common_app/services/search/parameter_validation.clj

@@ -102,7 +102,7 @@
   if the value cannot be converted to an Integer."
   [params value-keyword]
   (when-let [value (value-keyword params)]
-    ; Return null if value is a vector.  Assumes single-value-validation handles vectors.
+    ;; Return null if value is a vector.  Assumes single-value-validation handles vectors.
     (when-not (sequential? value)
       (Integer. value))))
 

common-app-lib/src/cmr/common_app/static.clj

@@ -150,8 +150,7 @@
    [ring.util.request :as request]
    [selmer.parser :as selmer])
   (:import
-   [org.pegdown PegDownProcessor
-                Extensions]))
+   (org.pegdown Extensions PegDownProcessor)))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;; Utility Functions & Constants
@@ -183,7 +182,7 @@
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;; Routing Setup
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-  
+
 (def site-provider-map
   "Defines site and a sample provider map for all sites"
   {"cmr.earthdata.nasa.gov" "PODAAC"
@@ -195,41 +194,41 @@
   "Defines routes for returning API documentation. Takes the public-protocol (http or https),
   relative-root-url of the application, and the location of the welcome page within the classpath."
   ([public-protocol relative-root-url]
-    (routes
-      (context "/site" []
-        ;; Return swagger.json if the application provides one
-        (GET "/swagger.json" {:keys [headers]}
-          (if-let [resource (site-resource "swagger.json")]
-            {:status 200
+   (routes
+     (context "/site" []
+       ;; Return swagger.json if the application provides one
+       (GET "/swagger.json" {:keys [headers]}
+         (if-let [resource (site-resource "swagger.json")]
+           {:status 200
              :body (-> resource
                        slurp
                        (string/replace "%CMR-PROTOCOL%" public-protocol)
                        (string/replace "%CMR-HOST%" (headers "host"))
                        (string/replace "%CMR-BASE-PATH%" relative-root-url))
              :headers (:headers cr/options-response)}
-            (route/not-found (site-resource "404.html"))))
-        ;; Static HTML resources, typically API documentation which needs endpoint URLs replaced
-        (GET ["/:page", :page #".*\.html$"] {headers :headers, {page :page} :params}
-          (when-let [resource (site-resource page)]
-            (let [cmr-root (str public-protocol "://" (headers "host") relative-root-url)
-                  site-example-provider (get site-provider-map (headers "host") "PROV1") 
-                  cmr-example-collection-id (str "C1234567-" site-example-provider)]
-              {:status 200
+           (route/not-found (site-resource "404.html"))))
+       ;; Static HTML resources, typically API documentation which needs endpoint URLs replaced
+       (GET ["/:page", :page #".*\.html$"] {headers :headers, {page :page} :params}
+         (when-let [resource (site-resource page)]
+           (let [cmr-root (str public-protocol "://" (headers "host") relative-root-url)
+                 site-example-provider (get site-provider-map (headers "host") "PROV1")
+                 cmr-example-collection-id (str "C1234567-" site-example-provider)]
+             {:status 200
                :body (-> resource
                          slurp
                          (string/replace "%CMR-ENDPOINT%" cmr-root)
                          (string/replace "%CMR-EXAMPLE-COLLECTION-ID%" cmr-example-collection-id))})))
-        ;; Other static resources (Javascript, CSS)
-        (route/resources "/" {:root resource-root})
-        (pages/not-found))))
+       ;; Other static resources (Javascript, CSS)
+       (route/resources "/" {:root resource-root})
+       (pages/not-found))))
   ([public-protocol relative-root-url welcome-page-location]
-    (routes
-      ;; CMR Application Welcome Page
-      (GET "/" req
-        (force-trailing-slash req ; Without a trailing slash, the relative URLs in index.html are wrong
-                              {:status 200
-                               :body (slurp (io/resource welcome-page-location))}))
-      (docs-routes public-protocol relative-root-url))))
+   (routes
+     ;; CMR Application Welcome Page
+     (GET "/" req
+       (force-trailing-slash req ; Without a trailing slash, the relative URLs in index.html are wrong
+                             {:status 200
+                              :body (slurp (io/resource welcome-page-location))}))
+     (docs-routes public-protocol relative-root-url))))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;; Core API Documentation Functions

common-app-lib/src/cmr/common_app/test/client_util.clj

@@ -1,10 +1,11 @@
 (ns cmr.common-app.test.client-util
   "Contains test utilities for testing clients"
-  (:require [clj-http.client :as client]
-            [cmr.transmit.config :as config]
-            [cmr.transmit.connection :as conn]
-            [cmr.common.lifecycle :as l]
-            [cmr.common-app.test.side-api :as side-api]))
+  (:require
+   [clj-http.client :as client]
+   [cmr.common-app.test.side-api :as side-api]
+   [cmr.common.lifecycle :as l]
+   [cmr.transmit.config :as config]
+   [cmr.transmit.connection :as conn]))
 
 (defn url-available?
   "Returns true if the given url can be connected to."

common-lib/src/cmr/common/api/web_server.clj

@@ -60,7 +60,7 @@
   :at-end? - indicates whether the entire stream has been read.
   :bytes-read - number of bytes read from the stream. If nothing was read return 0 rather than -1.
   :buffer-full? - indicates whether the max-bytes-to-read were read."
-  [input-stream buffer offset max-bytes-to-read]
+  [^InputStream input-stream buffer offset max-bytes-to-read]
   (let [bytes-read (.read input-stream buffer offset max-bytes-to-read)]
     {:at-end? (= -1 bytes-read)
      :bytes-read (max 0 bytes-read) ;; Java input stream read will return -1 when no bytes are read
@@ -130,8 +130,8 @@
               ;; Reconstruct request body into a new input stream since the current has been read
               (let [request-body (byte-stream-from-buffers all-buffers total-bytes-read)]
                 (handler (assoc request :body request-body))))
-            (recur total-bytes-read
-                   bytes-read-for-current-buffer
+            (recur (long total-bytes-read)
+                   (long bytes-read-for-current-buffer)
                    all-buffers
                    current-buffer)))))))
 

common-lib/src/cmr/common/background_jobs.clj

@@ -2,10 +2,25 @@
   "Namespace for creating simple background jobs that run at a specified interval in a thread.
   For more complex requirements see the cmr.common.jobs namespace which use Quartz. An alternative
   to Quartz was required for the legacy-services application because it is using a version of Quartz
-  which is not compatible with the cmr.common.jobs namespace."
+  which is not compatible with the cmr.common.jobs namespace.
+
+  Note: We allow start to be called on already started jobs and stop to be called on jobs that are
+  not running without returning an error. That way the caller does not need to know the current
+  state, they only care that after the call the job is either started or stopped."
   (:require
    [cmr.common.lifecycle :as lifecycle]))
 
+(defn- create-thread-for-background-job
+  "Returns a Thread object to use to run the background job."
+  [job-fn job-interval-secs]
+  (Thread.
+   (fn []
+     (try
+      (while (not (.isInterrupted (Thread/currentThread)))
+        (job-fn)
+        (Thread/sleep (* 1000 job-interval-secs)))
+      (catch InterruptedException _)))))
+
 (defrecord BackgroundJob
   [;; The job-function to run
    job-fn
@@ -18,25 +33,24 @@
 
   (start
     [this system]
-    (when thread-ref (.interrupt thread-ref))
-    (let [thread (Thread.
-                   (fn []
-                     (try
-                      (while (not (.isInterrupted (Thread/currentThread)))
-                        (job-fn)
-                        (Thread/sleep (* 1000 job-interval-secs)))
-                      (catch InterruptedException _))))
-          ;; Have to set the thread reference before starting the thread otherwise interrupt fails
-          updated-this (assoc this :thread-ref thread)]
-      (.start (:thread-ref updated-this))
-      updated-this))
+    (when (= (Thread$State/NEW) (.getState thread-ref))
+      (.start thread-ref))
+    this)
 
   (stop
     [this system]
-    (when thread-ref (.interrupt thread-ref))
-    (assoc this :thread-ref nil)))
+    (when (and (not= (Thread$State/NEW) (.getState thread-ref))
+               (not (.isInterrupted thread-ref)))
+      (.interrupt thread-ref))
+    (if (= (Thread$State/NEW) (.getState thread-ref))
+      this
+      (->BackgroundJob job-fn
+                       job-interval-secs
+                       (create-thread-for-background-job job-fn job-interval-secs)))))
 
 (defn create-background-job
   "Creates a background job using the provided job-fn and how often to run the job in seconds."
   [job-fn job-interval-secs]
-  (->BackgroundJob job-fn job-interval-secs nil))
+  (->BackgroundJob job-fn
+                   job-interval-secs
+                   (create-thread-for-background-job job-fn job-interval-secs)))

common-lib/src/cmr/common/dev/util.clj

@@ -1,11 +1,12 @@
 (ns cmr.common.dev.util
   "This contains utility functions for development"
-  (:require [clojure.java.shell :as sh]
-            [clojure.java.io :as io]
-            [clojure.string :as str])
-  (:import java.awt.datatransfer.StringSelection
-           java.awt.datatransfer.Clipboard
-           java.awt.Toolkit))
+  (:require
+   [clojure.java.shell :as sh]
+   [clojure.java.io :as io])
+  (:import
+   (java.awt.datatransfer Clipboard StringSelection)
+   (java.awt Toolkit)
+   (java.io File)))
 
 (defn touch-file
   [file]
@@ -27,8 +28,11 @@
   "Touches all top level files in the folder."
   [dir]
   (let [d (io/file dir)
-        files (seq (.listFiles d))]
-    (dorun (map #(-> % str touch-file) (remove #(.isDirectory %) files)))))
+        files-or-directories (.listFiles d)
+        files (remove (fn [^File file]
+                        (.isDirectory file))
+                      files-or-directories)]
+    (dorun (map #(-> % str touch-file) files))))
 
 (defn speak
   "Says the specified text outloud."

common-lib/src/cmr/common/nrepl.clj

@@ -1,9 +1,10 @@
 (ns cmr.common.nrepl
   "Common nREPL component for CMR apps."
-  (:require [clojure.tools.nrepl.server :as nrepl]
-            [cmr.common.config :as config]
-            [cmr.common.lifecycle :as lifecycle]
-            [cmr.common.log :refer [info]]))
+  (:require
+   [clojure.tools.nrepl.server :as nrepl]
+   [cmr.common.config :as config]
+   [cmr.common.lifecycle :as lifecycle]
+   [cmr.common.log :refer [info]]))
 
 (defrecord nREPLComponent [port server]
   lifecycle/Lifecycle

common-lib/src/cmr/common/util.clj

@@ -288,12 +288,12 @@
   [x]
   (cond
     (map? x)
-      (let [clean-map (remove-nil-keys
-                        (zipmap (keys x) (map remove-empty-maps (vals x))))]
-        (when (seq clean-map)
-          clean-map))
+    (let [clean-map (remove-nil-keys
+                      (zipmap (keys x) (map remove-empty-maps (vals x))))]
+      (when (seq clean-map)
+        clean-map))
     (sequential? x)
-      (keep remove-empty-maps x)
+    (keep remove-empty-maps x)
     :else
       x))
 
@@ -885,13 +885,13 @@
   run before the mapping takes place. This is useful for tests when you want to
   perform assertion checks upon the raw data, before transformation."
   ([data]
-    (cond
-      (sequential? data) (map map-keys->kebab-case data)
-      (map? data) (map-keys->kebab-case data)
-      :else data))
+   (cond
+     (sequential? data) (map map-keys->kebab-case data)
+     (map? data) (map-keys->kebab-case data)
+     :else data))
   ([data fun]
-    (fun data)
-    (kebab-case-data data)))
+   (fun data)
+   (kebab-case-data data)))
 
 (defn show-methods
   "Display a Java object's public methods."
@@ -938,7 +938,7 @@
 (defn get-index-or-nil
  "Get the index of the substring in the string. Return nil if the substring does not
  exist in the string"
- [s substring]
+ [^String s ^String substring]
  (when s
   (let [index (.indexOf s substring)]
    (when (>= index 0)

common-lib/src/cmr/common/xml.clj

@@ -95,7 +95,7 @@
   [xml-struct path]
   (map str (apply concat (contents-at-path xml-struct path))))
 
-(defn string-at-path
+(defn ^String string-at-path
   "Extracts a string from the given path in the XML structure."
   [xml-struct path]
   (first (strings-at-path xml-struct path)))