Implement fallback for builds without stored pipeline-structure, displaying warning in UI #131, #6

Author: flosell

src/clj/lambdacd/internal/default_pipeline_state_persistence.clj

@@ -98,12 +98,14 @@
     (spit f (pr-str pipeline-structure))))
 
 (defn- read-pipeline-structure-edn [f]
-  {(build-number-from-path f) (edn/read-string (slurp f))})
+  (let [build-number (build-number-from-path f)]
+    (if (file-exists? f)
+      {build-number (edn/read-string (slurp f))}
+      {build-number :fallback})))
 
 (defn read-pipeline-structures [home-dir]
   (->> (build-dirs home-dir)
        (map pipeline-structure-path)
-       (filter file-exists?)
        (map read-pipeline-structure-edn)
        (into {})))
 

src/clj/lambdacd/state/core.clj

@@ -64,10 +64,29 @@
   (get (get-step-results ctx build-number)
        step-id))
 
+(defn- annotated-step [step]
+  (let [annotate-children (fn [x]
+                            (if (:children x)
+                              (assoc x :children (map annotated-step (:children x)))
+                              x))]
+    (-> step
+        (assoc :pipeline-structure-fallback true)
+        (annotate-children))))
+
+(defn- annotated-fallback-structure [ctx]
+  (let [current-structure (pipeline-structure/pipeline-display-representation (:pipeline-def ctx))]
+    (map annotated-step current-structure)))
+
+(defn- stored-structure-or-fallback [ctx build-number]
+  (let [stored-structure (protocols/get-pipeline-structure (state-component ctx) build-number)]
+    (if (= :fallback stored-structure)
+      (annotated-fallback-structure ctx)
+      stored-structure)))
+
 (defn get-pipeline-structure
   "Returns a map describing the structure of the pipeline"
   [ctx build-number]
   (let [component (state-component ctx)]
     (if (satisfies? protocols/PipelineStructureSource component)
-      (protocols/get-pipeline-structure component build-number)
-      (pipeline-structure/pipeline-display-representation (:pipeline-def ctx)))))
+      (stored-structure-or-fallback ctx build-number)
+      (annotated-fallback-structure ctx))))

src/cljs/lambdacd/pipeline.cljs

@@ -189,13 +189,19 @@
        [:li [:a {:class (classes "pipeline__controls__control" (control-active-if @expand-active?)) :on-click #(re-frame/dispatch [::db/toggle-expand-active])} "Expand active"]]
        [:li [:a {:class (classes "pipeline__controls__control" (control-active-if @expand-failures?)) :on-click #(re-frame/dispatch [::db/toggle-expand-failures])} "Expand failures"]]])))
 
+(defn old-pipeline-structure-warning [build-steps]
+  (if (:pipeline-structure-fallback (first build-steps))
+    [:ul {:class "pipeline__controls"}
+     [:li {:class "pipeline__controls__control warning"} [:i {:class "fa fa-exclamation warning-icon"}] "This is an old build with no stored pipeline structure. Using current structure as a best guess. Display and retriggering might not work as expected."]]))
+
 (defn pipeline-component []
   (let [build-state-atom (re-frame/subscribe [::db/pipeline-state])
         build-number-subscription (re-frame/subscribe [::db/build-number])]
     (fn []
       [:div {:class "pipeline" :key "build-pipeline"}
        [pipeline-controls]
+       [old-pipeline-structure-warning @build-state-atom]
        [:ol {:class "pipeline__step-container pipeline__step-container--sequential"}
         (doall
           (for [step @build-state-atom]
-            ^{:key (:step-id step)} [build-step step @build-number-subscription]))]])))
+            ^{:key (:step-id step)} [build-step step @build-number-subscription]))]])))

src/less/app.less

@@ -47,4 +47,14 @@
   display: flex;
   justify-content: center;
   align-items: center;
-}
+}
+
+.warning-icon {
+  padding-top: 5px;
+  padding-bottom: 5px;
+  padding-right: 5px;
+}
+
+.warning {
+  color: @base-orange;
+}

test/clj/lambdacd/internal/default_pipeline_state_persistence_test.clj

@@ -74,10 +74,10 @@
   (testing "that it will return an empty data if no state has been written yet"
     (let [home-dir (utils/create-temp-dir)]
       (is (= {} (read-pipeline-structures home-dir)))))
-  (testing "that it ignores build directories with no pipeline structure (e.g. because they were created before this feature was available)"
+  (testing "that it adds a fallback-marker for build directories with no pipeline structure (e.g. because they were created before this feature was available)"
     (let [home-dir (utils/create-temp-dir)]
       (.mkdirs (io/file home-dir "build-1"))
-      (is (= {} (read-pipeline-structures home-dir))))))
+      (is (= {1 :fallback} (read-pipeline-structures home-dir))))))
 
 (defn- roundtrip-date-time [data]
   (dates->clj-times

test/clj/lambdacd/state/core_test.clj

@@ -10,7 +10,22 @@
 (def some-step-id [0])
 (def some-step-result {:foo :bat})
 (def some-structure {:some :structure})
-(def some-pipeline-def `(foo))
+
+(defn ^{:display-type :container} foo [& _])
+(defn bar [& _])
+(def some-pipeline-def `((foo
+                          bar)))
+
+(def some-pipeline-def-structure [{:name             "foo"
+                                   :type             :container
+                                   :has-dependencies false
+                                   :pipeline-structure-fallback true
+                                   :step-id          `(1)
+                                   :children [{:name             "bar"
+                                               :type             :step
+                                               :has-dependencies false
+                                               :pipeline-structure-fallback true
+                                               :step-id          `(1 1)}]}])
 
 (deftest consume-step-result-update-test
   (testing "that calls to a StepResultUpdateConsumer will just pass through"
@@ -85,8 +100,9 @@
       (is (received? component state-protocols/get-pipeline-structure [1]))))
   (testing "that we get the current pipeline structure if the component doesn't support PipelineStructures"
     (let [component (mock state-protocols/QueryStepResultsSource {:get-step-results {:some :step-results}})]
-      (is (= [{:name             "foo"
-               :type             :unknown
-               :has-dependencies false
-               :step-id          `(1)}] (s/get-pipeline-structure (some-ctx-with :pipeline-state-component component
-                                                                                 :pipeline-def some-pipeline-def) 1))))))
+      (is (= some-pipeline-def-structure (s/get-pipeline-structure (some-ctx-with :pipeline-state-component component
+                                                                                 :pipeline-def some-pipeline-def) 1)))))
+  (testing "that we get the current pipeline structure if the component returns :fallback and annotate the structure accordingly"
+    (let [component (mock state-protocols/PipelineStructureSource {:get-pipeline-structure :fallback})]
+      (is (= some-pipeline-def-structure (s/get-pipeline-structure (some-ctx-with :pipeline-state-component component
+                                                                                            :pipeline-def some-pipeline-def) 1))))))