Patch existing entities in POST /bundle/import (#1383)

**Epic** advthreat/iroh#7341 Related advthreat/iroh#8207 Current behavior of bundle import on existing entities is to ignore them. This PR changes behavior to patch in this case, and loosens the schema for bundle import so then partial entities are allowed if patching.  <a name="qa">[§](#qa)</a> QA ============================ This PR adds the ability for `POST /bundle/import` to patch entities. The following steps test if this procedure works. 1. Create an existing entity (for example an Indicator) and note its id. 2. Patch this entity using `POST /bundle/import`. For example, patching the `:producer` field of the indicator: ``` POST /bundle/import?patch-existing=true {:indicators [{:id "<id-from-step-1>" :producer "PATCHED"}]} ``` 3. Verify that the result looks something like this (the `"updated"` part is the most important part): ``` {:results [{:result "updated" :id "<id-from-step-1>"}]} ``` 4. Look up the indicator via `GET /ctia/indicator` to verify that the `producer` field was updated.   <a name="release-notes">[§](#release-notes)</a> Release Notes =============================================================  ``` intern: Patch existing entities in POST /bundle/import ``` <a name="squashed-commits">[§](#squashed-commits)</a> Squashed Commits ======================================================================
threatgrid · Oct 13, 2023 · f077c14 · f077c14
1 parent 5907759
commit f077c14
Show file tree

Hide file tree

Showing 16 changed files with 1,122 additions and 484 deletions.
diff --git a/project.clj b/project.clj
@@ -210,7 +210,8 @@
                                          "git" "symbolic-ref" "--short" "HEAD")))})}]
 
 
-  :profiles {:dev {:dependencies [[puppetlabs/trapperkeeper ~trapperkeeper-version
+  :profiles {:dev {:jvm-opts ["-XX:-OmitStackTraceInFastThrow"]
+                   :dependencies [[puppetlabs/trapperkeeper ~trapperkeeper-version
                                    :classifier "test"]
                                   [puppetlabs/kitchensink ~trapperkeeper-version
                                    :classifier "test"]
@@ -228,7 +229,8 @@
                   :global-vars {*warn-on-reflection* true}
                   :jvm-opts [ ;; actually print stack traces instead of useless
                              ;; "Full report at: /tmp/clojure-8187773283812483853.edn"
-                             "-Dclojure.main.report=stderr"]}
+                             "-Dclojure.main.report=stderr"
+                             "-XX:-OmitStackTraceInFastThrow"]}
              :next-clojure {:dependencies [[org.clojure/clojure "1.12.0-master-SNAPSHOT"]]
                             :repositories [["snapshots" "https://oss.sonatype.org/content/repositories/snapshots/"]]}
              :jmx {:jvm-opts ["-Dcom.sun.management.jmxremote"

diff --git a/resources/ctia/public/doc/bulk-bundle.org b/resources/ctia/public/doc/bulk-bundle.org
@@ -667,10 +667,18 @@ When a bundle is submitted:
 
 1. All entities that have already been imported with the external ID whose prefix has been configured with the `ctia.store.external-key-prefixes` property are searched.
 2. If they are identified by transient IDs, a mapping table between transient and stored IDs is built.
-3. Only new entities are created in the same way as the `/bulk` API endpoint with transient IDs resolutions. Existing entities are not modified.
+3. New entities are created in the same way as the `/bulk` API endpoint with transient IDs and external ids resolutions. If query parameter `patch-existing=true`, then existing entities are similarly patched with `result=updated`; otherwise, existing entities are not modified with `result=exists`.
 
 If more than one entity is referenced by the same external ID, an error is reported.
 
+If provided query parameter `asset_properties-merge-strategy=merge-overriding-previous`, then when patching asset properties,
+existing asset properties will be retrieved and combined with the asset properties in the request bundle
+as if by concatenating existing and new properties together in a single list,
+removing properties to the left of a property with the same name, 
+then sorting the list lexicographically by name before using this list to patch the existing entity.
+
+If `asset_properties-merge-strategy=ignore-existing`, then asset properties will be patched to their new values as they appear in the request bundle.
+
 Example for an incident along with its context, note the transient ids
 #+begin_src HTTP
 POST /ctia/bundle/import HTTP/1.1
@@ -778,7 +786,7 @@ accept: application/json
 |----------------+----------------------------------------------------------|
 | `:id`          | The real ID                                              |
 | `:original_id` | Provided ID if different from real ID (ex: transient ID) |
-| `:result`      | `error`, `created` or `exists`                           |
+| `:result`      | `error`, `created`, `exists` or `updated`                |
 | `:external_id` | External ID used to identify the entity                  |
 | `:error`       | Error message                                            |
 

diff --git a/src/ctia/auth.clj b/src/ctia/auth.clj
@@ -39,13 +39,16 @@
 
 (def denied-identity-singleton (->DeniedIdentity))
 
+(s/defschema AuthIdentity
+  (s/protocol IIdentity))
+
 (s/defschema IdentityMap
   {:client-id (s/maybe s/Str)
    :login (s/maybe s/Str)
    :groups [s/Str]})
 
 (s/defn ident->map :- (s/maybe IdentityMap)
-  [ident]
+  [ident :- (s/maybe AuthIdentity)]
   (when ident
     {:login (login ident)
      :groups (groups ident)

diff --git a/src/ctia/bulk/core.clj b/src/ctia/bulk/core.clj
@@ -42,17 +42,47 @@
 
 (s/defn create-fn
   "return the create function provided an entity type key"
-  [k auth-identity params
+  [k 
+   auth-identity :- auth/AuthIdentity
+   params
    {{:keys [get-store]} :StoreService} :- APIHandlerServices]
   #(-> (get-store k)
        (store/create-record
          %
          (auth/ident->map auth-identity)
          params)))
 
-(s/defn create-entities
+(s/defschema EntitiesResult
+  [(s/conditional
+     string? schemas/ID
+     :else {(s/optional-key :error) (s/conditional
+                                      string? s/Str
+                                      :else {:type (s/conditional
+                                                     string? s/Str
+                                                     :else s/Keyword)
+                                             :reason s/Str
+                                             (s/optional-key :index) s/Str
+                                             (s/optional-key :index_uuid) s/Str})
+            (s/optional-key :msg) s/Str
+            (s/optional-key :entity) (s/pred map?)
+            (s/optional-key :type) (s/conditional
+                                     string? s/Str
+                                     :else s/Keyword)
+            (s/optional-key :id) (s/maybe s/Str)
+            s/Keyword s/Any})])
+
+(s/defschema EnvelopedEntities+TempIDs
+  (s/maybe
+    {:data EntitiesResult
+     (s/optional-key :tempids) TempIDs}))
+
+(s/defn create-entities :- EnvelopedEntities+TempIDs
   "Create many entities provided their type and returns a list of ids"
-  [new-entities entity-type tempids auth-identity params
+  [new-entities :- flows/Entities
+   entity-type :- s/Keyword
+   tempids :- TempIDs
+   auth-identity :- auth/AuthIdentity
+   params
    services :- APIHandlerServices]
   (when (seq new-entities)
     (let [{:keys [realize-fn new-spec]} (get (all-entities) entity-type)]
@@ -82,9 +112,11 @@
                      (st/assoc s/Keyword s/Any))
    s/Keyword s/Any})
 
-(s/defn read-entities
+(s/defn read-entities :- [(s/maybe (s/pred map?))]
   "Retrieve many entities of the same type provided their ids and common type"
-  [ids entity-type auth-identity
+  [ids :- [s/Str]
+   entity-type :- s/Keyword
+   auth-identity :- auth/AuthIdentity
    {{:keys [get-store]} :StoreService
     :as services} :- ReadEntitiesServices]
   (let [store (get-store entity-type)]
@@ -125,9 +157,19 @@
                  concat
                  (map #(to-long-id % services) not-found)))))
 
+(defn make-patch-bulk-enveloped-result
+  [fm]
+  (-> fm
+      flows/make-enveloped-result
+      (update :data #(mapv (fn [{:keys [error id] :as result}]
+                             (if error result id))
+                           %))))
+
 (s/defn delete-fn
   "return the delete function provided an entity type key"
-  [k auth-identity params
+  [k 
+   auth-identity :- auth/AuthIdentity
+   params
    {{:keys [get-store]} :StoreService} :- APIHandlerServices]
   #(-> (get-store k)
        (store/bulk-delete
@@ -137,7 +179,9 @@
 
 (s/defn update-fn
   "return the update function provided an entity type key"
-  [k auth-identity params
+  [k
+   auth-identity :- auth/AuthIdentity
+   params
    {{:keys [get-store]} :StoreService} :- APIHandlerServices]
   #(-> (get-store k)
        (store/bulk-update
@@ -159,7 +203,10 @@
 
 (s/defn delete-entities
   "delete many entities provided their type and returns a list of ids"
-  [entity-ids entity-type auth-identity params
+  [entity-ids 
+   entity-type
+   auth-identity :- auth/AuthIdentity
+   params
    services :- APIHandlerServices]
   (when (seq entity-ids)
     (let [get-fn #(read-entities %  entity-type auth-identity services)]
@@ -176,7 +223,10 @@
 
 (s/defn update-entities
   "update many entities provided their type and returns errored and successed entities' ids"
-  [entities entity-type auth-identity params
+  [entities 
+   entity-type
+   auth-identity :- auth/AuthIdentity
+   params
    services :- APIHandlerServices]
   (when (seq entities)
     (let [get-fn #(read-entities %  entity-type auth-identity services)
@@ -196,24 +246,32 @@
 
 (s/defn patch-entities
   "patch many entities provided their type and returns errored and successed entities' ids"
-  [patches entity-type auth-identity params
-   services :- APIHandlerServices]
+  [patches
+   entity-type 
+   tempids :- TempIDs
+   auth-identity :- auth/AuthIdentity
+   params
+   services :- APIHandlerServices
+   {:keys [make-result]} :- {(s/optional-key :make-result) s/Any}]
   (when (seq patches)
     (let [get-fn #(read-entities %  entity-type auth-identity services)
           {:keys [realize-fn new-spec]} (get (all-entities) entity-type)]
       (flows/patch-flow
-       :services services
-       :get-fn get-fn
-       :realize-fn realize-fn
-       :update-fn (update-fn entity-type auth-identity params services)
-       :long-id-fn #(with-long-id % services)
-       :entity-type entity-type
-       :identity auth-identity
-       :patch-operation :replace
-       :partial-entities patches
-       :spec new-spec
-       :make-result make-bulk-result
-       :get-success-entities (get-success-entities-fn :updated)))))
+        :services services
+        :get-fn get-fn
+        :realize-fn realize-fn
+        :update-fn (update-fn entity-type auth-identity params services)
+        :long-id-fn #(with-long-id % services)
+        :entity-type entity-type
+        :identity auth-identity
+        :patch-operation :replace
+        :partial-entities patches
+        :tempids tempids
+        :spec new-spec
+        :make-result (or make-result make-bulk-result)
+        :get-success-entities (get-success-entities-fn :updated)))))
+
+(s/defschema BulkEntities {s/Keyword flows/Entities})
 
 (defn gen-bulk-from-fn
   "Kind of fmap but adapted for bulk
@@ -237,7 +295,7 @@
     (catch java.util.concurrent.ExecutionException e
       (throw (.getCause e)))))
 
-(defn merge-tempids
+(s/defn merge-tempids :- TempIDs
   "Merges tempids from all entities
    {:entity-type1 {:data []
                    :tempids {transientid1 id1
@@ -255,7 +313,7 @@
 
   The create-entities set the enveloped-result? to True in the flow
   configuration to get :data and :tempids for each entity in the result."
-  [entities-by-type]
+  [entities-by-type :- {s/Keyword EnvelopedEntities+TempIDs}]
   (into {}
         (map (fn [[_ v]] (:tempids v)))
         entities-by-type))
@@ -278,15 +336,19 @@
     (bad-request! (str "Bulk max number of entities: "
                        (get-bulk-max-size get-in-config)))))
 
+(s/defschema BulkRefs {s/Keyword EntitiesResult})
+
 (s/defschema BulkRefs+TempIDs
-  {:bulk-refs {s/Keyword [s/Any]}
+  {:bulk-refs BulkRefs
    :tempids TempIDs})
 
+(s/defschema BulkRefsAssocTempIDs
+  (st/assoc BulkRefs (s/optional-key :tempids) TempIDs))
+
 (s/defn import-bulks-with :- BulkRefs+TempIDs
   "Import each new-bulk in order while accumulating tempids."
-  [f :- (s/=> {s/Keyword {:data [s/Any]
-                          :tempids TempIDs}}
-              (s/named (s/pred map?) 'new-bulk)
+  [f :- (s/=> {s/Keyword EnvelopedEntities+TempIDs}
+              BulkEntities
               TempIDs)
    new-bulks
    tempids :- TempIDs]
@@ -300,15 +362,15 @@
            :tempids tempids}
           new-bulks))
 
-(s/defn create-bulk
+(s/defn create-bulk :- BulkRefsAssocTempIDs
   "Creates entities in bulk. To define relationships between entities,
    transient IDs can be used. They are automatically converted into
    real IDs.
 
    1. Creates all entities except Relationships
    2. Creates Relationships with mapping between transient and real IDs"
   ([new-bulk login services :- APIHandlerServices] (create-bulk new-bulk {} login {} services))
-  ([new-bulk
+  ([new-bulk :- BulkEntities
     tempids :- TempIDs
     login
     params
@@ -334,22 +396,40 @@
        (seq tempids) (assoc :tempids tempids)))))
 
 (s/defn fetch-bulk
-  [bulk auth-identity
+  [bulk 
+   auth-identity :- auth/AuthIdentity
    services :- APIHandlerServices]
   (ent/un-store-map
    (gen-bulk-from-fn read-entities bulk auth-identity services)))
 
 (s/defn delete-bulk
-  [bulk auth-identity params
+  [bulk 
+   auth-identity :- auth/AuthIdentity
+   params
    services :- APIHandlerServices]
   (gen-bulk-from-fn delete-entities bulk auth-identity params services))
 
 (s/defn update-bulk
-  [bulk auth-identity params
+  [bulk 
+   auth-identity :- auth/AuthIdentity
+   params
    services :- APIHandlerServices]
   (gen-bulk-from-fn update-entities bulk auth-identity params services))
 
 (s/defn patch-bulk
-  [bulk auth-identity params
-   services :- APIHandlerServices]
-  (gen-bulk-from-fn patch-entities bulk auth-identity params services))
+  ([bulk
+    tempids :- TempIDs
+    auth-identity :- auth/AuthIdentity
+    params
+    services :- APIHandlerServices]
+   (patch-bulk bulk tempids auth-identity params services {}))
+  ([bulk
+    tempids :- TempIDs
+    auth-identity :- auth/AuthIdentity
+    params
+    services :- APIHandlerServices
+    {:keys [enveloped-result?] :as opts}]
+   (let [entities (gen-bulk-from-fn patch-entities bulk tempids auth-identity params services (select-keys opts [:make-result]))]
+     (cond-> entities
+       enveloped-result? (-> (update-vals :data)
+                             (assoc :tempids (into tempids (merge-tempids entities))))))))
diff --git a/src/ctia/bulk/routes.clj b/src/ctia/bulk/routes.clj
@@ -150,6 +150,7 @@
         :auth-identity auth-identity
         (core/validate-bulk-size! bulk services)
         (ok (core/patch-bulk bulk
+                             {} ;; transient ids only supported via PATCH bundle/import
                              auth-identity
                              (common/wait_for->refresh wait_for)
                              services))))