Issue: save_optimistic misbehaving when foreign key is assigned before saved
Author: NicolasCaous
State: closed
Created: 2019-02-15 16:52:03

I'm not sure if this is intended behavior or a valid issue. @coleifer please help :)

# The Issue

Assume [Optimistic Locking](http://docs.peewee-orm.com/en/3.6.0/peewee/hacks.html#optimistic-locking) is in place as in the docs

When a foreign key is assigned before it exists on the database, it results in a None (aka NULL) reference after `_prune_fields` call. This should be a valid sequence of events, but, for some reason, its not:

```
>>> from peewee import *
>>>
>>> db = SqliteDatabase(':memory:')
>>>
>>> class BaseModel(Model):
...     class Meta:
...         database = db
... 
>>>
>>> class Phone(BaseModel):
...     number = CharField(max_length=255)
...
>>> class Person(BaseModel):
...     phone = ForeignKeyField(Phone, backref="person", null=True)
...
>>> db.connect()
True
>>> db.create_tables([Phone, Person])
>>>
>>> person = Person()
>>> person
<Person: None>
>>> person.save()
1
>>> person
<Person: 1>
>>>
>>> phone = Phone(number="dummy")
>>> phone
<Phone: None>
>>> person.phone = phone
>>>
>>> person.phone
<Phone: None>
>>> person._prune_fields(dict(person.__data__), person.dirty_fields) # Simulating "save_optimistic", here phone should be None, so it's correct.
{'phone': None}
>>> phone.save() # Now save
1
>>> phone # Save was succesfull
<Phone: 1>
>>> person.phone # Relation is also correct
<Phone: 1>
>>> person._prune_fields(dict(person.__data__), person.dirty_fields) # Phone should be 1, not None now, incorrect behavior
{'phone': None}
>>> 
>>> 
>>> person2 = Person()
>>> person2
<Person: None>
>>> person2.save()
1
>>> person2
<Person: 2>
>>> 
>>> phone2 = Phone(number="dummy")
>>> phone2
<Phone: None>
>>> phone2.save() # Simulating save before assing
1
>>> phone2
<Phone: 2>
>>> person2.phone = phone2
>>> person2._prune_fields(dict(person2.__data__), person2.dirty_fields) # now it's correct
{'phone': 2}

```

For copy and paste:
```
from peewee import *

db = SqliteDatabase(':memory:')

class BaseModel(Model):
    class Meta:
        database = db

class Phone(BaseModel):
    number = CharField(max_length=255)

class Person(BaseModel):
     phone = ForeignKeyField(Phone, backref="person", null=True)

db.connect()
db.create_tables([Phone, Person])

person = Person()
person
person.save()
person

phone = Phone(number="dummy")
phone
person.phone = phone
person.phone
person._prune_fields(dict(person.__data__), person.dirty_fields) # Simulating "save_optimistic", here phone should be None, so it's correct.
phone.save() # Now save
phone # Save was succesfull
person.phone # Relation is also correct
person._prune_fields(dict(person.__data__), person.dirty_fields) # Phone should be 1, not None now, incorrect behavior

person2 = Person()
person2
person2.save()
person2
phone2 = Phone(number="dummy")
phone2
phone2.save() # Simulating save before assing
phone2
person2.phone = phone2
person2._prune_fields(dict(person2.__data__), person2.dirty_fields) # now it's correct
```

# Peewee Version

```
$ python -c "from peewee import __version__; print(__version__)"
3.8.0
```

Happens with any database (Tested with Sqlite and MySQL)

Well, for what its worth, this seems like a very silly and inefficient way to use Peewee... As you are essentially doing two INSERTs and an UPDATE when you only need to be doing two INSERTs. I'd suggest that the right way would be to first create the `Phone` object, then to create the `Person`...not creating the `Person`, `Phone`, then updating the `Person`.

However I agree this is probably a bug.

Here's a fix:

```python
        # Update any data that has changed and bump the version counter.
        field_data = dict(self.__data__)
        current_version = field_data.pop('version', 1)
        self._populate_unsaved_relations(field_data)  # ADD THIS LINE.
        field_data = self._prune_fields(field_data, self.dirty_fields)
        if not field_data:
            raise ValueError('No changes have been made.')
```

> this seems like a very silly and inefficient way to use Peewee...

I strongly agree :laughing: , this is just a Proof of Concept. I ran into this issue in a totally different context. Since it was a complex scenario, I decided to create this PoC just for the issue for simplicity sake.

> [If you have found a bug in the code and submit a failing test-case, then hats-off to you, you are a hero!](url)

A hero once again!