Class-based Validator with set_context - not thread safe

[AbstractVisions]

set_context method in Validation class - change internal state of callable instance in process.
DRF Field class override deepcopy for create shared "validators".
It means, that validators must be a ThreadSafe, but with set_context it is not so...

---

Note from @tomchristie: See also #3863, #3870

[xordoquy]

At first sight I'll second this analysis.
Validators do hold the instance while not copied which means that they indeed aren't thread safe.

[xordoquy]

Test case for the above issue (test_validators.py):

def test_is_thread_safe(self):
    from rest_framework.validators import UniqueValidator

    class UniquenessSerializer(serializers.ModelSerializer):
        username = serializers.CharField(max_length=100, validators=[UniqueValidator(queryset=UniquenessModel.objects.all())])

        class Meta:
            model = UniquenessModel

    data2 = {'username': 'other'}
    serializer2 = UniquenessSerializer(data=data2)
    data1 = {'username': 'existing'}
    serializer1 = UniquenessSerializer(data=data1)
    assert not serializer1.is_valid()
    assert serializer2.is_valid()
    validator1 = serializer1.fields['username'].validators[0]
    validator2 = serializer2.fields['username'].validators[0]
    assert isinstance(validator1, UniqueValidator)
    assert isinstance(validator2, UniqueValidator)
    assert id(validator1) != id(validator2)
    assert validator1.instance != validator2.instance

[AbstractVisions]

This feature used in UniqueValidator's and could make problems in some cases...
I think the simplest decision to exclude a mutable status - to make context injection in the functional style instead of assignment of properties. Same decision is close to Django ideology
For support of backward compatibility when using set_context it is possible to use as "self" every time the new created simple object and a binding its properties with descriptors.
If it is actual, I could prepare a concept of decision

[AbstractVisions]

The little strange test...
Django validators shall be immutabel (It even is reflected in comments: rest_framework/fields.py, "__deepcopy__" method of Field class) and can quite be identical in different objects

[xordoquy]

I don't think the context can be passed in a functional style.
I know it's more like what Django does but at the same time it makes things way harder for forms and fields for example.
Need to think again about this.

A few things to note:

• Validators are isolated as long as they are automatically extracted from the model constraints.
• Django validators are thread safe

The only possible case where this could be an issue is for a DRF explicitly set validator.

[AbstractVisions]

Indeed...ok. Then two simple ways seem:

1. good. set_context return the dict of context, that pass to __call__ method of Validator as addition parameter context=.... Pros - thread safe, Cons - that may be some backward compatibility issues. Backward compatibility problem can be solved with desctiptors
2. bad. create another "ugly hack" in __deepcopy__ method of DRF Field class:
   if specific validator has a set_context attribute, then use native deepcopy to it instead of sharing.
   And add to doc's anything like that: Validators with set_context mast be able to use 'deepcopy'
   Alternative approach: always pass serializer_field instance to __call__ method of Validator as addition parameter field=... and use context logic with it, then need for set_context disappears

[skyjur]

1. good. set_context return the dict of context, that pass to call method of Validator as addition parameter context=.... Pros - thread safe, Cons - that may be some backward compatibility issues. Backward compatibility problem can be solved with desctiptors

For me having "set_context/get_context" seems to be unecesary more complicated compared to passing serializer instance directly to validator function

[lovelydinosaur]

Tempting.