Bug 1011053 - Criteria.as_dict() says its output can be used as input to Criteria.from_client_input(), but it fails validation
Criteria.as_dict() says its output can be used as input to Criteria.from_clie...
Status: CLOSED CURRENTRELEASE
Product: Pulp
Classification: Community
Component: z_other (Show other bugs)
Master
All All
unspecified Severity medium
: ---
: 2.4.0
Assigned To: Randy Barlow
Preethi Thomas
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-09-23 10:30 EDT by Randy Barlow
Modified: 2014-08-09 02:55 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-08-09 02:55:50 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Randy Barlow 2013-09-23 10:30:43 EDT
The Criteria.as_dict() docblock says that its output is "compatible as input to the from_client_input method." However, if you create a Criteria object, serialize it with as_dict(), and then try to create a new one with from_client_input(), it will fail validation.

For example, I've converted the consumer applicability regeneration code to use this method to send the input through a message broker, and on the other end I get this traceback:

Traceback (most recent call last):
  File "/home/rbarlow/devel/pulp/server/test/unit/test_server_managers_consumer_applicability.py", line 170, in test_regenerate_applicability_for_consumer_criteria_no_bindings
    manager.regenerate_applicability_for_consumers(self.CONSUMER_CRITERIA)
  File "/home/rbarlow/devel/pulp/server/pulp/server/managers/consumer/applicability.py", line 44, in regenerate_applicability_for_consumers
    consumer_criteria = Criteria.from_client_input(consumer_criteria)
  File "/home/rbarlow/devel/pulp/server/pulp/server/db/model/criteria.py", line 80, in from_client_input
    sort = _validate_sort(doc.pop('sort', None))
  File "/home/rbarlow/devel/pulp/server/pulp/server/db/model/criteria.py", line 318, in _validate_sort
    if not isinstance(entry[0], basestring):
KeyError: 0

This happens because the sort attribute looks like this:

[{'id': 1}]

It's already been converted from user input to a dictionary, but the validation code is assuming that it's user input (i.e., using words like "ascending").
Comment 1 Randy Barlow 2013-09-23 10:43:15 EDT
We could make a from_dict() method that is able to to consume the output of the to_dict() method as a solution to this bug. Then the from_client_input() method could make use of from_dict() after doing its validation and type conversion.

This will be important for our distributed task conversion work.
Comment 2 Randy Barlow 2013-09-23 10:46:39 EDT
We could also consider not using the Criteria model anymore, and just use dictionaries. As far as I can tell, this is pretty much just a data structure that can convert itself from client input and to dictionaries. It does also have a "spec" property, but it doesn't have a docblock so I'm not very clear about what it does.

If we can get rid of this class, I think it will make some of our code more maintainable and easier to understand to to greater simplicity.
Comment 3 Randy Barlow 2013-09-26 14:39:50 EDT
I am fixing this by making a from_dict().
Comment 4 Randy Barlow 2013-09-27 10:23:28 EDT
https://github.com/pulp/pulp/pull/635
Comment 5 Randy Barlow 2014-03-18 12:04:43 EDT
This was fixed in a prior release of Pulp, but we never put it through the QE process. Moving to ON_QA.
Comment 6 Randy Barlow 2014-06-26 09:38:01 EDT
Preethi,

This bug is difficult to verify using Pulp's features, as it is an internal refactor that was necessary for the new tasks to work in 2.4.0. There is a way to verify that it works in a Python shell:

rbarlow@guava ~/d/p/pulp> python
Python 2.7.6 (default, May 27 2014, 16:39:58) 
[GCC 4.9.0 20140518 (Red Hat 4.9.0-5)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from pulp.server.db.model.criteria import Criteria
>>> c1 = Criteria(filters={'name': 'test'})
>>> c1
{'sort': None, 'skip': None, 'limit': None, 'filters': {'name': 'test'}, 'fields': None, '_id': ObjectId('53ac21a5e5147d1119496316'), 'id': '53ac21a5e5147d1119496316'}
>>> c2 = Criteria.from_dict(c1.as_dict())
>>> c2
{'sort': None, 'skip': None, 'limit': None, 'filters': {'name': 'test'}, 'fields': None, '_id': ObjectId('53ac21bfe5147d1119496317'), 'id': '53ac21bfe5147d1119496317'}

If you can create c2 from c1, using c1's as_dict() method and Criteria's from_dict() static method as shown above, this bug is solved.
Comment 7 Preethi Thomas 2014-06-27 08:38:00 EDT
verified

[root@yttrium ~]# rpm -qa pulp-server
pulp-server-2.4.0-0.23.beta.el6.noarch
[root@yttrium ~]# 

[root@yttrium ~]# python
Python 2.6.6 (r266:84292, Sep  4 2013, 07:46:00) 
[GCC 4.4.7 20120313 (Red Hat 4.4.7-3)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from pulp.server.db.model.criteria import Criteria
>>> c1 = Criteria(filters={'name': 'test'})
>>> c1
{'sort': None, 'skip': None, 'limit': None, 'filters': {'name': 'test'}, 'fields': None, '_id': ObjectId('53ad650a7bcdb34a1e2a20eb'), 'id': '53ad650a7bcdb34a1e2a20eb'}
>>> c2 = Criteria.from_dict(c1.as_dict())
>>> c2
{'sort': None, 'skip': None, 'limit': None, 'filters': {'name': 'test'}, 'fields': None, '_id': ObjectId('53ad65277bcdb34a1e2a20ec'), 'id': '53ad65277bcdb34a1e2a20ec'}
>>>
Comment 8 Randy Barlow 2014-08-09 02:55:50 EDT
This has been fixed in Pulp 2.4.0-1.

Note You need to log in before you can comment on or make changes to this bug.