Skip to content
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.

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

Add immediate project removal to fix auto-tests #177

Merged
merged 5 commits into from
Oct 4, 2023
Merged
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
56 changes: 54 additions & 2 deletions mergin/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
)
from .client_pull import pull_project_async, pull_project_wait, pull_project_finalize
from .client_push import push_project_async, push_project_wait, push_project_finalize
from .utils import DateTimeEncoder, get_versions_with_file_changes, int_version
from .utils import DateTimeEncoder, get_versions_with_file_changes, int_version, is_version_acceptable
from .version import __version__

this_dir = os.path.dirname(os.path.realpath(__file__))
Expand Down Expand Up @@ -79,6 +79,7 @@ def __init__(self, url=None, auth_token=None, login=None, password=None, plugin_
self._auth_session = None
self._user_info = None
self._server_type = None
self._server_version = None
self.client_version = "Python-client/" + __version__
if plugin_version is not None: # this could be e.g. "Plugin/2020.1 QGIS/3.14"
self.client_version += " " + plugin_version
Expand Down Expand Up @@ -383,6 +384,23 @@ def server_type(self):

return self._server_type

def server_version(self):
"""
Returns version of the server

:returns: Version string, e.g. "2023.5.0". For old servers (pre-2023) this may be empty string.
:rtype: str
"""
if self._server_version is None:
try:
resp = self.get("/config")
config = json.load(resp)
self._server_version = config["version"]
except (ClientError, KeyError):
self._server_version = ""

return self._server_version

def workspaces_list(self):
"""
Find all available workspaces
Expand Down Expand Up @@ -803,14 +821,48 @@ def clone_project(self, source_project_path, cloned_project_name, cloned_project
request = urllib.request.Request(url, data=json.dumps(data).encode(), headers=json_headers, method="POST")
self._do_request(request)

def delete_project_now(self, project_path):
"""
Delete project repository on server immediately.

This should be typically in development, e.g. auto tests, where we
do not want scheduled project deletes as done by delete_project().

:param project_path: Project's full name (<namespace>/<name>)
:type project_path: String

"""
# TODO: this version check should be replaced by checking against the list
# of endpoints that server publishes in /config (once implemented)
if not is_version_acceptable(self.server_version(), "2023.5"):
raise NotImplementedError("This needs server at version 2023.5 or later")

project_info = self.project_info(project_path)
project_id = project_info["id"]
path = "/v2/projects/" + project_id
url = urllib.parse.urljoin(self.url, urllib.parse.quote(path))
request = urllib.request.Request(url, method="DELETE")
self._do_request(request)

def delete_project(self, project_path):
"""
Delete project repository on server.
Delete project repository on server. Newer servers since 2023
will schedule deletion of the project, but not fully remove it immediately.
This is the preferred way when the removal is user initiated, so that
it is not possible to replace the project with another one with the same
name, which would confuse clients that do not use project IDs yet.

There is also delete_project_now() on newer servers which deletes projects
immediately.

:param project_path: Project's full name (<namespace>/<name>)
:type project_path: String

"""
# TODO: newer server version (since 2023.5.0) have a new endpoint
# (POST /v2/projects/<id>/scheduleDelete) that does the same thing,
# but using project ID instead of the name. At some point we may
# want to migrate to it.
path = "/v1/project/%s" % project_path
url = urllib.parse.urljoin(self.url, urllib.parse.quote(path))
request = urllib.request.Request(url, method="DELETE")
Expand Down
45 changes: 29 additions & 16 deletions mergin/test/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def create_workspace_for_client(mc):
def cleanup(mc, project, dirs):
# cleanup leftovers from previous test if needed such as remote project and local directories
try:
mc.delete_project(project)
mc.delete_project_now(project)
except ClientError:
pass
remove_folders(dirs)
Expand All @@ -76,6 +76,11 @@ def remove_folders(dirs):
shutil.rmtree(d)


def sudo_works():
sudo_res = subprocess.run(["sudo", "echo", "test"])
return sudo_res.returncode != 0


def test_login(mc):
token = mc._auth_session["token"]
assert MerginClient(mc.url, auth_token=token)
Expand Down Expand Up @@ -105,17 +110,17 @@ def test_create_delete_project(mc):
assert any(p for p in projects if p["name"] == test_project and p["namespace"] == API_USER)

# try again
with pytest.raises(ClientError, match=f"Project {test_project} already exists"):
with pytest.raises(ClientError, match=f"already exists"):
mc.create_project(test_project)

# remove project
mc.delete_project(API_USER + "/" + test_project)
mc.delete_project_now(API_USER + "/" + test_project)
projects = mc.projects_list(flag="created")
assert not any(p for p in projects if p["name"] == test_project and p["namespace"] == API_USER)

# try again, nothing to delete
with pytest.raises(ClientError):
mc.delete_project(API_USER + "/" + test_project)
mc.delete_project_now(API_USER + "/" + test_project)


def test_create_remote_project_from_local(mc):
Expand Down Expand Up @@ -1034,6 +1039,8 @@ def _use_wal(db_file):
con = sqlite3.connect(db_file)
cursor = con.cursor()
cursor.execute("PRAGMA journal_mode=wal;")
cursor.close()
con.close()


def _create_test_table(db_file):
Expand Down Expand Up @@ -1069,18 +1076,18 @@ def _delete_spatial_table(db_file):

def _check_test_table(db_file):
"""Checks whether the 'test' table exists and has one row - otherwise fails with an exception."""
# con_verify = sqlite3.connect(db_file)
# cursor_verify = con_verify.cursor()
# cursor_verify.execute('select count(*) from test;')
# assert cursor_verify.fetchone()[0] == 1
assert _get_table_row_count(db_file, "test") == 1


def _get_table_row_count(db_file, table):
con_verify = sqlite3.connect(db_file)
cursor_verify = con_verify.cursor()
cursor_verify.execute("select count(*) from {};".format(table))
return cursor_verify.fetchone()[0]
try:
con_verify = sqlite3.connect(db_file)
cursor_verify = con_verify.cursor()
cursor_verify.execute("select count(*) from {};".format(table))
return cursor_verify.fetchone()[0]
finally:
cursor_verify.close()
con_verify.close()


def _is_file_updated(filename, changes_dict):
Expand Down Expand Up @@ -1484,6 +1491,7 @@ def create_directory(root, data):
open(os.path.join(root, file_name), "w").close()


@pytest.mark.skipif(sudo_works(), reason="needs working sudo")
def test_unfinished_pull(mc):
"""
Checks whether a pull with failed rebase (due to remote DB schema change)
Expand Down Expand Up @@ -1527,7 +1535,8 @@ def test_unfinished_pull(mc):

# lock base file to emulate situation when we can't overwrite it, because
# it is used by another process
subprocess.run(["sudo", "chattr", "+i", test_gpkg])
sudo_res = subprocess.run(["sudo", "chattr", "+i", test_gpkg])
assert sudo_res.returncode == 0

mc.pull_project(project_dir)

Expand All @@ -1549,7 +1558,8 @@ def test_unfinished_pull(mc):
assert _get_table_row_count(test_gpkg_unfinished_pull, "simple") == 3

# unlock base file, so we can apply changes from the unfinished pull
subprocess.run(["sudo", "chattr", "-i", test_gpkg])
sudo_res = subprocess.run(["sudo", "chattr", "-i", test_gpkg])
assert sudo_res.returncode == 0

mc.resolve_unfinished_pull(project_dir)

Expand All @@ -1570,6 +1580,7 @@ def test_unfinished_pull(mc):
assert _get_table_row_count(test_gpkg_conflict, "simple") == 4


@pytest.mark.skipif(sudo_works(), reason="needs working sudo")
def test_unfinished_pull_push(mc):
"""
Checks client behaviour when performing push and pull of the project
Expand Down Expand Up @@ -1612,7 +1623,8 @@ def test_unfinished_pull_push(mc):

# lock base file to emulate situation when we can't overwrite it, because
# it is used by another process
subprocess.run(["sudo", "chattr", "+i", test_gpkg])
sudo_res = subprocess.run(["sudo", "chattr", "+i", test_gpkg])
assert sudo_res.returncode == 0

mc.pull_project(project_dir)

Expand Down Expand Up @@ -1644,7 +1656,8 @@ def test_unfinished_pull_push(mc):
mc.pull_project(project_dir)

# unlock base file, so we can apply changes from the unfinished pull
subprocess.run(["sudo", "chattr", "-i", test_gpkg])
sudo_res = subprocess.run(["sudo", "chattr", "-i", test_gpkg])
assert sudo_res.returncode == 0

# perform pull. This should resolve unfinished pull first and then
# collect data from the server
Expand Down
23 changes: 23 additions & 0 deletions mergin/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,26 @@ def edit_conflict_file_name(path, user, version):
ext = "".join(Path(tail).suffixes)
file_name = tail.replace(ext, "")
return os.path.join(head, file_name) + f" (edit conflict, {user} v{version}).json"


def is_version_acceptable(version, min_version):
"""
Checks whether given version is at least min_version or later (both given as strings).

Versions are expected to be using syntax X.Y.Z

Returns true if version >= min_version
"""
m = re.search("(\\d+)[.](\\d+)", min_version)
min_major, min_minor = m.group(1), m.group(2)

if len(version) == 0:
return False # unknown version is assumed to be old

m = re.search("(\\d+)[.](\\d+)", version)
if m is None:
return False

major, minor = m.group(1), m.group(2)

return major > min_major or (major == min_major and minor >= min_minor)
Loading