diff --git a/config/events/schema.json b/config/events/schema.json
index 7c5128b292a504d6a6fdeed67b86eec4e59d27ed..6e43b23bba2aa9008c746d9f458bb7c2b4a52795 100644
--- a/config/events/schema.json
+++ b/config/events/schema.json
@@ -47,33 +47,73 @@
}
},
"additional_properties": {
- "type": "object",
- "properties": {
- "label": {
+ "anyOf": [
+ {
+ "description": "Only base additional properties are provided",
"type": "object",
"properties": {
- "description": {
- "type": "string"
+ "label": {
+ "$ref": "#additional_prop"
+ },
+ "property": {
+ "$ref": "#additional_prop"
+ },
+ "value": {
+ "$ref": "#additional_prop"
+ }
+ },
+ "additionalProperties": {
+ "type": "null",
+ "x-error": "Either property/label or value must be used before defining other additional_properties"
+ }
+ },
+ {
+ "description": "All string-type base additional properties are provided, allowing custom properties",
+ "type": "object",
+ "properties": {
+ "label": {
+ "$ref": "#additional_prop"
+ },
+ "property": {
+ "$ref": "#additional_prop"
+ },
+ "value": {
+ "$ref": "#additional_prop"
}
},
"required": [
- "description"
+ "property",
+ "label"
],
- "additionalProperties": false
+ "additionalProperties": {
+ "$ref": "#additional_prop"
+ }
},
- "property": {
+ {
+ "description": "All numeric-type base additional properties are provided, allowing custom properties",
"type": "object",
"properties": {
- "description": {
- "type": "string"
+ "label": {
+ "$ref": "#additional_prop"
+ },
+ "property": {
+ "$ref": "#additional_prop"
+ },
+ "value": {
+ "$ref": "#additional_prop"
}
},
"required": [
- "description"
+ "value"
],
- "additionalProperties": false
- },
- "value": {
+ "additionalProperties": {
+ "$ref": "#additional_prop"
+ }
+ }
+ ],
+ "$defs": {
+ "additional_prop": {
+ "$anchor": "additional_prop",
"type": "object",
"properties": {
"description": {
@@ -85,8 +125,7 @@
],
"additionalProperties": false
}
- },
- "additionalProperties": true
+ }
},
"iglu_schema_url": {
"type": [
diff --git a/lib/gitlab/tracking/event_definition_validator.rb b/lib/gitlab/tracking/event_definition_validator.rb
index 2ba65a20054b78fa38bc3f4e5a679a2784874622..b0d5b8a897b392d234a15f3b873540abf6e50959 100644
--- a/lib/gitlab/tracking/event_definition_validator.rb
+++ b/lib/gitlab/tracking/event_definition_validator.rb
@@ -5,7 +5,6 @@ module Tracking
class EventDefinitionValidator
EVENT_SCHEMA_PATH = Rails.root.join('config/events/schema.json')
SCHEMA = ::JSONSchemer.schema(EVENT_SCHEMA_PATH)
- NOT_VALIDATED_PROPERTIES = [:value].freeze
def initialize(definition)
@attributes = definition.attributes
@@ -13,37 +12,6 @@ def initialize(definition)
end
def validation_errors
- (validate_schema << validate_additional_properties).compact
- end
-
- private
-
- attr_reader :attributes, :path
-
- def validate_additional_properties
- additional_props = attributes[:additional_properties]&.map { |k, _| k }
-
- return unless additional_props.present?
-
- extra_props = additional_props - Gitlab::Tracking::EventValidator::BASE_ADDITIONAL_PROPERTIES.keys
- unused_props = prioritized_properties - additional_props
-
- return unless extra_props.present? && unused_props.present?
-
- <<~ERROR_MSG
- --------------- VALIDATION ERROR ---------------
- Definition file: #{path}
- Error type: consider using the built-in additional properties:
- "#{prioritized_properties.join(', ')}"
- before adding custom extra properties: #{extra_props.join(', ')}
- ERROR_MSG
- end
-
- def prioritized_properties
- Gitlab::Tracking::EventValidator::BASE_ADDITIONAL_PROPERTIES.keys - NOT_VALIDATED_PROPERTIES
- end
-
- def validate_schema
SCHEMA.validate(attributes.deep_stringify_keys).map do |error|
<<~ERROR_MSG
--------------- VALIDATION ERROR ---------------
@@ -51,10 +19,14 @@ def validate_schema
Error type: #{error['type']}
Data: #{error['data']}
Path: #{error['data_pointer']}
- Details: #{error['details']}
+ Details: #{error['details'] || error['error']}
ERROR_MSG
end
end
+
+ private
+
+ attr_reader :attributes, :path
end
end
end
diff --git a/spec/fixtures/scripts/internal_events/events/event_with_multiple_custom_properties.yml b/spec/fixtures/scripts/internal_events/events/event_with_multiple_custom_properties.yml
index 7e91c993dbfebcf6a23f7c40710df716de4a9882..552647fda285b560072eab03b1864e01af73e0b4 100644
--- a/spec/fixtures/scripts/internal_events/events/event_with_multiple_custom_properties.yml
+++ b/spec/fixtures/scripts/internal_events/events/event_with_multiple_custom_properties.yml
@@ -9,6 +9,8 @@ identifiers:
additional_properties:
label:
description: TODO
+ property:
+ description: TODO
custom_key1:
description: The extra custom property name 1
custom_key2:
diff --git a/spec/fixtures/scripts/internal_events/new_metrics.yml b/spec/fixtures/scripts/internal_events/new_metrics.yml
index c14c252af2871bccf3a63382e1fbb730c6cc4f49..c57d137bcaf092f04c07b55297019c18927cad48 100644
--- a/spec/fixtures/scripts/internal_events/new_metrics.yml
+++ b/spec/fixtures/scripts/internal_events/new_metrics.yml
@@ -379,6 +379,7 @@
- "\e[B\e[B\e[B" # Arrow down to: Weekly count of unique users where label/value is...
- "\n" # Select: Weekly count of unique users where label/value is...
- "failure\n" # Input value for "label" filter
+ - " \n" # Ignore whitespace to skip "property" filter
- "metrics\n" # Input value for "custom_key1" filter
- "\n" # Skip "custom_key2" filter
- "30,13\n" # Input value for "custom_key3" filter
diff --git a/spec/lib/gitlab/tracking/event_definition_validator_spec.rb b/spec/lib/gitlab/tracking/event_definition_validator_spec.rb
index 6d60a2b05ae52a9c1274174cb40bbf91d3e2f1c6..1dcbede57e34bc96849c71310639cc1d429588bb 100644
--- a/spec/lib/gitlab/tracking/event_definition_validator_spec.rb
+++ b/spec/lib/gitlab/tracking/event_definition_validator_spec.rb
@@ -22,7 +22,6 @@
let(:path) { File.join('events', 'issues_create.yml') }
let(:definition) { Gitlab::Tracking::EventDefinition.new(path, attributes) }
- # let(:yaml_content) { attributes.deep_stringify_keys.to_yaml }
describe '#validate' do
using RSpec::Parameterized::TableSyntax
@@ -69,15 +68,21 @@
where(:label, :property, :value, :custom, :error?) do
true | true | true | true | false
true | true | true | false | false
- true | true | false | false | false
true | true | false | true | false
+ true | true | false | false | false
+ true | false | true | true | false
+ true | false | true | false | false
+ true | false | false | true | true
true | false | false | false | false
- false | true | false | false | false
+ false | true | true | true | false
false | true | true | false | false
- true | false | true | true | true
- true | false | false | true | true
- false | true | true | true | true
- false | false | true | true | true
+ false | true | false | true | true
+ false | true | false | false | false
+ false | false | true | true | false
+ false | false | true | false | false
+ false | false | false | true | true
+ false | false | false | false | false
+ nil | nil | nil | nil | false
end
with_them do
@@ -86,6 +91,8 @@
attributes[:additional_properties][:property] = { description: "button state" } if property
attributes[:additional_properties][:value] = { description: "package version" } if value
attributes[:additional_properties][:custom] = { description: "custom" } if custom
+
+ attributes.delete(:additional_properties) if [label, property, value, custom].all?(&:nil?)
end
subject { described_class.new(definition).validation_errors.any? }