Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions morango/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from django.db.models.expressions import CombinedExpression
from django.db.models.fields.related import ForeignKey
from django.db.models.functions import Cast
from django.db.models.functions import NullIf
from django.utils import timezone
from django.utils.functional import cached_property

Expand Down Expand Up @@ -440,10 +441,30 @@ def char_ids_list(self):
.values_list("fixed_id", flat=True)
)

def filter_deserialization_error(self, has_error: bool) -> "StoreQueryset":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: This is a verbatim extraction of operations.py:326-336, comment included, but that call site still hand-rolls the annotation. Swapping it for store_models.exclude_has_deserialization_error() is one line and keeps the two from drifting — #317's "do not modify the existing deserialization process" is about behaviour, and this is behaviour-preserving.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code in operations.py will eventually be removed. There is no need to update it.

"""
Filters the queryset to return Store records that have or have not any
deserialization errors
"""
# this nullIf assertion is generally more performant than an OR'd statement on
# unindexed columns, in both SQLite and PostgreSQL
return self.annotate(
_deserialization_error=NullIf(
F("deserialization_error"), Value(""), output_field=models.TextField()
)
).filter(_deserialization_error__isnull=not has_error)

def filter_has_deserialization_error(self) -> "StoreQueryset":
"""Filters the queryset to return Store records that have deserialization errors"""
return self.filter_deserialization_error(True)

def exclude_has_deserialization_error(self) -> "StoreQueryset":
"""Filters the queryset to return Store records that have no deserialization error"""
return self.filter_deserialization_error(False)


class StoreManager(models.Manager):
def get_queryset(self):
return StoreQueryset(self.model, using=self._db)
class StoreManager(models.Manager.from_queryset(StoreQueryset)):
pass


class Store(AbstractStore):
Expand Down
12 changes: 12 additions & 0 deletions morango/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,18 @@ def get_model_querysets(self, profile) -> Generator[QuerySet, None, None]:
queryset = queryset.order_by(*self._get_nulls_last_ordering(ordering))
yield queryset

def get_store_querysets(self, profile) -> Generator[QuerySet, None, None]:
"""
Complementary method to `get_model_querysets` but for Store querysets
"""
from morango.models.core import Store

for model in self.get_models(profile):
store_qs = Store.objects.filter(profile=profile, model_name=model.morango_model_name)
if self.get_self_referential_fk(model) is not None:
store_qs = store_qs.order_by(*self._get_nulls_last_ordering(("_self_ref_order",)))
yield store_qs

@staticmethod
def _get_nulls_last_ordering(ordering):
normalized = []
Expand Down
87 changes: 87 additions & 0 deletions morango/sync/stream/deserialize.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
from typing import Dict
from typing import Generator
from typing import List
from typing import Optional
from typing import Type

from morango.models.certificates import Filter
from morango.models.core import Store
from morango.models.core import SyncableModel
from morango.registry import syncable_models
from morango.sync.stream.source import MorangoSource
from morango.sync.stream.source import SourceTask


class DeserializeTask(SourceTask):
"""Carrier class for providing context through the deserialization pipeline."""

__slots__ = ("store", "app_model", "fk_cache", "errors")

def __init__(self, store: Store, fk_cache: Dict):
self.store = store
self.fk_cache: Dict = fk_cache
self.app_model: Optional[SyncableModel] = None
self.errors: List[Exception] = []

@property
def id(self) -> str:
return self.store.id

@property
def model(self) -> Type[SyncableModel]:
return syncable_models.get_model(self.store.profile, self.store.model_name)

@property
def has_errors(self) -> bool:
return len(self.errors) > 0

def set_app_model(self, app_model: Optional[SyncableModel]) -> None:
self.app_model = app_model

def add_error(self, error: Exception) -> None:
self.errors.append(error)


class StoreModelSource(MorangoSource[DeserializeTask]):
"""
Yields ``DeserializeTask`` objects for dirty store models that match the optional
*sync_filter*.
"""

def __init__(
self,
profile: str,
sync_filter: Optional[Filter] = None,
dirty_only: bool = True,
partition_order: str = "asc",
fk_cache: Optional[Dict] = None,
skip_errored: bool = False,
):
"""
:param profile: The Morango model profile
:param sync_filter: The Filter object for this sync
:param dirty_only: Whether to filter on dirty records only
:param partition_order: Controls how the filter specificity is applied, "asc" or "desc"
:param fk_cache: Dictionary cache for FK references
:param skip_errored: Whether to skip Store records with deserialization errors
"""
super().__init__(profile, sync_filter, dirty_only, partition_order)
self.fk_cache = fk_cache or {}
self.skip_errored = skip_errored

def stream_for_filter(
self, partition_condition: Optional[str]
) -> Generator[DeserializeTask, None, None]:
# the registry yields models in foreign key dependency order, so streaming model by model
# ensures a record's foreign key targets are deserialized before it is
for store_qs in syncable_models.get_store_querysets(self.profile):
qs = store_qs
if partition_condition is not None:
qs = qs.filter(partition__startswith=partition_condition)
if self.dirty_only:
qs = qs.filter(dirty_bit=True)
if self.skip_errored:
qs = qs.exclude_has_deserialization_error()

for store_model in qs.iterator():
yield DeserializeTask(store_model, self.fk_cache)
69 changes: 18 additions & 51 deletions morango/sync/stream/serialize.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from typing import Type

from django.core.serializers.json import DjangoJSONEncoder
from django.db.models import Q

from morango.models.certificates import Filter
from morango.models.core import DatabaseMaxCounter
Expand All @@ -19,14 +18,15 @@
from morango.registry import syncable_models
from morango.sync.stream.core import Buffer
from morango.sync.stream.core import Sink
from morango.sync.stream.core import Source
from morango.sync.stream.core import Transform
from morango.sync.stream.core import Unbuffer
from morango.sync.stream.source import MorangoSource
from morango.sync.stream.source import SourceTask

logger = logging.getLogger(__name__)


class SerializeTask(object):
class SerializeTask(SourceTask):
"""Carrier class for providing context through the pipeline"""

__slots__ = (
Expand All @@ -47,6 +47,10 @@ def __init__(self, model: Type[SyncableModel], obj: SyncableModel):
self._self_ref_fk_value: Optional[str] = None
self._self_ref_order: Optional[int] = None

@property
def id(self) -> str:
return self.obj.id

@property
def is_store_update(self):
return self.store is not None and not self.store._state.adding
Expand Down Expand Up @@ -80,59 +84,22 @@ def set_self_ref_order(self, value: Optional[int]):
self._self_ref_order = value


class AppModelSource(Source[SerializeTask]):
class AppModelSource(MorangoSource[SerializeTask]):
"""
Yields ``SerializeTask`` objects for every syncable-model record that matches the
optional *sync_filter*.
"""

def __init__(
self,
profile: str,
sync_filter: Optional[Filter] = None,
dirty_only: bool = True,
partition_order: str = "asc",
):
"""
:param profile: The Morango model profile
:param sync_filter: The Filter object for this sync
:param dirty_only: Whether to filter on dirty records only
:param partition_order: Controls how the filter specificity is applied, "asc" or "desc"
"""
self.profile = profile
self.sync_filter = sync_filter
self.dirty_only = dirty_only
self.partition_order = partition_order
self._seen = set()

def prefix_conditions(self) -> Generator[Optional[Q], None, None]:
if self.sync_filter is None:
# yield None once, so we do one query without a partition filter (everything)
yield None
else:
partitions_prefixes = [str(prefix) for prefix in self.sync_filter]
partition_iterator = sorted(
partitions_prefixes,
reverse=self.partition_order == "desc",
)

for prefix in partition_iterator:
yield Q(_morango_partition__startswith=prefix)

def stream(self) -> Generator[SerializeTask, None, None]:
for partition_condition in self.prefix_conditions():
for qs in syncable_models.get_model_querysets(self.profile):
if partition_condition is not None:
qs = qs.filter(partition_condition)
if self.dirty_only:
qs = qs.filter(_morango_dirty_bit=True)
for obj in qs.iterator():
# partition filtering could result in overlaps, and since we're walking
# through the partitions one by one, we should avoid duplicates. Morango
# syncable models have unique IDs across the entire profile
if obj.id not in self._seen:
self._seen.add(obj.id)
yield SerializeTask(qs.model, obj)
def stream_for_filter(
self, partition_condition: Optional[str]
) -> Generator[SerializeTask, None, None]:
for qs in syncable_models.get_model_querysets(self.profile):
if partition_condition is not None:
qs = qs.filter(_morango_partition__startswith=partition_condition)
if self.dirty_only:
qs = qs.filter(_morango_dirty_bit=True)
for obj in qs.iterator():
yield SerializeTask(qs.model, obj)


class StoreLookup(Transform[List[SerializeTask]]):
Expand Down
100 changes: 100 additions & 0 deletions morango/sync/stream/source.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import abc
from typing import Generator
from typing import Iterator
from typing import Optional
from typing import TypeVar

from morango.models.certificates import Filter
from morango.sync.stream.core import Source


class SourceTask(abc.ABC):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: SourceTask declares no __slots__, so it contributes a __dict__ to every subclass and SerializeTask/DeserializeTask's own slots become inert. SerializeTask had working slots before this refactor.

Verified on this class shape:

t = T(); hasattr(t, "__dict__")  # True
t.zzz = 5                        # succeeds

One task instance exists per record streamed, in a pipeline whose stated point is "reducing memory overhead" (core.py:1-6). abc.ABC itself sets __slots__ = (); the same line on SourceTask restores both the memory saving and the typo-catching.

"""Typing for source object passed through streaming pipeline"""

__slots__ = ()

@property
@abc.abstractmethod
def id(self) -> str:
pass


T = TypeVar("T", bound=SourceTask)


class MorangoSource(Source[T], abc.ABC):
"""
Common source functionality for Morango sources, such as SyncableModels and Store records.
"""

def __init__(
self,
profile: str,
sync_filter: Optional[Filter] = None,
dirty_only: bool = True,
partition_order: str = "asc",
):
"""
:param profile: The Morango model profile
:param sync_filter: The Filter object for this sync
:param dirty_only: Whether to filter on dirty records only
:param partition_order: Controls how the filter specificity is applied, "asc" or "desc"
"""
self.profile = profile
self.sync_filter = sync_filter
self.dirty_only = dirty_only
self.partition_order = partition_order
self._seen = set()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: _seen accumulates one entry per record for the source's lifetime, even when nothing can duplicate. With sync_filter is None, prefix_conditions() yields exactly one None, so stream_for_filter runs once and no id can repeat — but a full-store pass still holds every Store id in memory, which is the cost the streaming design exists to avoid.

Skipping the bookkeeping when there is a single prefix condition covers that case. Also note _seen is never reset, so a second stream() call on the same instance yields nothing.


def prefix_conditions(self) -> Generator[Optional[str], None, None]:
"""
Generates partition prefixes for queries based on the sync filter and partition order.

This method outputs prefixes in sorted order according to the specified partition
order. If no sync filter is provided, it yields `None` to indicate a query
without filtering by partition.

:return: A generator yielding partition prefixes or `None` if no filtering is applied.
"""
if self.sync_filter is None:
# yield None once, so we do one query without a partition filter (everything)
yield None
else:
partitions_prefixes = [str(prefix) for prefix in self.sync_filter]
partition_iterator = sorted(
partitions_prefixes,
reverse=self.partition_order == "desc",
)

for prefix in partition_iterator:
yield prefix

def stream(self) -> Generator[T, None, None]:
"""
Streams unique objects based on prefix conditions. This generator method iterates over
partition conditions defined in the sync_filter and passes through to `stream_for_filter`
to stream back objects, ensuring that only objects with unique `id` values are yielded.

:return: A generator yielding unique objects.
"""
for partition_condition in self.prefix_conditions():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Partition-major iteration drops the model-dependency ordering deserialization relies on.

This loops prefixes outer, models inner, so emission is prefix1 × (model1..modelN), then prefix2 × (...). The comment at deserialize.py:71-72 claims streaming model by model ensures FK targets come first — that only holds within a single prefix pass.

_deserialize_from_store is the opposite shape: models outer (operations.py:303) with every prefix OR'd into one partition__startswith condition (operations.py:313-319), so dependency order is global across the whole filter. Here, a modelN record in prefix1 whose FK target is a model1 record living only in prefix2 is emitted before its target.

Ordering is irrelevant for serialization, which is presumably why the base landed this way. For deserialization it is the guarantee. Either invert the loops in StoreModelSource (models outer, prefixes OR'd inner, matching the existing code), or drop the comment's claim and document that cross-prefix FK targets are unordered.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no cross-partition FKs. The ordering is a naive approach that works with known filters, which prioritize the shortest filters first, which are likely the least specific.

for obj in self.stream_for_filter(partition_condition):
# partition filtering could result in overlaps, and since we're walking
# through the partitions one by one, we should avoid duplicates. Morango
# syncable models and store records have unique IDs across the entire profile
if obj.id not in self._seen:
# without sync filters, we do not need to worry about repeating objects
if self.sync_filter is not None:
self._seen.add(obj.id)
yield obj

@abc.abstractmethod
def stream_for_filter(self, partition_condition: Optional[str]) -> Iterator[T]:
"""
This method is intended to generate an iterator that yields data based on the given
filtering condition.

:param partition_condition: A string representing a partition filter prefix condition
:return: An iterator yielding items
"""
pass
Loading
Loading