Skip to content

Commit

Permalink
common: do not validate enrichment names (close #767)
Browse files Browse the repository at this point in the history
Users are often confused by the “vendor” and “name” fields in the enrichment configs.
There is no reason we need to validate or even use these fields, because the “schema” field already contains enough information.
This commit removes the validation on “name” (I believe “vendor” is not validated) and only uses “schema” instead.
  • Loading branch information
stanch committed Apr 11, 2023
1 parent 47b74be commit 28e7feb
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -181,59 +181,54 @@ object EnrichmentRegistry {
CirceUtils.extract[Boolean](enrichmentConfig, "enabled").toEither match {
case Right(false) => None.validNel // Enrichment is disabled
case _ =>
(for {
nm <- CirceUtils
.extract[String](enrichmentConfig, "name")
.toValidatedNel[String, String]
.toEither
e = if (nm == "ip_lookups")
IpLookupsEnrichment.parse(enrichmentConfig, schemaKey, localMode).map(_.some)
else if (nm == "anon_ip")
AnonIpEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
else if (nm == "referer_parser")
RefererParserEnrichment.parse(enrichmentConfig, schemaKey, localMode).map(_.some)
else if (nm == "campaign_attribution")
CampaignAttributionEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
else if (nm == "user_agent_utils_config")
UserAgentUtilsEnrichmentConfig.parse(enrichmentConfig, schemaKey).map(_.some)
else if (nm == "ua_parser_config")
UaParserEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
else if (nm == "yauaa_enrichment_config")
YauaaEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
else if (nm == "currency_conversion_config")
CurrencyConversionEnrichment
.parse(enrichmentConfig, schemaKey)
.map(_.some)
else if (nm == "javascript_script_config")
JavascriptScriptEnrichment
.parse(enrichmentConfig, schemaKey)
.map(_.some)
else if (nm == "event_fingerprint_config")
EventFingerprintEnrichment
.parse(enrichmentConfig, schemaKey)
.map(_.some)
else if (nm == "cookie_extractor_config")
CookieExtractorEnrichment
.parse(enrichmentConfig, schemaKey)
.map(_.some)
else if (nm == "http_header_extractor_config")
HttpHeaderExtractorEnrichment
.parse(enrichmentConfig, schemaKey)
.map(_.some)
else if (nm == "weather_enrichment_config")
WeatherEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
else if (nm == "api_request_enrichment_config")
ApiRequestEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
else if (nm == "sql_query_enrichment_config")
SqlQueryEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
else if (nm == "pii_enrichment_config")
PiiPseudonymizerEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
else if (nm == "iab_spiders_and_robots_enrichment")
IabEnrichment.parse(enrichmentConfig, schemaKey, localMode).map(_.some)
else
None.validNel // Enrichment is not recognized
enrichment <- e.toEither
} yield enrichment).toValidated
schemaKey.name match {
case "ip_lookups" =>
IpLookupsEnrichment.parse(enrichmentConfig, schemaKey, localMode).map(_.some)
case "anon_ip" =>
AnonIpEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
case "referer_parser" =>
RefererParserEnrichment.parse(enrichmentConfig, schemaKey, localMode).map(_.some)
case "campaign_attribution" =>
CampaignAttributionEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
case "user_agent_utils_config" =>
UserAgentUtilsEnrichmentConfig.parse(enrichmentConfig, schemaKey).map(_.some)
case "ua_parser_config" =>
UaParserEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
case "yauaa_enrichment_config" =>
YauaaEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
case "currency_conversion_config" =>
CurrencyConversionEnrichment
.parse(enrichmentConfig, schemaKey)
.map(_.some)
case "javascript_script_config" =>
JavascriptScriptEnrichment
.parse(enrichmentConfig, schemaKey)
.map(_.some)
case "event_fingerprint_config" =>
EventFingerprintEnrichment
.parse(enrichmentConfig, schemaKey)
.map(_.some)
case "cookie_extractor_config" =>
CookieExtractorEnrichment
.parse(enrichmentConfig, schemaKey)
.map(_.some)
case "http_header_extractor_config" =>
HttpHeaderExtractorEnrichment
.parse(enrichmentConfig, schemaKey)
.map(_.some)
case "weather_enrichment_config" =>
WeatherEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
case "api_request_enrichment_config" =>
ApiRequestEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
case "sql_query_enrichment_config" =>
SqlQueryEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
case "pii_enrichment_config" =>
PiiPseudonymizerEnrichment.parse(enrichmentConfig, schemaKey).map(_.some)
case "iab_spiders_and_robots_enrichment" =>
IabEnrichment.parse(enrichmentConfig, schemaKey, localMode).map(_.some)
case _ =>
Option.empty[EnrichmentConf].validNel // Enrichment is not recognized
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,17 @@ object SpecHelpers {
s => s
)

// Vendor and name are intentionally tweaked in the first enrichment
// to test that we are no longer validating them (users were confused about such validation)
val enrichmentConfig =
"""|{
|"schema": "iglu:com.snowplowanalytics.snowplow/enrichments/jsonschema/1-0-0",
|"data": [
|{
|"schema": "iglu:com.snowplowanalytics.snowplow/anon_ip/jsonschema/1-0-0",
|"data": {
|"vendor": "com.snowplowanalytics.snowplow",
|"name": "anon_ip",
|"vendor": "com.snowplowanalytics.snowplow_custom",
|"name": "anon_ip_custom",
|"enabled": true,
|"parameters": {
|"anonOctets": 1
Expand Down

0 comments on commit 28e7feb

Please sign in to comment.