Common: fix PII enrichment adding empty objects instead of missing pr…

…operties (close #351)
snowplow · Sep 23, 2020 · 1e937a1 · 1e937a1
1 parent 86536a5
commit 1e937a1
Show file tree

Hide file tree

Showing 6 changed files with 440 additions and 33 deletions.
diff --git a/...nalytics.snowplow.enrich/common/enrichments/registry/pii/PiiPseudonymizerEnrichment.scala b/...nalytics.snowplow.enrich/common/enrichments/registry/pii/PiiPseudonymizerEnrichment.scala
@@ -133,6 +133,25 @@ object PiiPseudonymizerEnrichment extends ParseableEnrichment {
       .get(fieldName)
       .map(_.asRight)
       .getOrElse(s"The specified json field $fieldName is not supported".asLeft)
+
+  /** Helper to remove fields that were wrongly added and are not in the original JSON. See #351. */
+  private[pii] def removeAddedFields(hashed: Json, original: Json): Json = {
+    val fixedObject = for {
+      hashedFields <- hashed.asObject
+      originalFields <- original.asObject
+      newFields = hashedFields.toList.flatMap {
+                    case (k, v) => originalFields(k).map(origV => (k, removeAddedFields(v, origV)))
+                  }
+    } yield Json.fromFields(newFields)
+
+    lazy val fixedArray = for {
+      hashedArr <- hashed.asArray
+      originalArr <- original.asArray
+      newArr = hashedArr.zip(originalArr).map { case (hashed, orig) => removeAddedFields(hashed, orig) }
+    } yield Json.fromValues(newArr)
+
+    fixedObject.orElse(fixedArray).getOrElse(hashed)
+  }
 }
 
 /**
@@ -204,7 +223,8 @@ final case class PiiJson(
                                           )
                                         }
                                         .getOrElse((parsed, List.empty[JsonModifiedField]))
-    } yield (substituted.noSpaces, modifiedFields.toList)).getOrElse((null, List.empty))
+    } yield (PiiPseudonymizerEnrichment.removeAddedFields(substituted, parsed).noSpaces, modifiedFields.toList))
+      .getOrElse((null, List.empty))
 
   /** Map context top fields with strategy if they match. */
   private def mapContextTopFields(tuple: (String, Json), strategy: PiiStrategy): (String, (Json, List[JsonModifiedField])) =
@@ -272,10 +292,6 @@ final case class PiiJson(
           jsonPath,
           new ScrambleMapFunction(strategy, modifiedFields, fieldMutator.fieldName, jsonPath, schema)
         )
-        // make sure it is a structure preserving method, see #3636
-        //val transformedJValue = JsonMethods.fromJsonNode(documentContext.json[JsonNode]())
-        //val Diff(_, erroneouslyAdded, _) = jValue diff transformedJValue
-        //val Diff(_, withoutCruft, _) = erroneouslyAdded diff transformedJValue
         (jacksonToCirce(documentContext2.json[JsonNode]()), modifiedFields.toList)
     }
   }

diff --git a/modules/common/src/test/resources/iglu-schemas/schemas/com.acme/email_sent/jsonschema/1-0-0 b/modules/common/src/test/resources/iglu-schemas/schemas/com.acme/email_sent/jsonschema/1-0-0
@@ -14,6 +14,9 @@
     },
     "emailAddress2": {
       "type": "string"
+    },
+    "emailAddress3": {
+      "type": "string"
     }
   },
   "required": ["emailAddress", "emailAddress2"],

diff --git a/modules/common/src/test/resources/iglu-schemas/schemas/com.acme/email_sent/jsonschema/2-0-0 b/modules/common/src/test/resources/iglu-schemas/schemas/com.acme/email_sent/jsonschema/2-0-0
@@ -0,0 +1,24 @@
+{
+  "$schema": "http://iglucentral.com/schemas/com.snowplowanalytics.self-desc/schema/jsonschema/1-0-0#",
+  "description": "Schema for acme stuff",
+  "self": {
+    "vendor": "com.acme",
+    "name": "email_sent",
+    "format": "jsonschema",
+    "version": "1-1-0"
+  },
+  "type": "object",
+  "properties": {
+    "emailAddress": {
+      "type": "string"
+    },
+    "emailAddress2": {
+      "type": "string"
+    },
+    "emailAddress3": {
+      "type": ["string", "null"]
+    }
+  },
+  "required": ["emailAddress", "emailAddress2"],
+  "additionalProperties": false
+}
diff --git a/modules/common/src/test/resources/iglu-schemas/schemas/com.test/array/jsonschema/1-0-0 b/modules/common/src/test/resources/iglu-schemas/schemas/com.test/array/jsonschema/1-0-0
@@ -28,6 +28,10 @@
             "type": "string"
           }
         }
+      },
+      "field4": {
+        "type": "string",
+        "maxLength": 64
       }
     },
     "required": ["field"],