From 8a49ad4586edc0539833aff4e6880849b3414f65 Mon Sep 17 00:00:00 2001 From: Lex Berezhny Date: Fri, 13 Dec 2019 10:44:41 -0500 Subject: [PATCH] catch invalid None value for all cases when creating/updating claims --- lbry/lbry/error/README.md | 1 + lbry/lbry/error/__init__.py | 6 +++++ lbry/lbry/schema/claim.py | 17 +++++++----- lbry/tests/unit/schema/test_models.py | 37 +++++++++++++++------------ 4 files changed, 38 insertions(+), 23 deletions(-) diff --git a/lbry/lbry/error/README.md b/lbry/lbry/error/README.md index f66ab4a9a..034a2032f 100644 --- a/lbry/lbry/error/README.md +++ b/lbry/lbry/error/README.md @@ -33,6 +33,7 @@ Code | Name | Message 105 | CommandPermanentlyUnavailable | Command '{command}' is permanently unavailable. -- such as when required component was intentionally configured not to start. **11x** | InputValue(ValueError) | Invalid argument value provided to command. 111 | GenericInputValue | The value '{value}' for argument '{argument}' is not valid. +112 | InputValueIsNone | None or null is not valid value for argument '{argument}'. **2xx** | Configuration | Configuration errors. 201 | ConfigWrite | Cannot write configuration file '{path}'. -- When writing the default config fails on startup, such as due to permission issues. 202 | ConfigRead | Cannot find provided configuration file '{path}'. -- Can't open the config file user provided via command line args. diff --git a/lbry/lbry/error/__init__.py b/lbry/lbry/error/__init__.py index a0c179d34..59f244092 100644 --- a/lbry/lbry/error/__init__.py +++ b/lbry/lbry/error/__init__.py @@ -61,6 +61,12 @@ class GenericInputValueError(InputValueError): super().__init__(f"The value '{value}' for argument '{argument}' is not valid.") +class InputValueIsNoneError(InputValueError): + + def __init__(self, argument): + super().__init__(f"None or null is not valid value for argument '{argument}'.") + + class ConfigurationError(BaseError): """ Configuration errors. diff --git a/lbry/lbry/schema/claim.py b/lbry/lbry/schema/claim.py index 41abb2e2d..860ffe432 100644 --- a/lbry/lbry/schema/claim.py +++ b/lbry/lbry/schema/claim.py @@ -16,6 +16,7 @@ from lbry.schema.attrs import ( LanguageList, LocationList, ClaimList, ClaimReference, TagList ) from lbry.schema.types.v2.claim_pb2 import Claim as ClaimMessage +from lbry.error import InputValueIsNoneError hachoir_log.use_print = False @@ -119,17 +120,19 @@ class BaseClaim: claim['locations'] = [l.to_dict() for l in self.locations] return claim + def none_check(self, kwargs): + for key, value in kwargs.items(): + if value is None: + raise InputValueIsNoneError(key) + def update(self, **kwargs): + self.none_check(kwargs) + for key in list(kwargs): for field in self.object_fields: if key.startswith(f'{field}_'): attr = getattr(self, field) - - attr_value = kwargs.pop(key) - if attr_value is None: - raise ValueError(f"Error updating claim - Null value provided for attribute {field}") - - setattr(attr, key[len(f'{field}_'):], attr_value) + setattr(attr, key[len(f'{field}_'):], kwargs.pop(key)) continue for l in self.repeat_fields: @@ -212,6 +215,8 @@ class Stream(BaseClaim): return claim def update(self, file_path=None, height=None, width=None, duration=None, **kwargs): + self.none_check(kwargs) + if kwargs.pop('clear_fee', False): self.message.ClearField('fee') else: diff --git a/lbry/tests/unit/schema/test_models.py b/lbry/tests/unit/schema/test_models.py index f8d834295..a2587d2f3 100644 --- a/lbry/tests/unit/schema/test_models.py +++ b/lbry/tests/unit/schema/test_models.py @@ -1,8 +1,8 @@ from unittest import TestCase -from unittest.mock import patch from decimal import Decimal -from lbry.schema.claim import Claim, Stream, Collection, BaseClaim +from lbry.schema.claim import Claim, Stream, Collection +from lbry.error import InputValueIsNoneError class TestClaimContainerAwareness(TestCase): @@ -198,19 +198,22 @@ class TestLocations(TestCase): self.assertEqual(stream.locations[0].country, 'UA') -class TestBaseClaimUpdates(TestCase): - - @patch('lbry.schema.claim.Claim') - def test_claim_update_url_ok(self, MockClaim): - # test if an attribute is provided correctly - base_claim = BaseClaim(MockClaim) - base_claim.update(thumbnail_url="somescheme:some/path") - self.assertEqual(base_claim.thumbnail.url, "somescheme:some/path") - - @patch('lbry.schema.claim.Claim') - def test_claim_update_url_error(self, MockClaim): - # test if an attribute is null - base_claim = BaseClaim(MockClaim) - with self.assertRaisesRegex(ValueError, "Error updating claim - Null value provided for attribute thumbnail"): - base_claim.update(thumbnail_url=None) +class TestStreamUpdating(TestCase): + def test_stream_update(self): + stream = Stream() + # each of these values is set differently inside of .update() + stream.update( + title="foo", + thumbnail_url="somescheme:some/path", + file_name="file-name" + ) + self.assertEqual(stream.title, "foo") + self.assertEqual(stream.thumbnail.url, "somescheme:some/path") + self.assertEqual(stream.source.name, "file-name") + with self.assertRaises(InputValueIsNoneError): + stream.update(title=None) + with self.assertRaises(InputValueIsNoneError): + stream.update(file_name=None) + with self.assertRaises(InputValueIsNoneError): + stream.update(thumbnail_url=None)