diff --git a/src/clj/auto_ap/jobs/rekey_square_external_ids.clj b/src/clj/auto_ap/jobs/rekey_square_external_ids.clj new file mode 100644 index 00000000..dc508c89 --- /dev/null +++ b/src/clj/auto_ap/jobs/rekey_square_external_ids.clj @@ -0,0 +1,106 @@ +(ns auto-ap.jobs.rekey-square-external-ids + "One-shot migration re-keying Square refunds and charges to client-scoped external ids. + + Refund and charge keys carry no client scoping today, so two clients configured on the same + Square location share a single entity: the refund's owner flips every time either client + imports, and one charge ends up referenced by both clients' orders. Sales orders already scope + their keys by client and location; this brings the other two in line. + + Run AFTER the importer knows how to resolve both key schemes (`square.core3/existing-id`). + Running it first would be harmless, but the importer would then re-create legacy-keyed + entities on its next pass. + + The migration is idempotent: an entity already carrying its scoped key is skipped, so it can + be re-run over a partially migrated database." + (:require + [auto-ap.datomic :refer [conn]] + [auto-ap.logging :as alog] + [datomic.api :as dc])) + +(def refund-prefix "square/refund/") +(def charge-prefix "square/charge/") + +(defn- scope-of + "`[client-code location]` for an entity, or nil when it cannot be determined. + + Charges are the awkward case: about an eighth of them carry neither `:charge/client` nor + `:charge/location`. Those are stubs minted by the payout path, which asserts an external id + alone and lets unique-identity upsert bring a bare entity into being, plus older tender + records that predate the client attribute. None are orphaned, so the scope is recovered from + whatever references them — the sales order first, then the expected deposit." + [db attr e] + (let [ent (dc/entity db e) + pair (fn [code loc] (when (and code loc) [code loc]))] + (or (case attr + :sales-refund/external-id (pair (:client/code (:sales-refund/client ent)) + (:sales-refund/location ent)) + :charge/external-id (pair (:client/code (:charge/client ent)) + (:charge/location ent))) + (when-let [o (:e (first (dc/datoms db :vaet e :sales-order/charges)))] + (let [oe (dc/entity db o)] + (pair (:client/code (:sales-order/client oe)) (:sales-order/location oe)))) + (when-let [d (:e (first (dc/datoms db :vaet e :expected-deposit/charges)))] + (let [de (dc/entity db d)] + (pair (:client/code (:expected-deposit/client de)) (:expected-deposit/location de))))))) + +(defn planned-key + "`[eid new-key]` for an entity that still needs re-keying, or nil when it is already scoped or + cannot be scoped at all. + + Detection compares against the key this entity *should* have rather than pattern-matching the + id, because Square ids may themselves contain dashes and no pattern separates the two schemes + reliably. That also makes the migration idempotent." + [db attr prefix datom] + (let [old (:v datom)] + (when-let [[code loc] (scope-of db attr (:e datom))] + (let [scoped-prefix (str prefix code "-" loc "-")] + (when-not (.startsWith ^String old scoped-prefix) + [(:e datom) (str scoped-prefix (subs old (count prefix)))]))))) + +(defn plan + "Everything the migration would change, plus what it cannot touch. Read-only — run this and + check `:collisions` is empty before transacting anything." + [db attr prefix] + (let [acc (reduce (fn [acc d] + (let [acc (update acc :total inc)] + (if-let [[e new-key] (planned-key db attr prefix d)] + (-> acc + (update :to-migrate inc) + (update :new-keys conj! [e new-key])) + (if (scope-of db attr (:e d)) + (update acc :already-scoped inc) + (update acc :unscopable inc))))) + {:total 0 :to-migrate 0 :already-scoped 0 :unscopable 0 :new-keys (transient [])} + (dc/datoms db :aevt attr))] + (update acc :new-keys persistent!))) + +(defn collisions + "Any two entities that would land on the same new key. Must be empty: a collision would merge + two entities into one and lose whichever lost." + [new-keys] + (->> new-keys + (group-by second) + (keep (fn [[k es]] (when (> (count es) 1) [k (mapv first es)]))) + vec)) + +(defn migrate! + "Asserts the new external id on each planned entity. The attribute is cardinality one, so the + legacy value is retracted by the same assertion and the entity keeps its identity — nothing is + created and nothing is deleted. + + Returns the number of entities re-keyed." + [attr new-keys batch-size] + (let [total (count new-keys)] + (alog/info ::migrating :attr attr :count total) + (doseq [[i batch] (map-indexed vector (partition-all batch-size new-keys))] + @(dc/transact conn (for [[e new-key] batch] {:db/id e attr new-key})) + (when (zero? (mod i 20)) + (alog/info ::migrated :attr attr :done (* i batch-size) :of total))) + total)) + +(defn counts + "Entity totals, for the before/after assertion that is this migration's real safety net: if + either number moves, the re-key created duplicates instead of updating in place." + [db] + {:refunds (reduce (fn [n _] (inc n)) 0 (dc/datoms db :aevt :sales-refund/external-id)) + :charges (reduce (fn [n _] (inc n)) 0 (dc/datoms db :aevt :charge/external-id))}) diff --git a/src/clj/auto_ap/square/core3.clj b/src/clj/auto_ap/square/core3.clj index 77ab94e1..43d118a0 100644 --- a/src/clj/auto_ap/square/core3.clj +++ b/src/clj/auto_ap/square/core3.clj @@ -269,6 +269,29 @@ 0.0 [])) +(defn scoped-key + "Client-scoped external id, in the shape sales order keys already use. + + Without the client and location in the key, two clients configured on the same Square location + collide on a single entity: a refund changes owner every time either client imports, and one + charge ends up shared between both clients' orders." + [prefix client location id] + (str prefix (:client/code client) "-" (:square-location/client-location location) "-" id)) + +(defn existing-id + "Entity id of the refund or charge this id already refers to, trying the client-scoped key + first and the legacy unscoped key second. + + This is what makes re-keying safe. These external ids are `:db.unique/identity`, so the import + relies on upsert-by-identity; changing the key format on its own would match nothing and + Datomic would create a SECOND entity for every refund and charge, orphaning the original under + its legacy key. Pinning the result as `:db/id` makes the write land on the existing entity + whichever scheme it currently carries." + [db attr prefix client location id] + (when id + (or (dc/entid db [attr (scoped-key prefix client location id)]) + (dc/entid db [attr (str prefix id)])))) + (defn tender->charge [order client location t] (remove-nils #:charge @@ -278,8 +301,9 @@ :note (:note t) :location (:square-location/client-location location) :reference-link (str (url/url "https://squareup.com/receipt/preview" (:id t))) + :db/id (existing-id (dc/db conn) :charge/external-id "square/charge/" client location (:id t)) :external-id (when (:id t) - (str "square/charge/" (:id t))) + (scoped-key "square/charge/" client location (:id t))) :processor (cond (#{"OTHER" "THIRD_PARTY_CARD"} (:type t)) (condp = (some-> (:note t) str/lower-case) @@ -561,7 +585,11 @@ (coerce/to-date))) :charges (reverse (->> (:payout_entries payout) (filter (comp :payment_id :type_charge_details)) - (map (fn [p] {:charge/external-id (str "square/charge/" (:payment_id (:type_charge_details p)))}))))}) + (map (fn [p] + (let [payment-id (:payment_id (:type_charge_details p))] + (remove-nils + {:charge/external-id (scoped-key "square/charge/" client location payment-id) + :db/id (existing-id (dc/db conn) :charge/external-id "square/charge/" client location payment-id)}))))))}) (filter :expected-deposit/date) (into [])) (catch Throwable e @@ -585,7 +613,8 @@ (de/chain (get-payment client (:payment_id r)) (fn [payment] - #:sales-refund {:external-id (str "square/refund/" (:id r)) + #:sales-refund {:db/id (existing-id (dc/db conn) :sales-refund/external-id "square/refund/" client l (:id r)) + :external-id (scoped-key "square/refund/" client l (:id r)) :vendor :vendor/ccp-square :total (amount->money (:amount_money r)) :fee (transduce diff --git a/test/clj/auto_ap/square/core3_test.clj b/test/clj/auto_ap/square/core3_test.clj new file mode 100644 index 00000000..0f642c91 --- /dev/null +++ b/test/clj/auto_ap/square/core3_test.clj @@ -0,0 +1,77 @@ +(ns auto-ap.square.core3-test + (:require + [auto-ap.datomic :refer [conn]] + [auto-ap.integration.util :refer [setup-test-data wrap-setup]] + [auto-ap.square.core3 :as sut] + [clojure.test :refer [deftest is testing use-fixtures]] + [datomic.api :as dc])) + +(use-fixtures :each wrap-setup) + +(def client {:client/code "NGCD"}) +(def location {:square-location/client-location "CD"}) + +(defn- refund-count [] + (count (dc/q '[:find ?e :where [?e :sales-refund/external-id]] (dc/db conn)))) + +(defn- resolve-refund [id] + (sut/existing-id (dc/db conn) :sales-refund/external-id "square/refund/" client location id)) + +(deftest scoped-key-carries-client-and-location + (testing "the same shape sales order keys already use, so a shared location cannot contend" + (is (= "square/refund/NGCD-CD-abc" (sut/scoped-key "square/refund/" client location "abc"))) + (is (= "square/charge/NGCD-CD-xyz" (sut/scoped-key "square/charge/" client location "xyz"))))) + +(deftest legacy-keyed-entity-is-updated-not-duplicated + (testing "an entity still carrying its unscoped key is found and re-keyed in place. + + This is the sharpest hazard in the migration: these external ids are + :db.unique/identity, so writing the new key without resolving the old one first + matches nothing and creates a second entity, orphaning the original." + (setup-test-data []) + @(dc/transact conn [{:db/id "r" + :sales-refund/external-id "square/refund/abc" + :sales-refund/total 10.0}]) + (is (= 1 (refund-count))) + (let [eid (resolve-refund "abc")] + (is (some? eid) "resolves an entity carrying the legacy key") + @(dc/transact conn [{:db/id eid + :sales-refund/external-id (sut/scoped-key "square/refund/" client location "abc") + :sales-refund/total 10.0}]) + (is (= 1 (refund-count)) "no second entity was created") + (is (= eid (dc/entid (dc/db conn) [:sales-refund/external-id "square/refund/NGCD-CD-abc"])) + "the same entity now answers to the scoped key") + (is (nil? (dc/entid (dc/db conn) [:sales-refund/external-id "square/refund/abc"])) + "and no longer to the legacy one")))) + +(deftest already-scoped-entity-resolves-by-its-new-key + (testing "re-running the importer after migration finds the entity by the scoped key, so the + migration is not undone and nothing is duplicated" + (setup-test-data []) + @(dc/transact conn [{:db/id "r" + :sales-refund/external-id "square/refund/NGCD-CD-abc" + :sales-refund/total 10.0}]) + (is (= (dc/entid (dc/db conn) [:sales-refund/external-id "square/refund/NGCD-CD-abc"]) + (resolve-refund "abc"))) + (is (= 1 (refund-count))))) + +(deftest unknown-id-resolves-to-nothing + (testing "a refund never seen before has no id to pin, so the importer creates it fresh" + (setup-test-data []) + (is (nil? (resolve-refund "never-seen"))))) + +(deftest two-clients-on-one-location-get-their-own-entities + (testing "the point of the re-key: with the client in the key, a second client importing the + same Square refund creates its own entity instead of taking ownership of the first" + (setup-test-data []) + (let [other {:client/code "NGCC"} + other-loc {:square-location/client-location "CC"}] + @(dc/transact conn [{:db/id "r" + :sales-refund/external-id (sut/scoped-key "square/refund/" client location "shared") + :sales-refund/total 10.0}]) + (is (nil? (sut/existing-id (dc/db conn) :sales-refund/external-id "square/refund/" other other-loc "shared")) + "the second client does not resolve onto the first client's entity") + @(dc/transact conn [{:db/id "r2" + :sales-refund/external-id (sut/scoped-key "square/refund/" other other-loc "shared") + :sales-refund/total 10.0}]) + (is (= 2 (refund-count)) "two stable entities, one per client, rather than one that flips"))))