Introduce file diff table

Expand diff json column into separate table

Author: varmar05

server/mergin/sync/files.py

@@ -116,7 +116,9 @@ def create_obj(self, data, **kwargs):
 
 class ProjectFileSchema(FileSchema):
     mtime = DateTimeWithZ()
-    diff = fields.Nested(FileSchema())
+    diff = fields.Nested(
+        FileSchema(),
+    )
 
     @post_dump
     def patch_field(self, data, **kwargs):

server/mergin/sync/models.py

@@ -164,14 +164,18 @@ def files(self) -> List[ProjectFile]:
             SELECT
                 fp.path,
                 fh.size,
-                fh.diff,
                 fh.location,
                 fh.checksum,
-                pv.created AS mtime
+                pv.created AS mtime,
+                fd.path as diff_path,
+                fd.size as diff_size,
+                fd.checksum as diff_checksum,
+                fd.location as diff_location
             FROM files_ids
             LEFT OUTER JOIN file_history fh ON fh.id = files_ids.fh_id
             LEFT OUTER JOIN project_file_path fp ON fp.id = fh.file_path_id
-            LEFT OUTER JOIN project_version pv ON pv.id = fh.version_id;
+            LEFT OUTER JOIN project_version pv ON pv.id = fh.version_id
+            LEFT OUTER JOIN file_diff fd ON fd.file_path_id = fh.file_path_id AND fd.version = fh.project_version_name and fd.rank = 0;
         """
         params = {"project_id": self.id}
         files = [
@@ -181,7 +185,16 @@ def files(self) -> List[ProjectFile]:
                 checksum=row.checksum,
                 location=row.location,
                 mtime=row.mtime,
-                diff=File(**row.diff) if row.diff else None,
+                diff=(
+                    File(
+                        path=row.diff_path,
+                        size=row.diff_size,
+                        checksum=row.diff_checksum,
+                        location=row.diff_location,
+                    )
+                    if row.diff_path
+                    else None
+                ),
             )
             for row in db.session.execute(query, params).fetchall()
         ]
@@ -436,7 +449,6 @@ class FileHistory(db.Model):
     location = db.Column(db.String)
     size = db.Column(db.BigInteger, nullable=False)
     checksum = db.Column(db.String, nullable=False)
-    diff = db.Column(JSONB)
     change = db.Column(
         ENUM(
             *PushChangeType.values(),
@@ -470,17 +482,6 @@ class FileHistory(db.Model):
             file_path_id,
             project_version_name.desc(),
         ),
-        db.CheckConstraint(
-            text(
-                """
-                CASE
-                    WHEN (change = 'update_diff') THEN diff IS NOT NULL
-                    ELSE diff IS NULL
-                END
-                """
-            ),
-            name="changes_with_diff",
-        ),
     )
 
     def __init__(
@@ -491,22 +492,55 @@ def __init__(
         location: str,
         change: PushChangeType,
         diff: dict = None,
+        version_name: int = None,
     ):
         self.file = file
         self.size = size
         self.checksum = checksum
         self.location = location
-        self.diff = diff if diff is not None else null()
         self.change = change.value
+        self.project_version_name = version_name
+
+        if diff is not None:
+            basefile = FileHistory.get_basefile(file.id, version_name)
+            diff_file = FileDiff(
+                basefile,
+                diff.get("path"),
+                diff.get("size"),
+                diff.get("checksum"),
+                rank=0,
+                version=version_name,
+            )
+            db.session.add(diff_file)
 
     @property
     def path(self) -> str:
         return self.file.path
 
+    @property
+    def diff(self) -> Optional[FileDiff]:
+        """Diff file pushed with UPDATE_DIFF change type.
+
+        In FileDiff table it is defined as diff related to file, saved for the same project version with rank 0 (elementar diff)
+        """
+        if self.change != PushChangeType.UPDATE_DIFF.value:
+            return
+
+        return FileDiff.query.filter_by(
+            file_path_id=self.file_path_id, version=self.project_version_name, rank=0
+        ).first()
+
     @property
     def diff_file(self) -> Optional[File]:
-        if self.diff:
-            return File(**self.diff)
+        if not self.diff:
+            return
+
+        return File(
+            path=self.diff.path,
+            size=self.diff.size,
+            checksum=self.diff.checksum,
+            location=self.diff.location,
+        )
 
     @property
     def mtime(self) -> datetime:
@@ -518,7 +552,7 @@ def abs_path(self) -> str:
 
     @property
     def expiration(self) -> Optional[datetime]:
-        if not self.diff:
+        if not self.diff_file:
             return
 
         if os.path.exists(self.abs_path):
@@ -564,11 +598,7 @@ def changes(
                 break
 
             # if we are interested only in 'diffable' history (not broken with forced update)
-            if (
-                diffable
-                and item.change == PushChangeType.UPDATE.value
-                and not item.diff
-            ):
+            if diffable and item.change == PushChangeType.UPDATE.value:
                 break
 
         return history
@@ -615,7 +645,7 @@ def diffs_chain(
             if history:
                 first_change = history[-1]
                 # we have either full history of changes or v_x = v_x+n => no basefile in way, it is 'diffable' from the end
-                if first_change.diff:
+                if first_change.change == PushChangeType.UPDATE_DIFF.value:
                     # omit diff for target version as it would lead to previous version if reconstructed backward
                     diffs = [
                         value.diff_file
@@ -665,6 +695,75 @@ def diffs_chain(
 
         return basefile, diffs
 
+    @classmethod
+    def get_basefile(cls, file_path_id: int, version: int) -> Optional[FileHistory]:
+        """Get basefile (start of file diffable history) for diff file change at some version"""
+        return (
+            FileHistory.query.filter_by(file_path_id=file_path_id)
+            .filter(
+                FileHistory.project_version_name < version,
+                FileHistory.change.in_(
+                    [PushChangeType.CREATE.value, PushChangeType.UPDATE.value]
+                ),
+            )
+            .order_by(desc(FileHistory.project_version_name))
+            .first()
+        )
+
+
+class FileDiff(db.Model):
+    """File diffs related to versioned files, also contain higher order (rank) merged diffs"""
+
+    id = db.Column(db.BigInteger, primary_key=True, autoincrement=True)
+    file_path_id = db.Column(
+        db.BigInteger,
+        db.ForeignKey("project_file_path.id", ondelete="CASCADE"),
+        nullable=False,
+    )
+    # reference to actual full gpkg file
+    basefile_id = db.Column(
+        db.BigInteger,
+        db.ForeignKey("file_history.id", ondelete="CASCADE"),
+        index=True,
+        nullable=False,
+    )
+    path = db.Column(db.String, nullable=False, index=True)
+    # exponential order of merged diff, 0 is a source diff file uploaded by user, > 0 is merged diff
+    rank = db.Column(db.Integer, nullable=False, index=True)
+    # to which project version is this linked
+    version = db.Column(db.Integer, nullable=False, index=True)
+    # path on FS relative to project directory
+    location = db.Column(db.String)
+    size = db.Column(db.BigInteger, nullable=False)
+    checksum = db.Column(db.String, nullable=False)
+
+    __table_args__ = (
+        db.UniqueConstraint("file_path_id", "rank", "version", name="unique_diff"),
+        db.Index("ix_file_diff_file_path_id_version_rank", file_path_id, version, rank),
+    )
+
+    def __init__(
+        self,
+        basefile: FileHistory,
+        path: str,
+        size: int,
+        checksum: str,
+        rank: int,
+        version: int,
+    ):
+        self.basefile_id = basefile.id
+        self.file_path_id = basefile.file_path_id
+        self.path = path
+        self.size = size
+        self.checksum = checksum
+        self.rank = rank
+        self.version = version
+
+        if rank > 0:
+            self.location = f"diffs/{path}"
+        else:
+            self.location = f"v{version}/{path}"
+
 
 class ProjectVersion(db.Model):
     id = db.Column(db.Integer, primary_key=True, autoincrement=True)
@@ -760,11 +859,12 @@ def __init__(
                     diff=(
                         asdict(upload_file.diff)
                         if (is_diff_change and upload_file.diff)
-                        else null()
+                        else None
                     ),
                     change=(
                         PushChangeType.UPDATE_DIFF if is_diff_change else change_type
                     ),
+                    version_name=self.name,
                 )
                 fh.version = self
                 fh.project_version_name = self.name
@@ -822,14 +922,18 @@ def _files_from_start(self):
             SELECT
                 fp.path,
                 fh.size,
-                fh.diff,
                 fh.location,
                 fh.checksum,
-                pv.created AS mtime
+                pv.created AS mtime,
+                fd.path as diff_path,
+                fd.size as diff_size,
+                fd.checksum as diff_checksum,
+                fd.location as diff_location
             FROM latest_changes ch
             LEFT OUTER JOIN file_history fh ON (fh.file_path_id = ch.id AND fh.project_version_name = ch.version)
             LEFT OUTER JOIN project_file_path fp ON fp.id = fh.file_path_id
             LEFT OUTER JOIN project_version pv ON pv.id = fh.version_id
+            LEFT OUTER JOIN file_diff fd ON fd.file_path_id = fh.file_path_id AND fd.version = fh.project_version_name and fd.rank = 0
             WHERE fh.change != 'delete';
         """
         params = {"project_id": self.project_id, "version": self.name}
@@ -878,14 +982,18 @@ def _files_from_end(self):
             SELECT
                 fp.path,
                 fh.size,
-                fh.diff,
                 fh.location,
                 fh.checksum,
-                pv.created AS mtime
+                pv.created AS mtime,
+                fd.path as diff_path,
+                fd.size as diff_size,
+                fd.checksum as diff_checksum,
+                fd.location as diff_location
             FROM files_changes_before_version ch
             INNER JOIN file_history fh ON (fh.file_path_id = ch.file_id AND fh.project_version_name = ch.version)
             INNER JOIN project_file_path fp ON fp.id = fh.file_path_id
             INNER JOIN project_version pv ON pv.id = fh.version_id
+            LEFT OUTER JOIN file_diff fd ON fd.file_path_id = fh.file_path_id AND fd.version = fh.project_version_name and fd.rank = 0
             WHERE fh.change != 'delete'
             ORDER BY fp.path;
         """
@@ -909,7 +1017,16 @@ def files(self) -> List[ProjectFile]:
                 checksum=row.checksum,
                 location=row.location,
                 mtime=row.mtime,
-                diff=File(**row.diff) if row.diff else None,
+                diff=(
+                    File(
+                        path=row.diff_path,
+                        checksum=row.diff_checksum,
+                        size=row.diff_size,
+                        location=row.diff_location,
+                    )
+                    if row.diff_path
+                    else None
+                ),
             )
             for row in result
         ]

server/mergin/sync/public_api_controller.py

@@ -40,6 +40,7 @@
 from ..auth import auth_required
 from ..auth.models import User
 from .models import (
+    FileDiff,
     Project,
     ProjectVersion,
     Upload,

server/mergin/sync/tasks.py

@@ -89,7 +89,7 @@ def optimize_storage(project_id):
 
         for item in f_history:
             # no diffs, it is a basefile for geodiff
-            if not item.diff:
+            if not item.diff_file:
                 continue
 
             # skip the latest file version (high chance of being used)

server/mergin/sync/utils.py

@@ -345,9 +345,8 @@ def files_size():
             WHERE change = 'create'::push_change_type OR change = 'update'::push_change_type
             UNION
             SELECT
-                SUM(COALESCE((diff ->> 'size')::bigint, 0))
-            FROM file_history
-            WHERE change = 'update_diff'::push_change_type
+                SUM(size)
+            FROM file_diff
             UNION
             SELECT
                 SUM(size)

server/mergin/tests/test_project_controller.py

@@ -25,6 +25,7 @@
 from sqlalchemy import desc
 from ..app import db
 from ..sync.models import (
+    FileDiff,
     Project,
     Upload,
     ProjectVersion,
@@ -443,7 +444,7 @@ def test_add_project(client, app, data, expected):
         assert not any(file.diff for file in proj_files)
         assert not any(file.diff for file in pv.files)
         assert all(
-            item.change == PushChangeType.CREATE.value and not item.diff
+            item.change == PushChangeType.CREATE.value and not item.diff_file
             for item in pv.changes.all()
         )
         # cleanup
@@ -1597,7 +1598,7 @@ def test_push_no_diff_finish(client):
     file_meta = latest_version.changes.filter(
         FileHistory.change == PushChangeType.UPDATE_DIFF.value
     ).first()
-    assert file_meta.diff is not None
+    assert file_meta.diff_file is not None
     assert os.path.exists(
         os.path.join(upload.project.storage.project_dir, file_meta.diff_file.location)
     )
@@ -2370,7 +2371,8 @@ def test_version_files(client, diff_project):
             x.checksum == y.checksum
             and x.path == y.path
             and x.location == y.location
-            and x.diff == y.diff
+            and x.diff_path == y.diff_path
+            and x.diff_checksum == y.diff_checksum
             for x, y in zip(
                 sorted(forward_search, key=lambda f: f.path),
                 sorted(backward_search, key=lambda f: f.path),
@@ -2397,7 +2399,7 @@ def test_delete_diff_file(client):
         project_version_name=upload.project.latest_version,
         change=PushChangeType.UPDATE_DIFF.value,
     ).first()
-    assert fh.diff is not None
+    assert fh.diff_file is not None
 
     # delete file
     diff_change = next(
@@ -2424,7 +2426,7 @@ def test_delete_diff_file(client):
         project_version_name=upload.project.latest_version,
         change=PushChangeType.DELETE.value,
     ).first()
-    assert fh.path == "base.gpkg" and fh.diff is None
+    assert fh.path == "base.gpkg" and fh.diff_file is None
 
 
 def test_cache_files_ids(client):