jenkins-bot has submitted this change and it was merged.
Change subject: Revert "Ignore country values that are not two characters long"
......................................................................
Revert "Ignore country values that are not two characters long"
This reverts commit e4b68b1399ad1e20f84d3c3f44d1128c269fc8ba.
No longer required.
Change-Id: I14fdd42cc98c70f683f851470f50a1cf22d1b470
---
M server/bin/eventlogging-devserver
M server/eventlogging/schema.py
M server/tests/test_schema.py
3 files changed, 2 insertions(+), 157 deletions(-)
Approvals:
Ori.livneh: Looks good to me, approved
jenkins-bot: Verified
diff --git a/server/bin/eventlogging-devserver
b/server/bin/eventlogging-devserver
index 6670932..2414b97 100755
--- a/server/bin/eventlogging-devserver
+++ b/server/bin/eventlogging-devserver
@@ -124,10 +124,7 @@
return event, [err]
validator = jsonschema.Draft3Validator(schema)
- validation_errors = list(validator.iter_errors(event))
- if not validation_errors:
- eventlogging.post_validation_fixups(event)
- return event, validation_errors
+ return event, list(validator.iter_errors(event))
def handle_event(environ, start_response):
diff --git a/server/eventlogging/schema.py b/server/eventlogging/schema.py
index 326b18e..6010211 100644
--- a/server/eventlogging/schema.py
+++ b/server/eventlogging/schema.py
@@ -12,12 +12,10 @@
from __future__ import unicode_literals
import jsonschema
-import logging
from .compat import json, http_get
-__all__ = ('CAPSULE_SCID', 'get_schema', 'SCHEMA_URL_FORMAT',
- 'post_validation_fixups', 'validate')
+__all__ = ('CAPSULE_SCID', 'get_schema', 'SCHEMA_URL_FORMAT', 'validate')
# URL of index.php on the schema wiki (same as
@@ -63,80 +61,6 @@
return schema
-def delete_if_exists_and_length_mismatches(capsule, field, expected_length):
- """Remove the field from the capsule's event, if it exists and
- does not have a length of expected_length.
-
- Relies on capsule being valid.
- """
- try:
- value = capsule['event'][field]
- actual_length = len(value)
- if actual_length != expected_length:
- del capsule['event'][field]
- logging.error(
- 'Post validation fixup: {0}_{1}, removing field {2} '
- 'because length is {3} instead of {4}'.format(
- capsule['schema'],
- capsule['revision'],
- field,
- actual_length,
- expected_length
- )
- )
- except KeyError:
- # capsule['event'][event_field] does not exist
- # That's ok. Nothing to fixup.
- pass
-
-
-def post_validation_fixups(capsule):
- """Clean known harmfull or semantically wrong data from validated
- capsules.
-
- This function never turns a valid capsule into an invalid one.
- """
- # Add checks only sparingly to this function.
- #
- # Do not use this function to implement more thorough checking of
- # existing schemas. Refine the schemas to achieve that, and refine
- # the client code to send the proper values.
- #
- # Instead, use this function only to clean up a few fields that
- # are known to be harmful and (after cleaning up client code) are
- # still arriving in the EventLogging pipeline, but do not warrant
- # throwing away the whole event.
-
- # As the capsule is required to be valid, schema and revision keys
- # have to exist.
- schema = capsule['schema']
-
- if schema == 'MultimediaViewerDuration':
- if capsule['revision'] in [8318615, 8572641]:
- # Cleanup against session cookies leaking in.
- # See bug #66478
- #
- # TODO: Please check after 2014-09-13, and remove if
- # clients stopped sending sessionId.
- delete_if_exists_and_length_mismatches(capsule, 'country', 2)
- elif schema == 'MultimediaViewerNetworkPerformance':
- if capsule['revision'] == 7917896:
- # Cleanup against session cookies leaking in.
- # See bug #66478
- #
- # TODO: Please check after 2014-09-13, and remove if
- # clients stopped sending sessionId.
- delete_if_exists_and_length_mismatches(capsule, 'country', 2)
- elif schema == 'NavigationTiming':
- if capsule['revision'] in [7494934, 8365252]:
- # Cleanup against session cookies leaking in.
- # See bug #66478
- #
- # TODO: Please check after 2014-09-13, and remove if
- # clients stopped sending sessionId.
- delete_if_exists_and_length_mismatches(capsule, 'originCountry', 2)
-
-
def validate(capsule):
"""Validates an encapsulated event.
:raises :exc:`jsonschema.ValidationError`: If event is invalid.
@@ -153,4 +77,3 @@
'Invalid revision ID: %(revision)s' % capsule)
schema = get_schema(scid, encapsulate=True)
jsonschema.Draft3Validator(schema).validate(capsule)
- post_validation_fixups(capsule)
diff --git a/server/tests/test_schema.py b/server/tests/test_schema.py
index d1d4643..2e17707 100644
--- a/server/tests/test_schema.py
+++ b/server/tests/test_schema.py
@@ -98,78 +98,3 @@
def test_empty_event(self):
"""An empty event with no mandatory properties should validate"""
self.assertIsValid(self.incorrectly_serialized_empty_event)
-
-
-class PostValidationFixupsTestCase(unittest.TestCase):
- """Tests for :module:`eventlogging.schema`s post validation fix-ups."""
-
- def test_delete_if_exists_and_length_mismatches_simple_match(self):
- """Validate a simple match of key and length."""
- capsule = {
- 'event': {
- 'foo': 'qux',
- }
- }
-
- eventlogging.schema.delete_if_exists_and_length_mismatches(
- capsule, 'foo', 3)
-
- self.assertEqual(capsule, {
- 'event': {
- 'foo': 'qux',
- }
- })
-
- def test_delete_if_exists_and_length_mismatches_match(self):
- """Validate a match for a not totally trivial capsule"""
- capsule = {
- 'event': {
- 'foo': 'qux',
- 'bar': 'quux',
- 'baz': 'quuz',
- },
- 'bar': 'quz',
- }
-
- eventlogging.schema.delete_if_exists_and_length_mismatches(
- capsule, 'bar', 4)
-
- self.assertEqual(capsule, {
- 'event': {
- 'foo': 'qux',
- 'bar': 'quux',
- 'baz': 'quuz',
- },
- 'bar': 'quz',
- })
-
- def test_delete_if_exists_and_length_mismatches_non_existing(self):
- """Validates that non existing keys are accepted."""
- capsule = {'event': {}}
-
- eventlogging.schema.delete_if_exists_and_length_mismatches(
- capsule, 'non_existing_field', 2)
-
- self.assertEqual(capsule, {'event': {}})
-
- def test_delete_if_exists_and_length_mismatches_different_length(self):
- """Validates removal of correct key upon length mismatch."""
- capsule = {
- 'event': {
- 'foo': 'qux',
- 'bar': 'quux',
- 'baz': 'quux',
- },
- 'bar': 'quz',
- }
-
- eventlogging.schema.delete_if_exists_and_length_mismatches(
- capsule, 'bar', 3)
-
- self.assertEqual(capsule, {
- 'event': {
- 'foo': 'qux',
- 'baz': 'quux',
- },
- 'bar': 'quz',
- })
--
To view, visit https://gerrit.wikimedia.org/r/176618
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I14fdd42cc98c70f683f851470f50a1cf22d1b470
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/EventLogging
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: QChris <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
