Skip to content

Commit

Permalink
Model.save() returns SaveResult instead of bool
Browse files Browse the repository at this point in the history
  • Loading branch information
mesozoic committed Sep 5, 2024
1 parent 4c87441 commit 4fc0dce
Show file tree
Hide file tree
Showing 7 changed files with 186 additions and 23 deletions.
3 changes: 3 additions & 0 deletions docs/source/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ API: pyairtable.orm
.. autoclass:: pyairtable.orm.Model
:members:

.. autoclass:: pyairtable.orm.SaveResult
:members:


API: pyairtable.orm.fields
*******************************
Expand Down
8 changes: 8 additions & 0 deletions docs/source/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ Changelog
- `PR #373 <https://github.com/gtalarico/pyairtable/pull/373>`_
* Added command line utility and ORM module generator. See :doc:`cli`.
- `PR #376 <https://github.com/gtalarico/pyairtable/pull/376>`_
* Changed the behavior of :meth:`Model.save <pyairtable.orm.Model.save>`
to no longer send unmodified field values to the API.
- `PR #381 <https://github.com/gtalarico/pyairtable/pull/381>`_
* Added ``use_field_ids=`` parameter to :class:`~pyairtable.Api`.
- `PR #386 <https://github.com/gtalarico/pyairtable/pull/386>`_
* Changed the return type of :meth:`Model.save <pyairtable.orm.Model.save>`
from ``bool`` to :class:`~pyairtable.orm.SaveResult`.
- `PR #387 <https://github.com/gtalarico/pyairtable/pull/387>`_

2.3.3 (2024-03-22)
------------------------
Expand Down
4 changes: 4 additions & 0 deletions docs/source/migrations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ Changes to the ORM in 3.0
:data:`Model.created_time <pyairtable.orm.Model.created_time>` is now a ``datetime`` (or ``None``)
instead of ``str``. This change also applies to all timestamp fields used in :ref:`API: pyairtable.models`.

:meth:`Model.save <pyairtable.orm.Model.save>` now only saves changed fields to the API, which
means it will sometimes not perform any network traffic. It also now returns an instance of
:class:`~pyairtable.orm.SaveResult` instead of ``bool``. This behavior can be overridden.

The 3.0 release has changed the API for retrieving ORM model configuration:

.. list-table::
Expand Down
3 changes: 2 additions & 1 deletion pyairtable/orm/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from . import fields
from .model import Model
from .model import Model, SaveResult

__all__ = [
"Model",
"SaveResult",
"fields",
]
79 changes: 71 additions & 8 deletions pyairtable/orm/model.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import dataclasses
import datetime
import warnings
from dataclasses import dataclass
from functools import cached_property
from typing import (
Expand All @@ -10,6 +12,7 @@
List,
Mapping,
Optional,
Set,
Type,
Union,
cast,
Expand Down Expand Up @@ -225,7 +228,7 @@ def exists(self) -> bool:
"""
return bool(self.id)

def save(self, *, force: bool = False) -> bool:
def save(self, *, force: bool = False) -> "SaveResult":
"""
Save the model to the API.
Expand All @@ -235,10 +238,6 @@ def save(self, *, force: bool = False) -> bool:
Args:
force: If ``True``, all fields will be saved, even if they have not changed.
Returns:
``True`` if a record was created;
``False`` if it was updated, or if the model had no changes.
"""
if self._deleted:
raise RuntimeError(f"{self.id} was deleted")
Expand All @@ -250,11 +249,11 @@ def save(self, *, force: bool = False) -> bool:
self.id = record["id"]
self.created_time = datetime_from_iso_str(record["createdTime"])
self._changed.clear()
return True
return SaveResult(self.id, created=True, field_names=set(field_values))

if not force:
if not self._changed:
return False
return SaveResult(self.id)
field_values = {
field_name: value
for field_name, value in field_values.items()
Expand All @@ -263,7 +262,9 @@ def save(self, *, force: bool = False) -> bool:

self.meta.table.update(self.id, field_values, typecast=self.meta.typecast)
self._changed.clear()
return False
return SaveResult(
self.id, forced=force, updated=True, field_names=set(field_values)
)

def delete(self) -> bool:
"""
Expand Down Expand Up @@ -632,3 +633,65 @@ def request_kwargs(self) -> Dict[str, Any]:
"time_zone": None,
"use_field_ids": self.use_field_ids,
}


@dataclass
class SaveResult:
"""
Represents the result of saving a record to the API. The result's
attributes contain more granular information about the save operation:
>>> result = model.save()
>>> result.record_id
'recWPqD9izdsNvlE'
>>> result.created
False
>>> result.updated
True
>>> result.forced
False
>>> result.field_names
{'Name', 'Email'}
If none of the model's fields have changed, calling :meth:`~pyairtable.orm.Model.save`
will not perform any API requests and will return a SaveResult with no changes.
>>> model = YourModel()
>>> result = model.save()
>>> result.saved
True
>>> second_result = model.save()
>>> second_result.saved
False
For backwards compatibility, instances of SaveResult will evaluate as truthy
if the record was created, and falsy if the record was not created.
"""

record_id: RecordId
created: bool = False
updated: bool = False
forced: bool = False
field_names: Set[FieldName] = dataclasses.field(default_factory=set)

def __bool__(self) -> bool:
"""
Returns ``True`` if the record was created. This is for backwards compatibility
with the behavior of :meth:`~pyairtable.orm.Model.save` prior to the 3.0 release,
which returned a boolean indicating whether a record was created.
"""
warnings.warn(
"Model.save() now returns SaveResult instead of bool; switch"
" to checking Model.save().created instead before the 4.0 release.",
DeprecationWarning,
)
return self.created

@property
def saved(self) -> bool:
"""
Whether the record was saved to the API. If ``False``, this indicates there
were no changes to the model and the :meth:`~pyairtable.orm.Model.save`
operation was not forced.
"""
return self.created or self.updated
5 changes: 3 additions & 2 deletions tests/test_orm.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def test_model_basics():
# save
with mock.patch.object(Table, "create") as m_save:
m_save.return_value = {"id": "id", "createdTime": NOW}
contact.save()
assert contact.save().created

assert m_save.called
assert contact.id == "id"
Expand Down Expand Up @@ -163,7 +163,8 @@ def test_unmodified_field_not_saved(contact_record):

# Do not call update() if the record is unchanged
with mock_update_contact() as m_update:
contact.save()
result = contact.save()
assert not (result.created or result.updated)
m_update.assert_not_called()

# By default, only pass fields which were changed to the API
Expand Down
107 changes: 95 additions & 12 deletions tests/test_orm_model.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from datetime import datetime, timezone
from functools import partial
from unittest import mock

Expand All @@ -6,9 +7,22 @@

from pyairtable.orm import Model
from pyairtable.orm import fields as f
from pyairtable.orm.model import SaveResult
from pyairtable.testing import fake_id, fake_meta, fake_record


class FakeModel(Model):
Meta = fake_meta()
one = f.TextField("one")
two = f.TextField("two")


class FakeModelByIds(Model):
Meta = fake_meta(use_field_ids=True, table_name="Apartments")
Name = f.TextField("fld1VnoyuotSTyxW1")
Age = f.NumberField("fld2VnoyuotSTy4g6")


@pytest.fixture(autouse=True)
def no_requests(requests_mock):
"""
Expand Down Expand Up @@ -126,18 +140,6 @@ def test_model_overlapping(name):
)


class FakeModel(Model):
Meta = fake_meta()
one = f.TextField("one")
two = f.TextField("two")


class FakeModelByIds(Model):
Meta = fake_meta(use_field_ids=True, table_name="Apartments")
Name = f.TextField("fld1VnoyuotSTyxW1")
Age = f.NumberField("fld2VnoyuotSTy4g6")


def test_repr():
record = fake_record()
assert repr(FakeModel.from_record(record)) == f"<FakeModel id='{record['id']}'>"
Expand Down Expand Up @@ -375,3 +377,84 @@ class Meta:
assert f.meta.base_id == data["base_id"]
assert f.meta.table_name == data["table_name"]
Fake.Meta.table_name.assert_called_once()


@mock.patch("pyairtable.Table.create")
def test_save__create(mock_create):
"""
Test that we can save a model instance we've created.
"""
mock_create.return_value = {
"id": fake_id,
"createdTime": datetime.now(timezone.utc).isoformat(),
"fields": {"one": "ONE", "two": "TWO"},
}
obj = FakeModel(one="ONE", two="TWO")
result = obj.save()
assert result.saved
assert result.created
assert result.field_names == {"one", "two"}
assert not result.updated
assert not result.forced
mock_create.assert_called_once_with({"one": "ONE", "two": "TWO"}, typecast=True)


@mock.patch("pyairtable.Table.update")
def test_save__update(mock_update):
"""
Test that we can save a model instance that already exists.
"""
obj = FakeModel.from_record(fake_record(one="ONE", two="TWO"))
obj.one = "new value"
result = obj.save()
assert result.saved
assert not result.created
assert result.updated
assert result.field_names == {"one"}
assert not result.forced
mock_update.assert_called_once_with(obj.id, {"one": "new value"}, typecast=True)


@mock.patch("pyairtable.Table.update")
def test_save__update_force(mock_update):
"""
Test that we can save a model instance that already exists,
and we can force saving all values to the API.
"""
obj = FakeModel.from_record(fake_record(one="ONE", two="TWO"))
obj.one = "new value"
result = obj.save(force=True)
assert result.saved
assert not result.created
assert result.updated
assert result.forced
assert result.field_names == {"one", "two"}
mock_update.assert_called_once_with(
obj.id, {"one": "new value", "two": "TWO"}, typecast=True
)


@mock.patch("pyairtable.Table.update")
def test_save__noop(mock_update):
"""
Test that if a model is unchanged, we don't try to save it to the API.
"""
obj = FakeModel.from_record(fake_record(one="ONE", two="TWO"))
result = obj.save()
assert not result.saved
assert not result.created
assert not result.updated
assert not result.field_names
assert not result.forced
mock_update.assert_not_called()


def test_save_bool_deprecated():
"""
Test that SaveResult instances can be used as booleans, but emit a deprecation warning.
"""
with pytest.deprecated_call():
assert bool(SaveResult(fake_id(), created=False)) is False

with pytest.deprecated_call():
assert bool(SaveResult(fake_id(), created=True)) is True

0 comments on commit 4fc0dce

Please sign in to comment.