[Patchew-devel] [PATCH] models: avoid IntegrityError when clearing log property

[Paolo Bonzini]

On 28/11/18 16:40, Caio Carrara wrote:
>> @@ -109,18 +117,15 @@ class Result(models.Model):
>>  
>>      @log.setter
>>      def log(self, value):
>> -        entry = self.log_entry
>>          if value is None:
>> -            if entry is not None:
>> -                self.log_entry = None
> 
> What if you just call self.save() here before calling entry.delete(),
> whouldn't work?

Same answer as below...

>> -                entry.delete()
>> -        else:
>> -            if entry is None:
>> -                entry = LogEntry()
>> -            entry.data = value
>> -            entry.save()
>> -            if self.log_entry is None:
>> -                self.log_entry = entry
>> +            self.log_entry = None
> 
> Shouldn't you have to save the model before returning here?

... The idea is that "log" behaves as a field.  The caller is supposed
to do save() on the Result, we should not do it on their behalf.

>> +            return
>> +
>> +        entry = self.log_entry or LogEntry()
>> +        entry.data = value
>> +        entry.save()
> 
> The way the check is done here you can create a new log entry and do not
> associate it with the Result. Right? Because you're checking the current
> self.log_entry after creating the new one.

That cannot happen; self.log_entry will be None if and only if I created
a new LogEntry().

Would you prefer if I had

    entry = self.log_entry
    if entry is None:
         entry = self.log_entry = LogEntry()
    entry.data = value
    entry.save()

?

Paolo