[search] Rework search to always use db jar data

Search was originally pom-driven, then grew to support indexing either
pom data or db jar data. We only used the pom data when indexing on
upload, and ended up with a different :at timestamp value, depending on
the source of the data.

This simplifies the indexing to only support db jar data as input, which
fixes that discrepancy.

Fixes #897.

Author: tobias

src/clojars/routes/repo.clj

@@ -22,7 +22,6 @@
     File
     IOException)
    (java.util
-    Date
     UUID)
    org.apache.commons.io.FileUtils))
 
@@ -388,7 +387,7 @@
                                     (str "invalid gradle module file: " (.getMessage e))
                                     {:file module-file})))
 
-          {:keys [group-path name version] :as posted-metadata}
+          {:keys [group group-path name version] :as posted-metadata}
           (read-metadata dir)
 
           md-file (io/file dir group-path name "maven-metadata.xml")]
@@ -422,8 +421,7 @@
         (log/audit db {:tag :deployed})
         (log/info {:tag :deploy-finalized})
         (future
-          (search/index! search (assoc pom
-                                       :at (Date. (.lastModified pom-file))))
+          (search/index! search (db/find-jar db group name version))
           (log/info {:tag :deploy-indexed}))
         (spit (io/file dir ".finalized") "")
         (emit-deploy-events db event-emitter (assoc posted-metadata :deployer-username account))))

src/clojars/search.clj

@@ -5,7 +5,6 @@
    [clojars.db :as db]
    [clojars.maven :as maven]
    [clojars.stats :as stats]
-   [clojure.set :as set]
    [clojure.string :as str]
    [com.stuartsierra.component :as component])
   (:import
@@ -55,27 +54,22 @@
 (set! *warn-on-reflection* true)
 
 (defprotocol Search
-  (index! [t pom])
+  (index! [t version-details])
   (search [t query page])
   (delete!
     [t group-id]
     [t group-id artifact-id]))
 
-(def ^:private renames
-  {:name       :artifact-id
-   :jar_name   :artifact-id
-   :group      :group-id
-   :group_name :group-id
-   :created    :at
-   :homepage   :url})
-
-(defn- doc-id ^String [group-id artifact-id]
+(defn- doc-id ^String
+  [group-id artifact-id]
   (str group-id ":" artifact-id))
 
-(defn- jar->id ^String [{:keys [artifact-id group-id]}]
-  (doc-id group-id artifact-id))
+(defn- jar->id ^String
+  [{:keys [group_name jar_name]}]
+  (doc-id group_name jar_name))
 
-(defn delete-from-index [^IndexWriter index-writer ^String group-id & [artifact-id]]
+(defn delete-from-index
+  [^IndexWriter index-writer ^String group-id & [artifact-id]]
   (let [term (if artifact-id
                (Term. "id" (doc-id group-id artifact-id))
                (Term. "group-id" group-id))]
@@ -152,22 +146,23 @@
       (str/replace v #"\.(\s|$)" " "))))
 
 (def ^:private content-items
-  [:artifact-id
-   (hyphen-remover :artifact-id)
-   :group-id
-   (hyphen-remover :group-id)
+  [:jar_name
+   (hyphen-remover :jar_name)
+   :group_name
+   (hyphen-remover :group_name)
    ;; Include 'group name' & 'group name/artifact-name' in content (for a
-   ;; group-id of group.name) to aid in searching for things where new projects
+   ;; group_name of group.name) to aid in searching for things where new projects
    ;; had to be deployed under a domain-based group
-   (period-remover :group-id)
-   (period-remover #(->> % ((juxt :group-id :artifact-id)) (str/join "/")))
+   (period-remover :group_name)
+   (period-remover #(->> % ((juxt :group_name :jar_name)) (str/join "/")))
    ;; Include 'group-name/artifact-name' in content to allow
    ;; the "group-name/artifact-name" phrase to find it
-   #(->> % ((juxt :group-id :artifact-id)) (str/join "/"))
+   #(->> % ((juxt :group_name :jar_name)) (str/join "/"))
    (sentence-period-remover :description)
-   :url
+   :homepage
    :version
-   #(->> % :authors (str/join " "))])
+   #(when-some [authors (:authors %)]
+      (str/replace authors #"," " "))])
 
 (def ^:private ^String content-field-name "_content")
 (def ^:private ^String boost-field-name "_download_boost")
@@ -182,28 +177,20 @@
   [^String name ^String value]
   (TextField. name value Field$Store/YES))
 
-(defn jar->doc
-  ^Iterable [{:keys [at
-                     artifact-id
-                     group-id
-                     description
-                     licenses
-                     url
-                     version]
-              :or   {at (Date.)}
-              :as   jar}
-             download-boost]
+(defn- jar->doc ^Iterable
+  [{:keys [created description group_name homepage jar_name licenses version] :as jar}
+   download-boost]
   (doto (Document.)
     ;; id: We need a unique identifier for each doc so that we can use updateDocument
     (.add (string-field "id" (jar->id jar)))
-    (.add (string-field "artifact-id" artifact-id))
-    (.add (string-field "group-id" group-id))
+    (.add (string-field "artifact-id" jar_name))
+    (.add (string-field "group-id" group_name))
     (cond-> description
       (.add (text-field "description" description)))
-    (.add (string-field "at" (str (.getTime ^Date at))))
+    (.add (string-field "at" (str (.getTime ^Date created))))
     (.add (text-field "licenses" (str/join " " (map :name licenses))))
-    (cond-> url
-      (.add (string-field "url" url)))
+    (cond-> homepage
+      (.add (string-field "url" homepage)))
     ;; version isn't really useful to search, since we only store the
     ;; most-recently-seen value, but we have it here because we've had it
     ;; historically
@@ -226,10 +213,10 @@
 (def download-score-weight 50)
 
 (defn- calculate-document-boost
-  [stats {:as _jar :keys [artifact-id group-id]}]
+  [stats {:as _jar :keys [group_name jar_name]}]
   (let [total (stats/total-downloads stats)]
     (* download-score-weight
-       (/ (or (stats/download-count stats group-id artifact-id) 0)
+       (/ (or (stats/download-count stats group_name jar_name) 0)
           (max 1 total)))))
 
 (defn disk-index
@@ -239,11 +226,10 @@
 
 (defn- index-jar
   [^IndexWriter index-writer stats jar]
-  (let [jar' (set/rename-keys jar renames)
-        doc (jar->doc jar' (calculate-document-boost stats jar'))]
+  (let [doc (jar->doc jar (calculate-document-boost stats jar))]
     ;; always delete and replace the doc, since we are indexing every version
     ;; and the last one wins
-    (.updateDocument index-writer (Term. "id" (jar->id jar')) doc)))
+    (.updateDocument index-writer (Term. "id" (jar->id jar)) doc)))
 
 (defn- track-index-status
   [{:keys [indexed last-time] :as status}]
@@ -384,9 +370,9 @@
 
 (defrecord LuceneSearch [stats index-factory ^Directory index]
   Search
-  (index! [_t pom]
+  (index! [_t version-details]
     (with-open [index-writer (index-writer index false)]
-      (index-jar index-writer stats pom)))
+      (index-jar index-writer stats version-details)))
   (search [_t query page]
     (-search index query page))
   (delete! [_t group-id]

src/clojars/web/search.clj

@@ -13,15 +13,13 @@
    [hiccup.element :refer [link-to]]
    [ring.util.codec :refer [url-encode]]))
 
-(defn- jar->json [jar]
-  (let [m {:jar_name (:artifact-id jar)
-           :group_name (:group-id jar)
-           :version (:version jar)
-           :description (:description jar)}
-        created (:at jar)]
-    (if created
-      (assoc m :created created)
-      m)))
+(defn- jar->json
+  [{:keys [artifact-id at description group-id version]}]
+  {:created     at
+   :description description
+   :group_name  group-id
+   :jar_name    artifact-id
+   :version     version})
 
 (defn json-search [search query page]
   (let [response {:status 200
@@ -43,15 +41,14 @@
           :error-message (format "Invalid search syntax for query `%s`" query)}
          (log/trace-id))))))
 
-(defn- jar->xml [jar]
-  (let [attrs {:jar_name (:artifact-id jar)
-               :group_name (:group-id jar)
-               :version (:version jar)
-               :description (some-> (:description jar) xml-escape)}
-        created (:at jar)]
-    {:tag :result :attrs (if created
-                           (assoc attrs :created created)
-                           attrs)}))
+(defn- jar->xml
+  [{:keys [artifact-id at description group-id version]}]
+  {:tag   :result
+   :attrs {:created     at
+           :description (some-> description xml-escape)
+           :group_name  group-id
+           :jar_name    artifact-id
+           :version     version}})
 
 (defn xml-search [search query page]
   (let [response {:status 200

test/clojars/integration/search_test.clj

@@ -4,10 +4,12 @@
    [clj-http.client :as client]
    [clojars.search :as search]
    [clojars.test-helper :as help]
-   [clojure.set :as set]
    [clojure.test :refer [deftest is testing use-fixtures]]
    [clojure.xml :as xml]
-   [matcher-combinators.test]))
+   [matcher-combinators.test])
+  (:import
+   (java.util
+    Date)))
 
 (use-fixtures :each
   help/default-fixture
@@ -28,15 +30,21 @@
               page)
       (client/get opts)))
 
+(defn created-as-str
+  [data]
+  (update data :created #(str (.getTime %))))
+
 (deftest search-test
   (let [base-data {:description "foo" :version "1.0"}
         search-data (map (partial merge base-data)
-                         [{:jar_name "test" :group_name "test"}
-                          {:jar_name "test" :group_name "testing"}])]
+                         [{:created (Date.)
+                           :group_name "test"
+                           :jar_name "test"}
+                          {:created (Date.)
+                           :group_name "testing"
+                           :jar_name "test"}])]
     (doseq [data search-data]
-      (search/index! (:search help/system)
-                     (set/rename-keys data {:jar_name :artifact-id
-                                            :group_name :group-id})))
+      (search/index! (:search help/system) data))
 
     (testing "json request returns json"
       (let [resp (do-search :json "test")]
@@ -81,7 +89,7 @@
             result (json/parse-string (:body resp) true)]
         (is (= 200 (:status resp)))
         (is (= 2 (:count result)))
-        (is (match? search-data (:results result)))))
+        (is (match? (mapv created-as-str search-data) (:results result)))))
 
     (testing "json request with invalid page returns error"
       (let [resp (do-search-with-page :json "test" "a" {:throw-exceptions false})

test/clojars/test_helper.clj

@@ -163,15 +163,6 @@
                   m))
     new-pom))
 
-(defn at-as-time-str
-  "Adjusts the :at (or :created) Date to a millis-since-epoch string to
-  match the search results."
-  [data]
-  (let [date->time-str #(str (.getTime %))]
-    (cond-> data
-      (:at data)      (update :at date->time-str)
-      (:created data) (update :created date->time-str))))
-
 (defn add-verified-group
   [account group]
   (db/add-group *db* account group)