From a427e184fb02f4108424448b045f90187291dcb0 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 11 Jun 2020 17:00:50 +0200
Subject: [PATCH] Allow negative_utc to be None in normalize_timestamp()

thus in TimestampWithTimezone.from_dict(). This is needed to help consuming
existing (invalid) messages from kafka.

Warning: tests added in this revision do not cover the whole
normalize_timestamp() function.
---
 swh/model/identifiers.py            |   2 +
 swh/model/tests/test_identifiers.py | 107 ++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+)

diff --git a/swh/model/identifiers.py b/swh/model/identifiers.py
index 4d25184b..4fe45bd3 100644
--- a/swh/model/identifiers.py
+++ b/swh/model/identifiers.py
@@ -295,6 +295,8 @@ def normalize_timestamp(time_representation):
         offset = time_representation["offset"]
         if "negative_utc" in time_representation:
             negative_utc = time_representation["negative_utc"]
+        if negative_utc is None:
+            negative_utc = False
     elif isinstance(time_representation, datetime.datetime):
         seconds = int(time_representation.timestamp())
         microseconds = time_representation.microsecond
diff --git a/swh/model/tests/test_identifiers.py b/swh/model/tests/test_identifiers.py
index da0e2aa5..fac86bd7 100644
--- a/swh/model/tests/test_identifiers.py
+++ b/swh/model/tests/test_identifiers.py
@@ -5,6 +5,7 @@
 
 import binascii
 import datetime
+import pytest
 import unittest
 
 from swh.model import hashutil, identifiers
@@ -17,6 +18,7 @@ from swh.model.identifiers import (
     REVISION,
     SNAPSHOT,
     PersistentId,
+    normalize_timestamp,
 )
 
 
@@ -963,3 +965,108 @@ class OriginIdentifier(unittest.TestCase):
             identifiers.origin_identifier(self.origin),
             "b63a575fe3faab7692c9f38fb09d4bb45651bb0f",
         )
+
+
+TS_DICTS = [
+    (
+        {"timestamp": 12345, "offset": 0},
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 0},
+            "offset": 0,
+            "negative_utc": False,
+        },
+    ),
+    (
+        {"timestamp": 12345, "offset": 0, "negative_utc": False},
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 0},
+            "offset": 0,
+            "negative_utc": False,
+        },
+    ),
+    (
+        {"timestamp": 12345, "offset": 0, "negative_utc": False},
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 0},
+            "offset": 0,
+            "negative_utc": False,
+        },
+    ),
+    (
+        {"timestamp": 12345, "offset": 0, "negative_utc": None},
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 0},
+            "offset": 0,
+            "negative_utc": False,
+        },
+    ),
+    (
+        {"timestamp": {"seconds": 12345}, "offset": 0, "negative_utc": None},
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 0},
+            "offset": 0,
+            "negative_utc": False,
+        },
+    ),
+    (
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 0},
+            "offset": 0,
+            "negative_utc": None,
+        },
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 0},
+            "offset": 0,
+            "negative_utc": False,
+        },
+    ),
+    (
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 100},
+            "offset": 0,
+            "negative_utc": None,
+        },
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 100},
+            "offset": 0,
+            "negative_utc": False,
+        },
+    ),
+    (
+        {"timestamp": 12345, "offset": 0, "negative_utc": True},
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 0},
+            "offset": 0,
+            "negative_utc": True,
+        },
+    ),
+    (
+        {"timestamp": 12345, "offset": 0, "negative_utc": None},
+        {
+            "timestamp": {"seconds": 12345, "microseconds": 0},
+            "offset": 0,
+            "negative_utc": False,
+        },
+    ),
+]
+
+
+@pytest.mark.parametrize("dict_input,expected", TS_DICTS)
+def test_normalize_timestamp_dict(dict_input, expected):
+    assert normalize_timestamp(dict_input) == expected
+
+
+TS_DICTS_INVALID_TIMESTAMP = [
+    {"timestamp": 1.2, "offset": 0},
+    {"timestamp": "1", "offset": 0},
+    # these below should really also trigger a ValueError...
+    # {"timestamp": {"seconds": "1"}, "offset": 0},
+    # {"timestamp": {"seconds": 1.2}, "offset": 0},
+    # {"timestamp": {"seconds": 1.2}, "offset": 0},
+]
+
+
+@pytest.mark.parametrize("dict_input", TS_DICTS_INVALID_TIMESTAMP)
+def test_normalize_timestamp_dict_invalid_timestamp(dict_input):
+    with pytest.raises(ValueError, match="non-integer timestamp"):
+        normalize_timestamp(dict_input)
-- 
GitLab