Skip to content

Make pygeodiff required for the client #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wonder-sk merged 7 commits into from
Jun 17, 2021
Merged

Make pygeodiff required for the client #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d9872bb
Require geodiff to be always present
wonder-sk May 5, 2021
7046695
Require geodiff to be always present
wonder-sk May 5, 2021
5df81c1
Require `pygeodiff` for the client.
erpas Jun 11, 2021
fb7a7ba
Merge remote-tracking branch 'origin/robustness-fixes' into robustnes…
erpas Jun 11, 2021
df5c9d9
Revert unneeded test changes.
erpas Jun 11, 2021
c03c33f
Avoid copying of versioned files
erpas Jun 15, 2021
348d77d
Avoid copying of versioned files continued
erpas Jun 16, 2021
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
3 changes: 1 addition & 2 deletions Pipfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ name = "pypi"

[packages]
python-dateutil = "==2.6.0"
pygeodiff = "==0.8.6"
pygeodiff = "==1.0.0"
pytz = "==2019.3"


[dev-packages]
pytest = "==3.10.1"
pytest-cov = "==2.6.1"
Expand Down
228 changes: 126 additions & 102 deletions Pipfile.lock
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions mergin/client_pull.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,7 @@ def apply(self, directory, mp):
file_to_merge.merge()

if mp.is_versioned_file(self.file_path):
shutil.copy(mp.fpath(self.file_path), mp.fpath_meta(self.file_path))

mp.geodiff.make_copy_sqlite(mp.fpath(self.file_path), mp.fpath_meta(self.file_path))


class DownloadQueueItem:
Expand Down
104 changes: 44 additions & 60 deletions mergin/merginproject.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from datetime import datetime
from dateutil.tz import tzlocal

from .common import UPLOAD_CHUNK_SIZE, InvalidProject
from .common import UPLOAD_CHUNK_SIZE, InvalidProject, ClientError
from .utils import generate_checksum, move_file, int_version, find, do_sqlite_checkpoint


Expand All @@ -22,10 +22,7 @@
try:
from .deps import pygeodiff
except ImportError:
try:
import pygeodiff
except ImportError:
os.environ['GEODIFF_ENABLED'] = 'False'
import pygeodiff


class MerginProject:
Expand All @@ -38,33 +35,31 @@ def __init__(self, directory):
if not os.path.exists(self.dir):
raise InvalidProject('Project directory does not exist')

# make sure we can load correct pygeodiff
if os.environ.get('GEODIFF_ENABLED', 'True').lower() == 'true':
try:
self.geodiff = pygeodiff.GeoDiff()
except pygeodiff.geodifflib.GeoDiffLibVersionError:
self.geodiff = None
else:
self.geodiff = None

self.meta_dir = os.path.join(self.dir, '.mergin')
if not os.path.exists(self.meta_dir):
os.mkdir(self.meta_dir)

self.setup_logging(directory)

# make sure we can load correct pygeodiff
try:
self.geodiff = pygeodiff.GeoDiff()
except pygeodiff.geodifflib.GeoDiffLibVersionError:
# this is a fatal error, we can't live without geodiff
self.log.error("Unable to load geodiff! (lib version error)")
raise ClientError("Unable to load geodiff library!")

# redirect any geodiff output to our log file
if self.geodiff:
def _logger_callback(level, text_bytes):
text = text_bytes.decode() # convert bytes to str
if level == pygeodiff.GeoDiff.LevelError:
self.log.error("GEODIFF: " + text)
elif level == pygeodiff.GeoDiff.LevelWarning:
self.log.warning("GEODIFF: " + text)
else:
self.log.info("GEODIFF: " + text)
self.geodiff.set_logger_callback(_logger_callback)
self.geodiff.set_maximum_logger_level(pygeodiff.GeoDiff.LevelDebug)
def _logger_callback(level, text_bytes):
text = text_bytes.decode() # convert bytes to str
if level == pygeodiff.GeoDiff.LevelError:
self.log.error("GEODIFF: " + text)
elif level == pygeodiff.GeoDiff.LevelWarning:
self.log.warning("GEODIFF: " + text)
else:
self.log.info("GEODIFF: " + text)
self.geodiff.set_logger_callback(_logger_callback)
self.geodiff.set_maximum_logger_level(pygeodiff.GeoDiff.LevelDebug)

def setup_logging(self, logger_name):
"""Setup logging into project directory's .mergin/client-log.txt file."""
Expand Down Expand Up @@ -127,8 +122,6 @@ def is_versioned_file(self, file):
:returns: if file is compatible with geodiff lib
:rtype: bool
"""
if not self.geodiff:
return False
diff_extensions = ['.gpkg', '.sqlite']
f_extension = os.path.splitext(file)[1]
return f_extension in diff_extensions
Expand Down Expand Up @@ -247,14 +240,13 @@ def get_pull_changes(self, server_files):
:returns: changes metadata for files to be pulled from server
:rtype: dict
"""

# first let's have a look at the added/updated/removed files
changes = self.compare_file_sets(self.metadata['files'], server_files)
if not self.geodiff:
self.log.warning("geodiff is not available!")
return changes

# then let's inspect our versioned files (geopackages) if there are any relevant changes
not_updated = []
for file in changes['updated']:
# for small geodiff files it does not make sense to download diff and then apply it (slow)
if not self.is_versioned_file(file["path"]):
continue

Expand Down Expand Up @@ -310,10 +302,6 @@ def get_push_changes(self):
file["checksum"] = checksum
file['chunks'] = [str(uuid.uuid4()) for i in range(math.ceil(file["size"] / UPLOAD_CHUNK_SIZE))]

if not self.geodiff:
self.log.warning("geodiff is not available!")
return changes

# need to check for for real changes in geodiff files using geodiff tool (comparing checksum is not enough)
not_updated = []
for file in changes['updated']:
Expand Down Expand Up @@ -374,7 +362,7 @@ def apply_pull_changes(self, changes, temp_dir):
Update project files according to file changes. Apply changes to geodiff basefiles as well
so they are up to date with server. In case of conflicts create backups from locally modified versions.

.. seealso:: self.pull_changes
.. seealso:: self.get_pull_changes

:param changes: metadata for pulled files
:type changes: dict[str, list[dict]]
Expand Down Expand Up @@ -410,18 +398,17 @@ def apply_pull_changes(self, changes, temp_dir):

# temporary backup of file pulled from server for recovery
f_server_backup = self.fpath(f'{path}-server_backup', temp_dir)
shutil.copy(src, f_server_backup)
self.geodiff.make_copy_sqlite(src, f_server_backup)

# create temp backup (ideally with geodiff) of locally modified file if needed later
f_conflict_file = self.fpath(f'{path}-local_backup', temp_dir)
try:
self.geodiff.create_changeset(basefile, dest, local_diff)
shutil.copy(basefile, f_conflict_file)
self.geodiff.make_copy_sqlite(basefile, f_conflict_file)
self.geodiff.apply_changeset(f_conflict_file, local_diff)
except (pygeodiff.GeoDiffLibError, pygeodiff.GeoDiffLibConflictError):
self.log.warning("backup of local file with geodiff failed - need to do hard copy (dangerous!)")
# FIXME hard copy can lead to data loss if changes from -wal file were not flushed !!!
shutil.copy(dest, f_conflict_file)
self.log.info("backup of local file with geodiff failed - need to do hard copy")
self.geodiff.make_copy_sqlite(dest, f_conflict_file)

# in case there will be any conflicting operations found during rebase,
# they will be stored in a JSON file - if there are no conflicts, the file
Expand All @@ -438,17 +425,12 @@ def apply_pull_changes(self, changes, temp_dir):
except (pygeodiff.GeoDiffLibError, pygeodiff.GeoDiffLibConflictError) as err:
self.log.warning("rebase failed! going to create conflict file")
# it would not be possible to commit local changes, they need to end up in new conflict file
shutil.copy(f_conflict_file, dest) # revert file
self.geodiff.make_copy_sqlite(f_conflict_file, dest)
conflict = self.backup_file(path)
conflicts.append(conflict)
# original file synced with server
shutil.copy(f_server_backup, basefile)
shutil.copy(f_server_backup, dest)
# changes in -wal have been already applied in conflict file or LOST (see above)
if os.path.exists(f'{dest}-wal'):
os.remove(f'{dest}-wal')
if os.path.exists(f'{dest}-shm'):
os.remove(f'{dest}-shm')
self.geodiff.make_copy_sqlite(f_server_backup, basefile)
self.geodiff.make_copy_sqlite(f_server_backup, dest)
else:
# The local file is not modified -> no rebase needed.
# We just apply the diff between our copy and server to both the local copy and its basefile
Expand All @@ -464,12 +446,11 @@ def apply_pull_changes(self, changes, temp_dir):
self.geodiff.apply_changeset(basefile, server_diff)
self.log.info("update successful")
except (pygeodiff.GeoDiffLibError, pygeodiff.GeoDiffLibConflictError):
self.log.warning("update failed! going to copy file (dangerous!)")
self.log.warning("update failed! going to copy file")
# something bad happened and we have failed to patch our local files - this should not happen if there
# wasn't a schema change or something similar that geodiff can't handle.
# FIXME: this is a last resort and may corrupt data! (we should warn user)
shutil.copy(src, dest)
shutil.copy(src, basefile)
self.geodiff.make_copy_sqlite(src, dest)
self.geodiff.make_copy_sqlite(src, basefile)
else:
# backup if needed
if path in modified and item['checksum'] != local_files_map[path]['checksum']:
Expand All @@ -485,9 +466,11 @@ def apply_pull_changes(self, changes, temp_dir):
if self.is_versioned_file(path):
os.remove(basefile)
else:
shutil.copy(src, dest)
if self.is_versioned_file(path):
shutil.copy(src, basefile)
self.geodiff.make_copy_sqlite(src, dest)
self.geodiff.make_copy_sqlite(src, basefile)
else:
shutil.copy(src, dest)

return conflicts

Expand All @@ -498,8 +481,6 @@ def apply_push_changes(self, changes):
:param changes: metadata for pulled files
:type changes: dict[str, list[dict]]
"""
if not self.geodiff:
return
for k, v in changes.items():
for item in v:
path = item['path']
Expand All @@ -510,12 +491,12 @@ def apply_push_changes(self, changes):
if k == 'removed':
os.remove(basefile)
elif k == 'added':
shutil.copy(self.fpath(path), basefile)
self.geodiff.make_copy_sqlite(self.fpath(path), basefile)
elif k == 'updated':
# in case for geopackage cannot be created diff (e.g. forced update with committed changes from wal file)
if "diff" not in item:
self.log.info("updating basefile (copy) for: " + path)
shutil.copy(self.fpath(path), basefile)
self.geodiff.make_copy_sqlite(self.fpath(path), basefile)
else:
self.log.info("updating basefile (diff) for: " + path)
# better to apply diff to previous basefile to avoid issues with geodiff tmp files
Expand Down Expand Up @@ -545,7 +526,10 @@ def backup_file(self, file):
while os.path.exists(backup_path):
backup_path = self.fpath(f'{file}_conflict_copy{index}')
index += 1
shutil.copy(src, backup_path)
if self.is_versioned_file(file):
self.geodiff.make_copy_sqlite(src, backup_path)
else:
shutil.copy(src, backup_path)
return backup_path

def apply_diffs(self, basefile, diffs):
Expand Down
70 changes: 13 additions & 57 deletions mergin/test/test_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@
CHANGED_SCHEMA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'modified_schema')


def toggle_geodiff(enabled):
os.environ['GEODIFF_ENABLED'] = str(enabled)


@pytest.fixture(scope='function')
def mc():
return create_client(API_USER, USER_PWD)
Expand Down Expand Up @@ -282,52 +278,34 @@ def test_ignore_files(mc):
assert not next((f for f in project_info['files'] if f['path'] == '.directory'), None)


# (diffs size limit, push geodiff enabled, pull geodiff enabled)
diff_test_scenarios = [
(True, True),
(True, False),
(False, True),
(False, False),
]

def test_sync_diff(mc):

@pytest.mark.parametrize("push_geodiff_enabled, pull_geodiff_enabled", diff_test_scenarios)
def test_sync_diff(mc, push_geodiff_enabled, pull_geodiff_enabled):

test_project = f'test_sync_diff_push{int(push_geodiff_enabled)}_pull{int(pull_geodiff_enabled)}'
test_project = f'test_sync_diff'
project = API_USER + '/' + test_project
project_dir = os.path.join(TMP_DIR, test_project) # primary project dir for updates
project_dir_2 = os.path.join(TMP_DIR, test_project + '_2') # concurrent project dir with no changes
project_dir_3 = os.path.join(TMP_DIR, test_project + '_3') # concurrent project dir with local changes

cleanup(mc, project, [project_dir, project_dir_2, project_dir_3])
# create remote project
toggle_geodiff(push_geodiff_enabled)
shutil.copytree(TEST_DATA_DIR, project_dir)
mc.create_project_and_push(test_project, project_dir)

# make sure we have v1 also in concurrent project dirs
toggle_geodiff(pull_geodiff_enabled)
mc.download_project(project, project_dir_2)
mc.download_project(project, project_dir_3)

# test push changes with diffs:
toggle_geodiff(push_geodiff_enabled)
mp = MerginProject(project_dir)
f_updated = 'base.gpkg'
# step 1) base.gpkg updated to inserted_1_A (inserted A feature)
if push_geodiff_enabled:
shutil.move(mp.fpath(f_updated), mp.fpath_meta(f_updated)) # make local copy for changeset calculation
shutil.move(mp.fpath(f_updated), mp.fpath_meta(f_updated)) # make local copy for changeset calculation
shutil.copy(mp.fpath('inserted_1_A.gpkg'), mp.fpath(f_updated))
mc.push_project(project_dir)
if push_geodiff_enabled:
mp.geodiff.create_changeset(mp.fpath(f_updated), mp.fpath_meta(f_updated), mp.fpath_meta('push_diff'))
assert not mp.geodiff.has_changes(mp.fpath_meta('push_diff'))
else:
assert not os.path.exists(mp.fpath_meta(f_updated))
mp.geodiff.create_changeset(mp.fpath(f_updated), mp.fpath_meta(f_updated), mp.fpath_meta('push_diff'))
assert not mp.geodiff.has_changes(mp.fpath_meta('push_diff'))
# step 2) base.gpkg updated to inserted_1_A_mod (modified 2 features)
if push_geodiff_enabled:
shutil.move(mp.fpath(f_updated), mp.fpath_meta(f_updated))
shutil.move(mp.fpath(f_updated), mp.fpath_meta(f_updated))
shutil.copy(mp.fpath('inserted_1_A_mod.gpkg'), mp.fpath(f_updated))
# introduce some other changes
f_removed = 'inserted_1_B.gpkg'
Expand All @@ -343,48 +321,27 @@ def test_sync_diff(mc, push_geodiff_enabled, pull_geodiff_enabled):
assert next((f for f in project_info['files'] if f['path'] == 'renamed.gpkg'), None)
assert not next((f for f in project_info['files'] if f['path'] == f_removed), None)
assert not os.path.exists(mp.fpath_meta(f_removed))
if push_geodiff_enabled:
assert 'diff' in f_remote
assert os.path.exists(mp.fpath_meta('renamed.gpkg'))
else:
assert 'diff' not in f_remote
assert not os.path.exists(mp.fpath_meta('renamed.gpkg'))
assert 'diff' in f_remote
assert os.path.exists(mp.fpath_meta('renamed.gpkg'))

# pull project in different directory
toggle_geodiff(pull_geodiff_enabled)
mp2 = MerginProject(project_dir_2)
mc.pull_project(project_dir_2)
if pull_geodiff_enabled:
mp2.geodiff.create_changeset(mp.fpath(f_updated), mp2.fpath(f_updated), mp2.fpath_meta('diff'))
assert not mp2.geodiff.has_changes(mp2.fpath_meta('diff'))
else:
server_file_checksum = next((f['checksum'] for f in project_info['files'] if f['path'] == f_updated), '')
assert server_file_checksum == generate_checksum(mp2.fpath(f_updated))
mp2.geodiff.create_changeset(mp.fpath(f_updated), mp2.fpath(f_updated), mp2.fpath_meta('diff'))
assert not mp2.geodiff.has_changes(mp2.fpath_meta('diff'))

# introduce conflict local change (inserted B feature to base)
mp3 = MerginProject(project_dir_3)
shutil.copy(mp3.fpath('inserted_1_B.gpkg'), mp3.fpath(f_updated))
checksum = generate_checksum(mp3.fpath('inserted_1_B.gpkg'))
mc.pull_project(project_dir_3)
if pull_geodiff_enabled:
assert not os.path.exists(mp3.fpath('base.gpkg_conflict_copy'))
else:
assert os.path.exists(mp3.fpath('base.gpkg_conflict_copy')) #
assert generate_checksum(mp3.fpath('base.gpkg_conflict_copy')) == checksum
assert not os.path.exists(mp3.fpath('base.gpkg_conflict_copy'))

# push new changes from project_3 and pull in original project
toggle_geodiff(push_geodiff_enabled)
mc.push_project(project_dir_3)
toggle_geodiff(pull_geodiff_enabled)
mc.pull_project(project_dir)
if pull_geodiff_enabled:
mp3.geodiff.create_changeset(mp.fpath(f_updated), mp3.fpath(f_updated), mp.fpath_meta('diff'))
assert not mp3.geodiff.has_changes(mp.fpath_meta('diff'))
else:
assert os.path.exists(mp.fpath('base.gpkg_conflict_copy'))

# make sure that we leave geodiff enabled for further tests
toggle_geodiff(True)
mp3.geodiff.create_changeset(mp.fpath(f_updated), mp3.fpath(f_updated), mp.fpath_meta('diff'))
assert not mp3.geodiff.has_changes(mp.fpath_meta('diff'))


def test_list_of_push_changes(mc):
Expand All @@ -396,7 +353,6 @@ def test_list_of_push_changes(mc):

cleanup(mc, project, [project_dir])
shutil.copytree(TEST_DATA_DIR, project_dir)
toggle_geodiff(True)
mc.create_project_and_push(test_project, project_dir)

f_updated = 'base.gpkg'
Expand Down
2 changes: 0 additions & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
'console_scripts': ['mergin=mergin.cli:cli'],
},

test_suite='nose.collector',
tests_require=['nose'],
classifiers=[
'Development Status :: 5 - Production/Stable',
'Intended Audience :: Developers',
Expand Down