From 9e2fe8448c81abc3e7bff146ac8e85314644e475 Mon Sep 17 00:00:00 2001 From: Toby Crawley Date: Fri, 10 Jun 2016 11:55:55 -0400 Subject: [PATCH] Don't ignore files uploaded after maven-metadata.xml [#515 #532] Maven will upload *any* classified artifact after uploading maven-metadata.xml. We use maven-metadata.xml as a trigger to know when to finalize the deploy, moving the contents to the actual repo and to cloudfiles. Before this change, we essentially ignored any files that came in after the finalization. Now, any files that come in after finalization go straight to the actual repo and to cloudfiles. This means that deployments with classified artifacts is no longer atomic - the primary artifact, pom, and maven-metadata.xml will be consistent, but any classified artifacts won't be part of that atomic unit. This commit also switches us from clj-http-lite to clj-http for testing, since we need cookie support to properly test this change, since we need to control the order of deployment, so can't use aether. --- dev-resources/test-0.0.1/maven-metadata.xml | 11 ++ project.clj | 2 +- src/clojars/routes/repo.clj | 128 +++++++++++--------- test/clojars/test/integration/api.clj | 2 +- test/clojars/test/integration/artifact.clj | 2 +- test/clojars/test/integration/search.clj | 2 +- test/clojars/test/integration/uploads.clj | 102 ++++++++++++---- test/clojars/test/test_helper.clj | 7 +- 8 files changed, 165 insertions(+), 91 deletions(-) create mode 100644 dev-resources/test-0.0.1/maven-metadata.xml diff --git a/dev-resources/test-0.0.1/maven-metadata.xml b/dev-resources/test-0.0.1/maven-metadata.xml new file mode 100644 index 00000000..97b7bd67 --- /dev/null +++ b/dev-resources/test-0.0.1/maven-metadata.xml @@ -0,0 +1,11 @@ + + fake +test + + 0.0.1 + + 0.0.1 + + 20160406222819 + + diff --git a/project.clj b/project.clj index 1b94c027..bbc9db68 100644 --- a/project.clj +++ b/project.clj @@ -64,7 +64,7 @@ [eftest "0.1.0"] [kerodon "0.7.0" :exclusions [org.apache.httpcomponents/httpcore]] - [clj-http-lite "0.3.0"] + [clj-http "2.2.0"] [com.google.jimfs/jimfs "1.0"] [net.polyc0l0r/bote "0.1.0" :exclusions [org.clojars.kjw/slf4j-simple]]] diff --git a/src/clojars/routes/repo.clj b/src/clojars/routes/repo.clj index cb6452f7..114b9fc9 100644 --- a/src/clojars/routes/repo.clj +++ b/src/clojars/routes/repo.clj @@ -27,17 +27,18 @@ (sort-by (comp - (memfn last-modified))) (map (memfn getName)))) -(defn save-to-file [sent-file input] - (-> sent-file +(defn save-to-file [dest input] + (-> dest .getParentFile .mkdirs) - (io/copy input sent-file)) + (io/copy input dest) + dest) -(defn- try-save-to-file [sent-file input] +(defn- try-save-to-file [dest input] (try - (save-to-file sent-file input) + (save-to-file dest input) (catch IOException e - (.delete sent-file) + (.delete dest) (throw e)))) (defn- pom? [file] @@ -200,59 +201,64 @@ (ex-data e)) (.getCause e)))))) +(defn upload-to-cloudfiles [cloudfiles reporter from-dir file] + (let [path (cf/remote-path (.getAbsolutePath from-dir) (.getAbsolutePath file))] + (try + (cf/put-file cloudfiles path file) + (catch Exception e + ;; catch and report anything that fails for now + ;; instead of letting it bubble up, since cloudfiles + ;; isn't yet the primary repo + (report-error reporter e {:path path :file file}))))) + (defn finalize-deploy [cloudfiles db reporter search account repo ^File dir] - (try - (if-let [pom-file (find-pom dir)] - (let [pom (try - (maven/pom-to-map pom-file) - (catch Exception e - (throw-invalid (str "invalid pom file: " (.getMessage e)) - {:file pom-file} - e))) - {:keys [group group-path name] :as posted-metadata} (read-string (slurp (io/file dir metadata-edn)))] - - ;; since we trigger on maven-metadata.xml, we don't actually - ;; have the sums for it because they are uploaded *after* the - ;; metadata file itself. This means that it's possible for a - ;; corrupted file to slip through, so we try to parse it - (let [md-file (io/file dir group-path name "maven-metadata.xml")] - (try - (maven/read-metadata md-file) - (catch Exception e - (throw-invalid "Failed to parse maven-metadata.xml" - {:file md-file} - e))) - - ;; If that succeeds, we create checksums for it - (fu/create-checksum-file md-file :md5) - (fu/create-checksum-file md-file :sha1)) - - (validate-deploy dir pom posted-metadata) - (db/check-and-add-group db account group) - (FileUtils/copyDirectory dir (io/file repo) - (reify FileFilter - (accept [_ f] - (not= metadata-edn (.getName f))))) - (run! - (fn [f] - (let [path (cf/remote-path (.getAbsolutePath dir) (.getAbsolutePath f))] - (try - (cf/put-file cloudfiles path f) + (if-let [pom-file (find-pom dir)] + (let [pom (try + (maven/pom-to-map pom-file) (catch Exception e - ;; catch and report anything that fails for now - ;; instead of letting it bubble up, since cloudfiles - ;; isn't yet the primary repo - (report-error reporter e {:path path :file f}))))) - (find-artifacts dir false)) - - (db/add-jar db account pom) - (search/index! search (assoc pom - :at (.lastModified pom-file)))) - (throw-invalid "no pom file was uploaded")) - (finally - (FileUtils/deleteQuietly dir)))) - -(defn- handle-versioned-upload [db body session group artifact version filename] + (throw-invalid (str "invalid pom file: " (.getMessage e)) + {:file pom-file} + e))) + {:keys [group group-path name] :as posted-metadata} (read-string (slurp (io/file dir metadata-edn)))] + + ;; since we trigger on maven-metadata.xml, we don't actually + ;; have the sums for it because they are uploaded *after* the + ;; metadata file itself. This means that it's possible for a + ;; corrupted file to slip through, so we try to parse it + (let [md-file (io/file dir group-path name "maven-metadata.xml")] + (try + (maven/read-metadata md-file) + (catch Exception e + (throw-invalid "Failed to parse maven-metadata.xml" + {:file md-file} + e))) + + ;; If that succeeds, we create checksums for it + (fu/create-checksum-file md-file :md5) + (fu/create-checksum-file md-file :sha1)) + + (validate-deploy dir pom posted-metadata) + (db/check-and-add-group db account group) + (FileUtils/copyDirectory dir (io/file repo) + (reify FileFilter + (accept [_ f] + (not= metadata-edn (.getName f))))) + (run! (partial upload-to-cloudfiles cloudfiles reporter dir) (find-artifacts dir false)) + + (db/add-jar db account pom) + (search/index! search (assoc pom + :at (.lastModified pom-file))) + (spit (io/file dir ".finalized") "")) + (throw-invalid "no pom file was uploaded"))) + +(defn- deploy-finalized? [dir] + (.exists (io/file dir ".finalized"))) + +(defn- deploy-post-finalized-file [cloudfiles reporter repo tmp-repo file] + (io/copy file (io/file repo (cf/remote-path (.getAbsolutePath tmp-repo) (.getAbsolutePath file)))) + (upload-to-cloudfiles cloudfiles reporter tmp-repo file)) + +(defn- handle-versioned-upload [cloudfiles db reporter repo body session group artifact version filename] (let [groupname (string/replace group "/" ".")] (upload-request db @@ -264,7 +270,9 @@ :group-path group :name artifact :version version})) - (try-save-to-file (io/file upload-dir group artifact version filename) body))))) + (let [file (try-save-to-file (io/file upload-dir group artifact version filename) body)] + (when (deploy-finalized? upload-dir) + (deploy-post-finalized-file cloudfiles reporter repo upload-dir file))))))) ;; web handlers (defn routes [cloudfiles db reporter search] @@ -280,7 +288,7 @@ group-parts (string/split group #"/") group (string/join "/" (butlast group-parts)) artifact (last group-parts)] - (handle-versioned-upload db body session group artifact version file)) + (handle-versioned-upload cloudfiles db reporter (config :repo) body session group artifact version file)) (if (re-find #"maven-metadata\.xml$" file) ;; ignore metadata sums, since we'll recreate those when ;; the deploy is finalizied @@ -301,7 +309,7 @@ :group #"[^\.]+" :artifact #"[^/]+" :version #"[^/]+" :filename #"[^/]+(\.pom|\.jar|\.sha1|\.md5|\.asc)$"] {body :body session :session {:keys [group artifact version filename]} :params} - (handle-versioned-upload db body session group artifact version filename)) + (handle-versioned-upload cloudfiles db reporter (config :repo) body session group artifact version filename)) (PUT "*" _ {:status 400 :headers {}}) (not-found "Page not found"))) diff --git a/test/clojars/test/integration/api.clj b/test/clojars/test/integration/api.clj index 7fd47d0d..a04a8f23 100644 --- a/test/clojars/test/integration/api.clj +++ b/test/clojars/test/integration/api.clj @@ -1,5 +1,5 @@ (ns clojars.test.integration.api - (:require [clj-http.lite.client :as client] + (:require [clj-http.client :as client] [clojars.test.integration.steps :refer [register-as inject-artifacts-into-repo!]] [clojars.test.test-helper :as help] [clojars.web :as web] diff --git a/test/clojars/test/integration/artifact.clj b/test/clojars/test/integration/artifact.clj index 4539b19c..10181665 100644 --- a/test/clojars/test/integration/artifact.clj +++ b/test/clojars/test/integration/artifact.clj @@ -2,7 +2,7 @@ (:require [clojure.test :refer :all] [clojars.test.test-helper :as help] [clojure.string :as str] - [clj-http.lite.client :as client] + [clj-http.client :as client] [kerodon.core :refer [session]] [clojars.test.integration.steps :refer [register-as inject-artifacts-into-repo!]] [cheshire.core :as json])) diff --git a/test/clojars/test/integration/search.clj b/test/clojars/test/integration/search.clj index e1b3aeb8..83dd70c2 100644 --- a/test/clojars/test/integration/search.clj +++ b/test/clojars/test/integration/search.clj @@ -1,6 +1,6 @@ (ns clojars.test.integration.search (:require [cheshire.core :as json] - [clj-http.lite.client :as client] + [clj-http.client :as client] [clojars.search :as search] [clojars.test.test-helper :as help] [clojure.set :as set] diff --git a/test/clojars/test/integration/uploads.clj b/test/clojars/test/integration/uploads.clj index 41f98044..d92736f6 100644 --- a/test/clojars/test/integration/uploads.clj +++ b/test/clojars/test/integration/uploads.clj @@ -3,12 +3,14 @@ [clojars [cloudfiles :as cf] [config :refer [config]] + [file-utils :as fu] [web :as web :refer [clojars-app]]] [clojars.test.integration.steps :refer :all] [clojars.test.test-helper :as help] [clojure.data.codec.base64 :as base64] [clojure.java.io :as io] [clojure.test :refer :all] + [clj-http.client :as client] [kerodon [core :refer :all] [test :refer :all]] @@ -18,6 +20,19 @@ help/default-fixture help/run-test-app) +(defn repo-url [] + (str "http://localhost:" help/test-port "/repo")) + +(defn tmp-file [f name] + (let [tmp-f (doto (io/file help/tmp-dir name) + (.deleteOnExit))] + (io/copy f tmp-f) + tmp-f)) + +(defn tmp-checksum-file [f type] + (doto (fu/create-checksum-file f type) + .deleteOnExit)) + (deftest user-can-register-and-deploy (-> (session (help/app-from-system)) (register-as "dantheman" "test@example.org" "password")) @@ -28,7 +43,7 @@ :jar-file (io/file (io/resource "test.jar")) :pom-file (help/rewrite-pom (io/file (io/resource "test-0.0.1/test.pom")) {:groupId "org.clojars.dantheman"}) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo) @@ -48,7 +63,7 @@ (aether/resolve-dependencies :coordinates '[[org.clojars.dantheman/test "0.0.1"]] :repositories {"test" {:url - (str "http://localhost:" help/test-port "/repo")}} + (repo-url)}} :local-repo help/local-repo2))) (-> (session (help/app-from-system)) (visit "/groups/org.clojars.dantheman") @@ -69,6 +84,47 @@ (within [:div.result] (has (text? "org.clojars.dantheman/test 0.0.1"))))) +(deftest user-can-deploy-artifacts-after-maven-metadata + (-> (session (help/app-from-system)) + (register-as "dantheman" "test@example.org" "password")) + (help/delete-file-recursively help/local-repo) + (help/delete-file-recursively help/local-repo2) + (let [add-checksums (partial mapcat (fn [[f no-version?]] + [[f no-version?] + [(tmp-checksum-file f :md5) no-version?] + [(tmp-checksum-file f :sha1) no-version?]])) + files (add-checksums [[(tmp-file + (io/file (io/resource "test.jar")) "test-0.0.1.jar")] + [(tmp-file + (help/rewrite-pom (io/file (io/resource "test-0.0.1/test.pom")) + {:groupId "org.clojars.dantheman"}) + "test-0.0.1.pom")] + [(tmp-file + (io/file (io/resource "test-0.0.1/maven-metadata.xml")) + "maven-metadata.xml") + :no-version] + [(tmp-file + (io/file (io/resource "test.jar")) "test-sources-0.0.1.jar")]])] + ;; we use clj-http here instead of aether to have control over the + ;; order the files are uploaded + (binding [clj-http.core/*cookie-store* (clj-http.cookies/cookie-store)] + (doseq [[f no-version?] files] + (client/put (format "%s/org/clojars/dantheman/test/%s%s" + (repo-url) + (if no-version? "" "0.0.1/") + (.getName f)) + {:body f + :basic-auth ["dantheman" "password"]}))) + (let [base-path "org/clojars/dantheman/test/" + cloudfiles (:cloudfiles help/system) + repo (:repo config)] + (doseq [[f no-version?] files] + (let [fname (.getName f) + base-path' (if no-version? base-path (str base-path "0.0.1/"))] + (is (.exists (io/file repo base-path' fname))) + (is (cf/artifact-exists? cloudfiles (str base-path' fname)))))))) + + (deftest user-can-deploy-to-new-group (-> (session (help/app-from-system)) (register-as "dantheman" "test@example.org" "password")) @@ -78,7 +134,7 @@ :coordinates '[fake/test "0.0.1"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.1/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo) @@ -86,7 +142,7 @@ (aether/resolve-dependencies :coordinates '[[fake/test "0.0.1"]] :repositories {"test" {:url - (str "http://localhost:" help/test-port "/repo")}} + (repo-url)}} :local-repo help/local-repo2))) (-> (session (help/app-from-system)) (visit "/groups/fake") @@ -112,7 +168,7 @@ :coordinates '[org.clojars.fixture/test "1.0.0"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.1/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo)))) @@ -124,7 +180,7 @@ :coordinates '[fake/test "0.0.1"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.1/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo) @@ -135,7 +191,7 @@ :coordinates '[fake/test "0.0.1"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.1/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo)))) @@ -147,7 +203,7 @@ :coordinates '[fake/test "0.0.3-SNAPSHOT"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.3-SNAPSHOT/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo) @@ -155,7 +211,7 @@ :coordinates '[fake/test "0.0.3-SNAPSHOT"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.3-SNAPSHOT/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo)) @@ -169,7 +225,7 @@ :pom-file (help/rewrite-pom (io/file (io/resource "test-0.0.3-SNAPSHOT/test.pom")) {:groupId "org.clojars.dantheman" :artifactId "test.thing"}) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo)) @@ -182,7 +238,7 @@ :coordinates '[fake/test "0.0.3-SNAPSHOT"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.3-SNAPSHOT/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo @@ -200,7 +256,7 @@ :artifact-map {[:extension "jar"] (io/file (io/resource "test.jar")) [:classifier "test" :extension "jar"] (io/file (io/resource "test.jar")) [:extension "pom"] (io/file (io/resource "test-0.0.1/test.pom"))} - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo) @@ -216,7 +272,7 @@ :artifact-map {[:extension "jar"] (io/file (io/resource "test.jar")) [:classifier "test" :extension "jar"] (io/file (io/resource "test.jar")) [:extension "pom"] (io/file (io/resource "test-0.0.3-SNAPSHOT/test.pom"))} - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo) @@ -237,7 +293,7 @@ ;; any content will do since we don't validate signatures [:extension "jar.asc"] pom [:extension "pom.asc"] pom} - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo))) @@ -254,7 +310,7 @@ [:extension "pom"] pom ;; any content will do since we don't validate signatures [:extension "jar.asc"] pom} - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo))))) @@ -266,7 +322,7 @@ :coordinates '[fake/test "1.0.0"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.1/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo")}} + :repository {"test" {:url (repo-url)}} :local-repo help/local-repo)))) (deftest bad-login-cannot-deploy @@ -275,7 +331,7 @@ :coordinates '[fake/test "1.0.0"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.1/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "guest" :password "password"}} :local-repo help/local-repo)))) @@ -289,7 +345,7 @@ :coordinates '[flake/test "0.0.1"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.1/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo))) @@ -299,7 +355,7 @@ :coordinates '[fake/toast "0.0.1"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.1/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo))) @@ -309,7 +365,7 @@ :coordinates '[fake/test "1.0.0"] :jar-file (io/file (io/resource "test.jar")) :pom-file (io/file (io/resource "test-0.0.1/test.pom")) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo)))) @@ -324,7 +380,7 @@ :jar-file (io/file (io/resource "test.jar")) :pom-file (help/rewrite-pom (io/file (io/resource "test-0.0.1/test.pom")) {:groupId "faKE"}) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo))) @@ -339,7 +395,7 @@ :jar-file (io/file (io/resource "test.jar")) :pom-file (help/rewrite-pom (io/file (io/resource "test-0.0.1/test.pom")) {:artifactId "teST"}) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo))))) @@ -354,7 +410,7 @@ :jar-file (io/file (io/resource "test.jar")) :pom-file (help/rewrite-pom (io/file (io/resource "test-0.0.1/test.pom")) {:version "1.α.0"}) - :repository {"test" {:url (str "http://localhost:" help/test-port "/repo") + :repository {"test" {:url (repo-url) :username "dantheman" :password "password"}} :local-repo help/local-repo)))) diff --git a/test/clojars/test/test_helper.clj b/test/clojars/test/test_helper.clj index 0a26b1e7..5ba7831e 100644 --- a/test/clojars/test/test_helper.clj +++ b/test/clojars/test/test_helper.clj @@ -18,10 +18,9 @@ [com.stuartsierra.component :as component]) (:import java.io.File)) -(def local-repo (io/file (System/getProperty "java.io.tmpdir") - "clojars" "test" "local-repo")) -(def local-repo2 (io/file (System/getProperty "java.io.tmpdir") - "clojars" "test" "local-repo2")) +(def tmp-dir (io/file (System/getProperty "java.io.tmpdir"))) +(def local-repo (io/file tmp-dir "clojars" "test" "local-repo")) +(def local-repo2 (io/file tmp-dir "clojars" "test" "local-repo2")) (def test-config {:port 0 :bind "127.0.0.1"