From c3a4db32771a24733625ad18d7571d808ac3cd75 Mon Sep 17 00:00:00 2001
From: James Carnegie <me@kipz.org>
Date: Mon, 18 Nov 2019 15:32:19 +0000
Subject: [PATCH] Don't set throughput on new indexes on pay-per-request tables

---
 src/taoensso/faraday.clj                 | 80 ++++++++++++------------
 test/taoensso/faraday/tests/requests.clj | 24 ++++++-
 2 files changed, 63 insertions(+), 41 deletions(-)

diff --git a/src/taoensso/faraday.clj b/src/taoensso/faraday.clj
index eccd0df..987523c 100644
--- a/src/taoensso/faraday.clj
+++ b/src/taoensso/faraday.clj
@@ -743,49 +743,49 @@
 ;;;; Table updates
 
 (defn- global-2nd-index-updates
-  "Implementation detail.
-  {:operation _ :name _ :hash-keydef _ :range-keydef _ :projection _ :throughput _}
-  index -> GlobalSecondaryIndexUpdate}"
-  [{index-name :name
-    :keys [hash-keydef range-keydef throughput projection operation]
-    :or   {projection :all}
-    :as   index}]
-
-  (case operation
-    :create
-    (do
-      (assert (and index-name hash-keydef projection throughput)
-              (str "Malformed global secondary index (GSI): " index))
+  "Validate new GSI configuration and create a GlobalSecondaryIndexUpdate"
+  [{:keys [gsindexes billing-mode] :as params}]
+  (let [{index-name :name
+         :keys [hash-keydef range-keydef throughput projection operation]
+         :or {projection :all}
+         :as index} gsindexes]
+    (case operation
+      :create
+      (do
+        (assert (and index-name hash-keydef projection (or (and throughput
+                                                                (not= :pay-per-request billing-mode))
+                                                           (and (not throughput)
+                                                                (= :pay-per-request billing-mode))))
+                (str "Malformed global secondary index (GSI): " index))
+        (doto (GlobalSecondaryIndexUpdate.)
+          (.setCreate
+           (doto-cond [_ (CreateGlobalSecondaryIndexAction.)]
+                      :always (.setIndexName (name index-name))
+                      :always (.setKeySchema (key-schema hash-keydef range-keydef))
+                      :always (.setProjection
+                               (let [pr (Projection.)
+                                     ptype (if (coll?* projection) :include projection)]
+                                 (.setProjectionType pr (utils/enum ptype))
+                                 (when (= ptype :include)
+                                   (.setNonKeyAttributes pr (mapv name projection)))
+                                 pr))
+                      throughput (.setProvisionedThroughput (provisioned-throughput throughput))))))
+
+      :update
       (doto (GlobalSecondaryIndexUpdate.)
-        (.setCreate
-          (doto
-            (CreateGlobalSecondaryIndexAction.)
-            (.setIndexName (name index-name))
-            (.setKeySchema (key-schema hash-keydef range-keydef))
-            (.setProjection
-              (let [pr    (Projection.)
-                    ptype (if (coll?* projection) :include projection)]
-                (.setProjectionType pr (utils/enum ptype))
-                (when (= ptype :include)
-                  (.setNonKeyAttributes pr (mapv name projection)))
-                pr))
-            (.setProvisionedThroughput (provisioned-throughput throughput))))))
-
-    :update
-    (doto (GlobalSecondaryIndexUpdate.)
-      (.setUpdate
-        (doto
+        (.setUpdate
+         (doto
           (UpdateGlobalSecondaryIndexAction.)
-          (.setIndexName (name index-name))
-          (.setProvisionedThroughput (provisioned-throughput throughput)))))
+           (.setIndexName (name index-name))
+           (.setProvisionedThroughput (provisioned-throughput throughput)))))
 
-    :delete
-    (doto (GlobalSecondaryIndexUpdate.)
-      (.setDelete
-        (doto
+      :delete
+      (doto (GlobalSecondaryIndexUpdate.)
+        (.setDelete
+         (doto
           (DeleteGlobalSecondaryIndexAction.)
-          (.setIndexName (name index-name)))))
-    nil))
+           (.setIndexName (name index-name)))))
+      nil)))
 
 (defn- update-table-request "Implementation detail."
   [table {:keys [throughput gsindexes stream-spec billing-mode] :as params}]
@@ -797,7 +797,7 @@
       :always (.setTableName (name table))
       throughput (.setProvisionedThroughput (provisioned-throughput throughput))
       billing-mode (.setBillingMode (utils/enum billing-mode))
-      gsindexes (.setGlobalSecondaryIndexUpdates [(global-2nd-index-updates gsindexes)])
+      gsindexes (.setGlobalSecondaryIndexUpdates [(global-2nd-index-updates params)])
       stream-spec (.setStreamSpecification (stream-specification stream-spec))
       (seq attr-defs) (.setAttributeDefinitions attr-defs))))
 
diff --git a/test/taoensso/faraday/tests/requests.clj b/test/taoensso/faraday/tests/requests.clj
index b51886c..d10238a 100644
--- a/test/taoensso/faraday/tests/requests.clj
+++ b/test/taoensso/faraday/tests/requests.clj
@@ -247,7 +247,29 @@
                  (update-table-request
                   :update-table
                   {:throughput {:read 4 :write 2}
-                   :billing-mode :pay-per-request})))))
+                   :billing-mode :pay-per-request}))))
+
+  (testing "If parent table is pay-per-request, then can't specify throughput when adding a GSI"
+    (is (thrown? AssertionError
+                 (update-table-request
+                  :update-table
+                  {:billing-mode :pay-per-request
+                   :gsindexes {:name "new-global-secondary"
+                               :operation :create
+                               :hash-keydef [:id :s]
+                               :projection :keys-only
+                               :throughput {:read 10 :write 9}}})))  )
+
+  (testing "If parent table is pay-per-request, then no need to specify billing mode on new GSIs"
+    (let [req ^UpdateTableRequest
+              (update-table-request
+               :update-table
+               {:billing-mode :pay-per-request
+                :gsindexes {:name "new-global-secondary"
+                            :operation :create
+                            :hash-keydef [:id :s]
+                            :projection :keys-only}})]
+      (is (= "update-table" (.getTableName req))))))
 
 (deftest get-item-request-creation
   (is (= "get-item"