diff --git a/openstack/block_storage/v2/_proxy.py b/openstack/block_storage/v2/_proxy.py index 2ae9b5642..0cd12a39b 100644 --- a/openstack/block_storage/v2/_proxy.py +++ b/openstack/block_storage/v2/_proxy.py @@ -758,10 +758,13 @@ def get_limits(self, project=None): :class:`~openstack.block_storage.v2.limits.RateLimit` :rtype: :class:`~openstack.block_storage.v2.limits.Limits` """ - params = {} if project: - params['project_id'] = resource.Resource._get_id(project) - return self._get(_limits.Limits, requires_id=False, **params) + return self._get( + _limits.Limits, + requires_id=False, + project_id=resource.Resource._get_id(project), + ) + return self._get(_limits.Limits, requires_id=False) # ========== Capabilities ========== diff --git a/openstack/network/v2/_proxy.py b/openstack/network/v2/_proxy.py index ee2ed815a..aa3c5f338 100644 --- a/openstack/network/v2/_proxy.py +++ b/openstack/network/v2/_proxy.py @@ -193,11 +193,10 @@ class Proxy(proxy.Proxy): "vpn_service": _vpn_service.VpnService, } - @proxy._check_resource(strict=False) def _update( self, resource_type: type[resource.ResourceT], - value: ty.Union[str, resource.ResourceT], + value: ty.Union[str, resource.ResourceT, None], base_path: ty.Optional[str] = None, if_revision: ty.Optional[int] = None, **attrs: ty.Any, @@ -212,11 +211,10 @@ def _update( return res.commit(self, base_path=base_path) - @proxy._check_resource(strict=False) def _delete( self, resource_type: type[resource.ResourceT], - value: ty.Union[str, resource.ResourceT], + value: ty.Union[str, resource.ResourceT, None], ignore_missing: bool = True, if_revision: ty.Optional[int] = None, **attrs: ty.Any, @@ -3594,10 +3592,10 @@ def delete_qos_minimum_packet_rate_rule( rule belongs or a :class:`~openstack.network.v2.qos_policy.QoSPolicy` instance. :param bool ignore_missing: When set to ``False`` - :class:`~openstack.exceptions.NotFoundException` will be raised when - the resource does not exist. When set to ``True``, no exception - will be set when attempting to delete a nonexistent minimum packet - rate rule. + :class:`~openstack.exceptions.NotFoundException` will be raised + when the resource does not exist. When set to ``True``, no + exception will be set when attempting to delete a nonexistent + minimum packet rate rule. :returns: ``None`` """ diff --git a/openstack/proxy.py b/openstack/proxy.py index e16aa8e3f..6ca840ab6 100644 --- a/openstack/proxy.py +++ b/openstack/proxy.py @@ -36,37 +36,6 @@ from openstack import connection -# The _check_resource decorator is used on Proxy methods to ensure that -# the `actual` argument is in fact the type of the `expected` argument. -# It does so under two cases: -# 1. When strict=False, if and only if `actual` is a Resource instance, -# it is checked to see that it's an instance of the `expected` class. -# This allows `actual` to be other types, such as strings, when it makes -# sense to accept a raw id value. -# 2. When strict=True, `actual` must be an instance of the `expected` class. -def _check_resource(strict=False): - def wrap(method): - def check(self, expected, actual=None, *args, **kwargs): - if ( - strict - and actual is not None - and not isinstance(actual, resource.Resource) - ): - raise ValueError(f"A {expected.__name__} must be passed") - elif isinstance(actual, resource.Resource) and not isinstance( - actual, expected - ): - raise ValueError( - f"Expected {expected.__name__} but received {actual.__class__.__name__}" - ) - - return method(self, expected, actual, *args, **kwargs) - - return check - - return wrap - - def normalize_metric_name(name): name = name.replace('.', '_') name = name.replace(':', '_') @@ -471,6 +440,11 @@ class if using an existing instance, or ``utils.Munch``, res = resource_type.new(id=value, connection=conn, **attrs) else: # An existing resource instance + if not isinstance(value, resource_type): + raise ValueError( + f'Expected {resource_type.__name__} but received ' + f'{value.__class__.__name__}' + ) res = value res._update(**attrs) @@ -529,6 +503,8 @@ def _find( ) -> ty.Optional[resource.ResourceT]: """Find a resource + :param resource_type: The type of resource to find. This should be a + :class:`~openstack.resource.Resource` subclass. :param name_or_id: The name or ID of a resource to find. :param bool ignore_missing: When set to ``False`` :class:`~openstack.exceptions.NotFoundException` will be @@ -545,22 +521,22 @@ def _find( self, name_or_id, ignore_missing=ignore_missing, **attrs ) - @_check_resource(strict=False) def _delete( self, resource_type: type[resource.ResourceT], - value: ty.Union[str, resource.ResourceT], + value: ty.Union[str, resource.ResourceT, None], ignore_missing: bool = True, **attrs: ty.Any, ) -> ty.Optional[resource.ResourceT]: """Delete a resource - :param resource_type: The type of resource to delete. This should - be a :class:`~openstack.resource.Resource` - subclass with a ``from_id`` method. - :param value: The value to delete. Can be either the ID of a - resource or a :class:`~openstack.resource.Resource` - subclass. + :param resource_type: The type of resource to delete. This should be a + :class:`~openstack.resource.Resource` subclass. + :param value: The resource to delete. This can be the ID of a resource, + a :class:`~openstack.resource.Resource` subclass instance, or None + for resources that don't have their own identifier or have + identifiers with multiple parts. If None, you must pass these other + identifiers as kwargs. :param bool ignore_missing: When set to ``False`` :class:`~openstack.exceptions.NotFoundException` will be raised when the resource does not exist. @@ -587,21 +563,22 @@ def _delete( return rv - @_check_resource(strict=False) def _update( self, resource_type: type[resource.ResourceT], - value: ty.Union[str, resource.ResourceT], + value: ty.Union[str, resource.ResourceT, None], base_path: ty.Optional[str] = None, **attrs: ty.Any, ) -> resource.ResourceT: """Update a resource - :param resource_type: The type of resource to update. - :type resource_type: :class:`~openstack.resource.Resource` - :param value: The resource to update. This must either be a - :class:`~openstack.resource.Resource` or an id - that corresponds to a resource. + :param resource_type: The type of resource to update. This should be a + :class:`~openstack.resource.Resource` subclass. + :param value: The resource to update. This can be the ID of a resource, + a :class:`~openstack.resource.Resource` subclass instance, or None + for resources that don't have their own identifier or have + identifiers with multiple parts. If None, you must pass these other + identifiers as kwargs. :param str base_path: Base part of the URI for updating resources, if different from :data:`~openstack.resource.Resource.base_path`. @@ -626,13 +603,10 @@ def _create( ) -> resource.ResourceT: """Create a resource from attributes - :param resource_type: The type of resource to create. - :type resource_type: :class:`~openstack.resource.Resource` - :param str base_path: Base part of the URI for creating resources, if - different from - :data:`~openstack.resource.Resource.base_path`. - :param path_args: A dict containing arguments for forming the request - URL, if needed. + :param resource_type: The type of resource to create. This should be a + :class:`~openstack.resource.Resource` subclass. + :param base_path: Base part of the URI for creating resources, if + different from :data:`~openstack.resource.Resource.base_path`. :param dict attrs: Attributes to be passed onto the :meth:`~openstack.resource.Resource.create` method to be created. These should correspond @@ -662,9 +636,9 @@ def _bulk_create( ) -> ty.Generator[resource.ResourceT, None, None]: """Create a resource from attributes - :param resource_type: The type of resource to create. - :type resource_type: :class:`~openstack.resource.Resource` - :param list data: List of attributes dicts to be passed onto the + :param resource_type: The type of resource to create. This should be a + :class:`~openstack.resource.Resource` subclass. + :param data: List of attributes dicts to be passed onto the :meth:`~openstack.resource.Resource.create` method to be created. These should correspond to either :class:`~openstack.resource.Body` @@ -679,7 +653,6 @@ def _bulk_create( """ return resource_type.bulk_create(self, data, base_path=base_path) - @_check_resource(strict=False) def _get( self, resource_type: type[resource.ResourceT], @@ -691,17 +664,20 @@ def _get( ) -> resource.ResourceT: """Fetch a resource - :param resource_type: The type of resource to get. - :type resource_type: :class:`~openstack.resource.Resource` - :param value: The value to get. Can be either the ID of a - resource or a :class:`~openstack.resource.Resource` - subclass. - :param str base_path: Base part of the URI for fetching resources, if + :param resource_type: The type of resource to get. This should be a + :class:`~openstack.resource.Resource` subclass. + :param value: The resource to get. This can be the ID of a resource, + a :class:`~openstack.resource.Resource` subclass instance, or None + for resources that don't have their own identifier or have + identifiers with multiple parts. If None, you must pass these other + identifiers as kwargs. + :param requires_id: Whether the resource is identified by an ID or not. + :param base_path: Base part of the URI for fetching resources, if different from :data:`~openstack.resource.Resource.base_path`. - :param bool skip_cache: A boolean indicating whether optional API + :param skip_cache: A boolean indicating whether optional API cache should be skipped for this invocation. - :param dict attrs: Attributes to be passed onto the + :param attrs: Attributes to be passed onto the :meth:`~openstack.resource.Resource.get` method. These should correspond to either :class:`~openstack.resource.Body` @@ -787,7 +763,7 @@ def _head( :param resource_type: The type of resource to retrieve. :type resource_type: :class:`~openstack.resource.Resource` - :param value: The value of a specific resource to retreive headers + :param value: The value of a specific resource to retrieve headers for. Can be either the ID of a resource, a :class:`~openstack.resource.Resource` subclass, or ``None``. diff --git a/openstack/tests/unit/test_proxy.py b/openstack/tests/unit/test_proxy.py index 9cc02d60b..431d70d77 100644 --- a/openstack/tests/unit/test_proxy.py +++ b/openstack/tests/unit/test_proxy.py @@ -73,48 +73,6 @@ def method(self, expected_type, value): self.fake_proxy = proxy.Proxy(self.session) self.fake_proxy._connection = self.cloud - def _test_correct(self, value): - decorated = proxy._check_resource(strict=False)(self.sot.method) - rv = decorated(self.sot, resource.Resource, value) - - self.assertEqual(value, rv) - - def test__check_resource_correct_resource(self): - res = resource.Resource() - self._test_correct(res) - - def test__check_resource_notstrict_id(self): - self._test_correct("abc123-id") - - def test__check_resource_strict_id(self): - decorated = proxy._check_resource(strict=True)(self.sot.method) - self.assertRaisesRegex( - ValueError, - "A Resource must be passed", - decorated, - self.sot, - resource.Resource, - "this-is-not-a-resource", - ) - - def test__check_resource_incorrect_resource(self): - class OneType(resource.Resource): - pass - - class AnotherType(resource.Resource): - pass - - value = AnotherType() - decorated = proxy._check_resource(strict=False)(self.sot.method) - self.assertRaisesRegex( - ValueError, - "Expected OneType but received AnotherType", - decorated, - self.sot, - OneType, - value, - ) - def test__get_uri_attribute_no_parent(self): class Child(resource.Resource): something = resource.Body("something")