Can't use actual extras and convert_to_extras fields when using a custom schema

[amercader]

TL;DR You could not use actual extras and convert_to_extras fields when using a custom schema because the convert_to_extras function was not working as the validation expected.

Update: This now also includes logic to prevent using a free extra that has the same key as a field from the schema (which will allow to bypass validation)

Take the following custom schema implemented with IDatasetForm (this is similar to the one used on the example_idatasetform):

class MyPlugin(p.SingletonPlugin, p.toolkit.DefaultDatasetForm):

# ...

def create_package_schema
    schema = super(MyPlugin, self).create_package_schema()

    schema['custom_text'] = [ignore_missing, convert_to_extras]
    schema['custom_text2'] = [ignore_missing, convert_to_extras]

    return schema

# ...

If I call validate with this schema without any extras, all works as expected:

    data_dict = {
        'name': 'test-dataset',
        'custom_text': 'Hi',
        'custom_text2': 'Hi2',
    }       

    data, errors = validate(data_dict, schema, context)

    '''
    {'custom_text': 'Hi',
     'custom_text2': 'Hi2',
     'extras': [{'key': 'custom_text', 'value': 'Hi'},
                   {'key': 'custom_text2', 'value': 'Hi2'}],
     'name': u'test-dataset',
     'title': u'test-dataset'}
    '''  

One could argue that the fields should not be on the root any more, but that's another matter, we got them on the extras, which is where we needed them to get them stored.

Now, if we provide some "actual" extras in the data_dict things start to fail:

    data_dict = {
        'name': 'test-dataset',
        'custom_text': 'Hi',
        'custom_text2': 'Hi2',
        'extras': [
            {'key': 'proper_extra', 'value': 'Bye'},
        ]

    }

    data, errors = validate(data_dict, schema, context)

    '''
    {'custom_text': 'Hi',
     'custom_text2': 'Hi2',
     'extras': [{'key': 'proper_extra', 'value': 'Bye'}
                   {'key': 'custom_text2', 'value': 'Hi2'}],
     ],
     'name': u'test-dataset',
     'title': u'test-dataset'}
    '''

For every actual extra that you provide on the data_dict, one of the fields that were supposed to be converted to extras will get ignored and not put in extras, which means that they won't get stored (and validation will pass even if you were using not_empty or similar).

Even worse, if there are more actual extras than convert_to_extras fields the dictization will actually raise an exception:

    data_dict = {
        'name': 'test-dataset',
        'custom_text': 'Hi',
        'extras': [
            {'key': 'proper_extra', 'value': 'Bye'},
            {'key': 'proper_extra2', 'value': 'Bye2'},
        ]

    }

    data, errors = validate(data_dict, schema, context)

    '''
    File "/home/adria/dev/pyenvs/glasgow/src/ckan/ckan/lib/navl/dictization_functions.py", line 235, in validate
        converted_data = unflatten(converted_data)
    File "/home/adria/dev/pyenvs/glasgow/src/ckan/ckan/lib/navl/dictization_functions.py", line 399, in unflatten
        current_pos = current_pos[key]
    IndexError: list index out of range
    '''

The relevant code to understand what's going on is located in dictization_functions.py, in the validatefunction:

    flattened = flatten_dict(data)
    converted_data, errors = _validate(flattened, schema, context)
    converted_data = unflatten(converted_data)

If we look at converted_data just after calling _validate when validating a dict against a schema that uses convert_to_extras, we see that these extras are not added in a "flattened" way, but in an ('extras',) key:

    {('custom_text',): 'Hi',
     ('extras',): [{'key': 'custom_text', 'value': 'Hi'}],
     ('extras', 0, 'key'): u'proper_extra',
     ('extras', 0, 'value'): 'Bye',
     ('name',): u'test-dataset',
     ('title',): u'test-dataset'}

When "unflattening" things, unflatten will replace the n first elements of the ('extras') list with the n actual extras that were provided.

So it's pretty obvious that the culprit is convert_to_extras itself, which is not adding the new extras in an unflattened way, which is the one expected later on.

After applying the patch on this PR, this is how converted_data looks like before unflattening:

    {('custom_text',): 'Hi',
     ('extras', 0, 'key'): u'proper_extra',
     ('extras', 0, 'value'): 'Bye',
     ('extras', 1, 'key'): 'custom_text',
     ('extras', 1, 'value'): 'Hi',
     ('name',): u'test-dataset',
     ('title',): u'test-dataset'}

Which gives the expected:

    {'custom_text': 'Hi',
     'extras': [{'key': u'proper_extra', 'value': 'Bye'},
                {'key': 'custom_text', 'value': 'Hi'}],
     'name': u'test-dataset',
     'title': u'test-dataset'}

[joetsoi]

ping @amercader. merged.

[maxious]

Note: I got this {"key": ["There is a schema field with the same name"]} error because there actually was two active entries of the same extra key for the same package in the package_extra/package_extra_revision tables for 4 datasets (out of >5000). Easily fixed :)