Use internal naming convention

Author: ugexe

minfraud/request.py

@@ -13,7 +13,7 @@
 from voluptuous import MultipleInvalid
 
 from .errors import InvalidRequestError
-from .validation import validate_report, validate_transaction
+from .validation import _validate_report, _validate_transaction
 
 _TYPO_DOMAINS = {
     # gmail.com
@@ -268,7 +268,7 @@ def prepare_report(request: Dict[str, Any], validate: bool):
     cleaned_request = _copy_and_clean(request)
     if validate:
         try:
-            validate_report(cleaned_request)
+            _validate_report(cleaned_request)
         except MultipleInvalid as ex:
             raise InvalidRequestError(f"Invalid report data: {ex}") from ex
     return cleaned_request
@@ -283,7 +283,7 @@ def prepare_transaction(
     cleaned_request = _copy_and_clean(request)
     if validate:
         try:
-            validate_transaction(cleaned_request)
+            _validate_transaction(cleaned_request)
         except MultipleInvalid as ex:
             raise InvalidRequestError(f"Invalid transaction data: {ex}") from ex
 

minfraud/validation.py

@@ -15,7 +15,7 @@
 from typing import Optional
 
 from email_validator import validate_email  # type: ignore
-from voluptuous import All, Any, In, Match, Range, Required, Schema
+from voluptuous import All, Any, In, Match, MultipleInvalid, Range, Required, Schema
 from voluptuous.error import UrlInvalid
 
 # Pylint doesn't like the private function type naming for the callable
@@ -296,7 +296,7 @@ def _uri(s: str) -> str:
     return s
 
 
-validate_transaction = Schema(
+_validate_transaction = Schema(
     {
         "account": {
             "user_id": str,
@@ -385,7 +385,7 @@ def _transaction_id(s: Optional[str]) -> str:
     raise ValueError
 
 
-validate_report_schema = Schema(
+_validate_report_schema = Schema(
     {
         "chargeback_code": str,
         "ip_address": _ip_address,
@@ -398,16 +398,19 @@ def _transaction_id(s: Optional[str]) -> str:
 )
 
 
-def validate_at_least_one_identifier_field(report):
+def _validate_at_least_one_identifier_field(report):
     optional_fields = ["ip_address", "maxmind_id", "minfraud_id", "transaction_id"]
     if not any(field in report for field in optional_fields):
-        raise ValueError(
-            "The report must contain at least one of the following fields: 'ip_address', 'maxmind_id', 'minfraud_id', 'transaction_id'."
+        # We return MultipleInvalid instead of ValueError to be consistent with what
+        # voluptuous returns.
+        raise MultipleInvalid(
+            "The report must contain at least one of the following fields: "
+            "'ip_address', 'maxmind_id', 'minfraud_id', 'transaction_id'."
         )
     return True
 
 
-def validate_report(report):
-    validate_report_schema(report)
-    validate_at_least_one_identifier_field(report)
+def _validate_report(report):
+    _validate_report_schema(report)
+    _validate_at_least_one_identifier_field(report)
     return True

tests/test_validation.py

@@ -1,7 +1,7 @@
 from decimal import Decimal
 from voluptuous import MultipleInvalid
 
-from minfraud.validation import validate_transaction, validate_report
+from minfraud.validation import _validate_transaction, _validate_report
 
 import unittest
 
@@ -17,12 +17,12 @@ def setup_transaction(self, transaction):
     def check_invalid_transaction(self, transaction):
         self.setup_transaction(transaction)
         with self.assertRaises(MultipleInvalid, msg=f"{transaction} is invalid"):
-            validate_transaction(transaction)
+            _validate_transaction(transaction)
 
     def check_transaction(self, transaction):
         self.setup_transaction(transaction)
         try:
-            validate_transaction(transaction)
+            _validate_transaction(transaction)
         except MultipleInvalid as e:
             self.fail(f"MultipleInvalid {e.msg} thrown for {transaction}")
 
@@ -55,15 +55,15 @@ def check_invalid_report(self, report):
 
     def check_invalid_report_no_setup(self, report):
         with self.assertRaises(MultipleInvalid, msg=f"{report} is invalid"):
-            validate_report(report)
+            _validate_report(report)
 
     def check_report(self, report):
         self.setup_report(report)
         self.check_report_no_setup(report)
 
     def check_report_no_setup(self, report):
         try:
-            validate_report(report)
+            _validate_report(report)
         except MultipleInvalid as e:
             self.fail(f"MultipleInvalid {e.msg} thrown for {report}")
 
@@ -79,7 +79,7 @@ def test_transaction_without_device(self):
                 "user_id": "usr",
             }
         }
-        validate_transaction(transaction)
+        _validate_transaction(transaction)
 
 
 class TestAccount(unittest.TestCase, ValidationBase):
@@ -252,8 +252,8 @@ def test_ip_address(self):
             self.check_invalid_transaction({"device": {"ip_address": invalid}})
 
     def test_missing_ip(self):
-        validate_transaction({"device": {}})
-        validate_transaction(
+        _validate_transaction({"device": {}})
+        _validate_transaction(
             {
                 "device": {
                     "user_agent": "foo",
@@ -262,7 +262,7 @@ def test_missing_ip(self):
         )
 
     def test_missing_device(self):
-        validate_transaction({})
+        _validate_transaction({})
 
     def test_user_agent(self):
         self.check_transaction_str_type("device", "user_agent")
@@ -439,6 +439,8 @@ def test_tag(self):
             self.check_invalid_report({"tag": bad})
 
     def test_report_valid_identifier(self):
+        self.check_invalid_report_no_setup({"tag": "chargeback"})
+
         self.check_report_no_setup({"tag": "chargeback", "ip_address": "1.1.1.1"})
         self.check_report_no_setup(
             {"tag": "chargeback", "minfraud_id": "58fa38d8-4b87-458b-a22b-f00eda1aa20d"}