From 45ce11fe268139bc3a23bc6e50fe02039f351ecb Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 9 Aug 2023 17:10:30 -0600 Subject: [PATCH 01/67] WIP: conflict-resolution things from pre-pause --- src/magic_folder/cli.py | 82 +++++++++++++++++++++++++++++++ src/magic_folder/client.py | 11 +++++ src/magic_folder/config.py | 19 ++++++- src/magic_folder/downloader.py | 31 +++++++++++- src/magic_folder/magic_file.py | 48 +++++++++++++++++- src/magic_folder/participants.py | 1 + src/magic_folder/snapshot.py | 8 ++- src/magic_folder/test/test_web.py | 68 +++++++++++++++++++++++++ src/magic_folder/web.py | 65 ++++++++++++++++++++++++ 9 files changed, 326 insertions(+), 7 deletions(-) diff --git a/src/magic_folder/cli.py b/src/magic_folder/cli.py index 91aed6917..d5cc9d7a7 100644 --- a/src/magic_folder/cli.py +++ b/src/magic_folder/cli.py @@ -800,6 +800,86 @@ def leave(options): raise SystemExit(1) + + +class ResolveOptions(usage.Options): + description = "Resolve a conflict" + optFlags = [ + ("mine", None, "Resolve the conflict by keeping only my version"), + ("theirs", None, "Resolve the conflict by keeping the other user's version"), + ] + optParameters = [ + ("use", "u", None, "Name of the partipant whose changes we should keep (for multiparty conflicts)", str), + ] + + # argument; the file we're resolving the conflict on + _filepath = None + + def parseArgs(self, fname=None): + if fname is None: + raise usage.UsageError( + "Must specify a single argument: the conflicted file" + ) + self._filepath = FilePath(fname) + if not self._filepath.exists(): + raise ValueError("{} doesn't exist".format(fname)) + + def postOptions(self): + super(ResolveOptions, self).postOptions() + if self["mine"] is None and self["theirs"] is None and self["use"] is None: + raise usage.UsageError( + "Must specify --mine, --theirs or --use" + ) + if self["mine"] and self["use"]: + raise usage.UsageError( + "Cannot specify --use and --mine at the same time" + ) + if self["theirs"] and self["use"]: + raise usage.UsageError( + "Cannot specify --use and --theirs at the same time" + ) + + +@inline_callbacks +def resolve(options): + client = options.parent.client + cfg = options.parent.config + + # figure out the folder name and relpath from the filename we have + for foldername in cfg.list_magic_folders(): + mf = cfg.get_magic_folder(foldername) + try: + relpath = options._filepath.segmentsFrom(mf.magic_path) + except ValueError: + relpath = None + if relpath: + break + if not relpath: + raise usage.UsageError( + "{}: not inside any magic-folder".format(options._filepath.path) + ) + + take = None + if options["mine"]: + take = "mine" + elif options["theirs"]: + take = "theirs" + if take is None and options["use"] is None: + raise usage.UsageError("Must specify one of --theirs, --mine or --use") + + try: + x = yield client.resolve_conflict( + foldername, + "/".join(relpath), + take, + options["use"], + ) + print(x) + except MagicFolderApiError as e: + print("Error: {}".format(e.reason), file=options.stderr) + raise SystemExit(1) + + class RunOptions(usage.Options): optParameters = [ ] @@ -997,6 +1077,7 @@ class MagicFolderCommand(BaseOptions): ["list", None, ListOptions, "List Magic Folders configured in this client."], ["run", None, RunOptions, "Run the Magic Folders daemon process."], ["status", None, StatusOptions, "Show the current status of a folder."], + ["resolve", None, ResolveOptions, "Choose how to resolve a conflict."], ] optFlags = [ ["debug", "d", "Print full stack-traces"], @@ -1054,6 +1135,7 @@ def getUsage(self, width=None): "leave": leave, "list": list_, "status": status, + "resolve": resolve, "run": run, } diff --git a/src/magic_folder/client.py b/src/magic_folder/client.py index c9a26b7d4..6f1c3be92 100644 --- a/src/magic_folder/client.py +++ b/src/magic_folder/client.py @@ -168,6 +168,17 @@ def list_conflicts(self, magic_folder): api_url = self.base_url.child(u'v1', u'magic-folder', magic_folder, u'conflicts') return self._authorized_request("GET", api_url) + def resolve_conflict(self, magic_folder, relpath, take, use): + api_url = self.base_url.child(u'v1', u'magic-folder', magic_folder, u'resolve-conflict') + body = { + "relpath": relpath, + } + if take: + body["take"] = take + if use: + body["use"] = use + return self._authorized_request("POST", api_url, body=json.dumps(body).encode("utf8")) + def tahoe_objects(self, magic_folder): api_url = self.base_url.child(u'v1', u'magic-folder', magic_folder, u'tahoe-objects') return self._authorized_request("GET", api_url) diff --git a/src/magic_folder/config.py b/src/magic_folder/config.py index 19c208922..5956949fa 100644 --- a/src/magic_folder/config.py +++ b/src/magic_folder/config.py @@ -269,6 +269,17 @@ [upload_duration_ns] INTEGER -- nanoseconds the last upload took ) """, + # From version 23.6.0 until 23.7.0 we recorded "author name" + # in the database, but "participant name" on the filesystem. + # + # ...from 23.7.0 onwards we record "participant name" in the + # database (and filesystem) + # + # So (relpath, conflict_author) _can_ be counted on to be + # unique ONLY IF conflict_author is actually the "participant" + # name from the Collective. Gridsync sets all author names to + # "user", though, so actual author names have zero assurance + # of being unique (and generally won't be unique in Gridsync usage) """ --- This table represents our notion of conflicts (although they are also represented --- on disk, our representation is canonical as the filesystem is part of the API) @@ -812,6 +823,7 @@ class Conflict(object): """ snapshot_cap = attr.ib() # Tahoe URI author_name = attr.ib(validator=instance_of(str)) +## participant_name = attr.ib(validator=instance_of(str)) @attr.s @@ -1587,11 +1599,13 @@ def list_conflicts_for(self, cursor, relpath): ] @with_cursor - def add_conflict(self, cursor, snapshot): + def add_conflict(self, cursor, snapshot, participant): """ Add a new conflicting author :param RemoteSnapshot snapshot: the conflicting Snapshot + + :param IParticipant participant: member of the Collective we found this update in """ with start_action(action_type="config:state-db:add-conflict", relpath=snapshot.relpath): cursor.execute( @@ -1601,7 +1615,8 @@ def add_conflict(self, cursor, snapshot): VALUES (?,?,?) """, - (snapshot.relpath, snapshot.author.name, snapshot.capability.danger_real_capability_string()), + ##(snapshot.relpath, snapshot.author.name, snapshot.capability.danger_real_capability_string()), + (snapshot.relpath, participant.name, snapshot.capability.danger_real_capability_string()), ) @with_cursor diff --git a/src/magic_folder/downloader.py b/src/magic_folder/downloader.py index f99e22d26..53963126c 100644 --- a/src/magic_folder/downloader.py +++ b/src/magic_folder/downloader.py @@ -232,8 +232,8 @@ def mark_conflict(relpath, conflict_path, staged_content): This snapshot causes a conflict. The existing magic-folder file is untouched. The downloaded / prepared content shall be moved to a file named `.theirs.` where `` is the - petname of the author of the conflicting snapshot and `` - is the relative path inside the magic-folder. + participant name of the conflicting snapshot and `` is + the relative path inside the magic-folder. XXX can deletes conflict? if so staged_content would be None @@ -243,6 +243,18 @@ def mark_conflict(relpath, conflict_path, staged_content): content. """ + def mark_not_conflicted(relpath, keep_path, rejected_paths): + """ + A formerly conflicted file is now no longer conflicted. + + :param FilePath keep_path: the variant we will retain + (i.e. that must end up at `relpath`) + + :param [FilePath] rejected_paths: the existing conflict files + which we should ensure no longer exist. Must contain at + least one item (or else this wasn't actually conflicted). + """ + def mark_delete(relpath): """ Mark this snapshot as a delete. The existing magic-folder file @@ -415,6 +427,21 @@ def mark_conflict(self, relpath, conflict_path, staged_content): local_path = self.magic_path.preauthChild(conflict_path) staged_content.moveTo(local_path) + + def mark_not_conflicted(self, relpath, keep_path, rejected_paths): + """ + """ + dest_path = self.magic_path.preauthChild(relpath) + src_path = self.magic_path.preauthChild(keep_path) + del_paths = [ + self.magic_path.preauthChild(p) + for p in rejected_paths + if p.exists() + ] + src_path.moveTo(dest_path) + for p in del_paths: + p.remove() + def mark_delete(self, relpath): """ Mark this snapshot as a delete. The existing magic-folder file diff --git a/src/magic_folder/magic_file.py b/src/magic_folder/magic_file.py index 1b18c295b..133131f1d 100644 --- a/src/magic_folder/magic_file.py +++ b/src/magic_folder/magic_file.py @@ -73,7 +73,7 @@ class MagicFileFactory(object): _config = attr.ib() # MagicFolderConfig _tahoe_client = attr.ib() _folder_status = attr.ib() - _local_snapshot_service = attr.ib() + _local_snapshot_service = attr.ib()#validator=attr.validators.instance_of(LocalSnapshotService)) _uploader = attr.ib() _write_participant = attr.ib() _remote_cache = attr.ib() @@ -194,6 +194,9 @@ class MagicFile(object): _queue_remote = attr.ib(default=attr.Factory(list)) _is_working = attr.ib(default=None) + # XXX + _known_remotes = attr.ib(default=None) + # to facilitate testing, sometimes we _don't_ want to use the real # reactor to wait one turn, or to delay for retry purposes. _call_later = attr.ib(default=None) # Callable to schedule work at least one reactor turn later @@ -258,6 +261,40 @@ def is_empty(arg): d.addCallback(is_empty) return d + def resolve_conflict(self, resolution): + """ + This file is in conflict and we specify a resolution. + """ + # ultimately, we want to do this: + # - make all local files "match" the resolution + # - move the favored file to relpath (check first?) + # - delete all (other) conflict-markers + # - make a new LocalSnapshot with multiplate parents + # + # XXX _WHEN_ do we "move files" and "delete markers"; if we + # have queued stuff for this file ... bail? Does it still / + # ever make sense to resolve a conflict if we're offline? + # + # If we say "mine", we take whatever is in relpath. + # - delete all conflict markers + # + # If we say "theirs", we: + # - move "their" content over top of relpath + # - delete any other conflict-marker files + # + # If we say "some participant name", we: + # - move that content over top of relpath + # - delete all (other) conflict-markers + # + # For all cases above, we _then_ create a new LocalSnapshot: + # - content is in relpath + # - parents are ALL the parents (at least 2!) + # - one (or more) from conflict database, one from remotesnapshots + # + # Delete all conflict entries from the database (last? first?) + # + ## self._factory._locate_snapshot_service. + def found_new_remote(self, remote_snapshot, participant): """ A RemoteSnapshot that doesn't match our existing database entry @@ -265,7 +302,14 @@ def found_new_remote(self, remote_snapshot, participant): resulting in conflicts). :param RemoteSnapshot remote_snapshot: the newly-discovered remote + + :param IParticipant participant: the participant we found this snapshot via """ + if self._known_remotes is None: + self._known_remotes = {remote_snapshot} + else: + self._known_remotes.add(remote_snapshot) + print("{} --[known]--> {}".format(remote_snapshot.relpath, [r.author.name for r in self._known_remotes])) self._remote_update(remote_snapshot, participant) return self.when_idle() @@ -862,7 +906,7 @@ def _mark_download_conflict(self, snapshot, staged_path, participant): participant.name, ) self._factory._magic_fs.mark_conflict(self._relpath, conflict_path, staged_path) - self._factory._config.add_conflict(snapshot) + self._factory._config.add_conflict(snapshot, participant) @_machine.output() def _update_personal_dmd_upload(self, snapshot): diff --git a/src/magic_folder/participants.py b/src/magic_folder/participants.py index b901fe507..9c24036ab 100644 --- a/src/magic_folder/participants.py +++ b/src/magic_folder/participants.py @@ -43,6 +43,7 @@ class IParticipant(Interface): particular magic folder. """ is_self = Attribute("``True`` if this participant is us, ``False`` otherwise.") + name = Attribute("The unique name of this participant in the Collective") def files(): """ diff --git a/src/magic_folder/snapshot.py b/src/magic_folder/snapshot.py index 692c29752..6771564e3 100644 --- a/src/magic_folder/snapshot.py +++ b/src/magic_folder/snapshot.py @@ -350,7 +350,7 @@ def deserialize_dict(snapshot_dict, author): return deserialize_dict(local_snapshot_dict, author) -@attr.s +@attr.define(frozen=True) class RemoteSnapshot(object): """ Represents a snapshot corresponding to a particular version of a @@ -393,6 +393,12 @@ def is_delete(self): """ return self.content_cap is not None + def __eq__(self, other): + return self.capability == other.capability + + def __hash__(self): + return hash(self.capability.danger_real_capability_string()) + @inline_callbacks def create_snapshot_from_capability(snapshot_cap, tahoe_client): diff --git a/src/magic_folder/test/test_web.py b/src/magic_folder/test/test_web.py index c731b8634..b958e199c 100644 --- a/src/magic_folder/test/test_web.py +++ b/src/magic_folder/test/test_web.py @@ -2133,6 +2133,74 @@ def test_one_conflict(self): ) + def test_resolve_conflict(self): + """ + We can resolve a conflict + """ + local_path = FilePath(self.mktemp()) + local_path.makedirs() + + folder_config = magic_folder_config( + "marta", + local_path, + ) + + node = MagicFolderNode.create( + Clock(), + FilePath(self.mktemp()), + AUTH_TOKEN, + { + "default": folder_config, + }, + start_folder_services=False, + ) + node.global_service.get_folder_service("default").file_factory._synchronous = True + + mf_config = node.global_config.get_magic_folder("default") + mf_config._get_current_timestamp = lambda: 42.0 + mf_config.store_currentsnapshot_state( + "foo", + PathState(123, seconds_to_ns(1), seconds_to_ns(2)), + ) + + snap = RemoteSnapshot( + "foo", + create_local_author("nelli"), + {"relpath": "foo", "modification_time": 1234}, + random_immutable(directory=True), + [], + random_immutable(), + random_immutable(), + ) + + mf_config.add_conflict(snap) + + # external API + self.assertThat( + authorized_request( + node.http_client, + AUTH_TOKEN, + u"POST", + self.url.child("default", "resolve-conflict"), + dumps({ + "take": "mine", + # "use": ..., for multi-conflicts + }).encode("utf8") + ), + succeeded( + matches_response( + code_matcher=Equals(200), + body_matcher=AfterPreprocessing( + loads, + Equals({ + "foo": ["nelli"], + }), + ) + ), + ) + ) + + class InviteTests(SyncTestCase): """ Tests relating to invites diff --git a/src/magic_folder/web.py b/src/magic_folder/web.py index c1c72554a..0a1cb3d7b 100644 --- a/src/magic_folder/web.py +++ b/src/magic_folder/web.py @@ -534,6 +534,71 @@ def list_conflicts(request, folder_name): for relpath, conflicts in folder_config.list_conflicts().items() }).encode("utf8") + @app.route("/magic-folder//resolve-conflict", methods=['POST']) + def resolve_conflict(request, folder_name): + """ + Resolve an existing conflict. + """ + # maybe 'too much' code to live in the Web API? Move to Service? + + # XXX maybe we can just "dumb-ly" pass these args through to + # the state-machine and have it throw these exceptions, which + # we catch (as ValueError) and re-raise as _InputError ? + + _application_json(request) # set reply headers + folder_config = global_config.get_magic_folder(folder_name) + resolution = _load_json(request.content.read()) + conflicts = folder_config.list_conflicts_for(resolution["relpath"]) + if not conflicts: + raise _InputError('No conflicts for "{relpath}"'.format(**resolution)) + if "take" in resolution: + if "use" in resolution: + raise _InputError('Cannot specify "take" and "use" at once') + who = resolution["take"] + if who not in {"mine", "theirs"}: + raise _InputError('"take" must be "mine" or "theirs"') + # if there is more than one conflicted party, then + # "theirs" is ambiguous and they must use "--take" + if who == "theirs" and len(conflicts) > 1: + raise _InputError('Cannot use "theirs" with {} conflicts'.format(len(conflicts))) + # now we know the resolution, and it's valid + if who == "theirs": + matching_conflicts = conflicts + else: + matching_conflicts = None + + elif "use" in resolution: + if "take" in resolution: + raise _InputError('Cannot specify "take" and "use" at once') + participant_name = resolution["take"] + matching_conflicts = [ + conflict + for conflict in conflicts + if conflict.name == participant_name + ] + if not matching_conflicts: + raise _InputError('"{relpath}" is not conflicted with "{take}"'.format(**resolution)) + if len(matching_conflicts) > 1: + raise _InputError('Multiple conflicts match; internal inconsistency?') + # now we know the resolution, and it's valid + + else: + raise _InputError('Must specify "take" or "use"') + + # if "matching_conflicts" is None, we want "us" + # ...otherwise, it contains a single Conflict which is the one we want + + # XXX names coming in are "participants" (since 23.6.0) not + # authors -- is it even possible to map between them? (I think + # at least for GridSync, it'll always be "user" for + # author-name unfortunately) + + # XXX probably want to use "the state-machine" for resolving + # conflicts, since that produces a new (local) snapshot .. and + # they may be others in the queue...? + + return json.dumps({"hello": "foo"}).encode("utf8") + @app.route("/magic-folder//tahoe-objects", methods=['GET']) def folder_tahoe_objects(request, folder_name): """ From c7c0e08e681bdbe2104b984fc5d3cb920b101d8f Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 21 Oct 2023 20:14:25 -0600 Subject: [PATCH 02/67] match api, if not semantics --- src/magic_folder/test/test_config.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/magic_folder/test/test_config.py b/src/magic_folder/test/test_config.py index 7d1ada4de..e19cc05dc 100644 --- a/src/magic_folder/test/test_config.py +++ b/src/magic_folder/test/test_config.py @@ -92,6 +92,9 @@ load_global_configuration, is_valid_experimental_feature, ) +from ..participants import ( + static_participants, +) from ..snapshot import ( create_local_author, create_snapshot, @@ -1336,6 +1339,11 @@ def test_add_conflict_twice(self, remote_cap, meta_cap, content_cap): """ It's an error to add the same conflict twice """ + participants = static_participants( + names=["marlyn"], + my_files=[], + other_files=[], + ) snap = RemoteSnapshot( "foo", self.author, @@ -1346,9 +1354,9 @@ def test_add_conflict_twice(self, remote_cap, meta_cap, content_cap): meta_cap, ) - self.db.add_conflict(snap) + self.db.add_conflict(snap, participants.list()[0]) with self.assertRaises(sqlite3.IntegrityError): - self.db.add_conflict(snap) + self.db.add_conflict(snap, participants.list()[0]) self.db.resolve_conflict("foo") @given( From ddb0b68668cb3cbdf537ba66da5c495fc6652442 Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 22 Oct 2023 01:08:09 -0600 Subject: [PATCH 03/67] fix test; refactor setup --- src/magic_folder/test/test_config.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/magic_folder/test/test_config.py b/src/magic_folder/test/test_config.py index e19cc05dc..49a013050 100644 --- a/src/magic_folder/test/test_config.py +++ b/src/magic_folder/test/test_config.py @@ -1290,6 +1290,7 @@ def setUp(self): self.magic = self.temp.child("magic") self.magic.makedirs() + def setup_example(self): self.db = MagicFolderConfig.initialize( u"some-folder", SQLite3DatabaseLocation.memory(), @@ -1311,6 +1312,11 @@ def test_add_list_conflict(self, remote_cap, meta_cap, content_cap): """ Adding a conflict allows us to list it """ + participants = static_participants( + names=["adele"], + my_files=[], + other_files=[], + ) snap = RemoteSnapshot( "foo", self.author, @@ -1321,14 +1327,13 @@ def test_add_list_conflict(self, remote_cap, meta_cap, content_cap): meta_cap, ) - self.db.add_conflict(snap) + self.db.add_conflict(snap, participants.list()[0]) self.assertThat( self.db.list_conflicts(), Equals({ - "foo": [Conflict(remote_cap, self.author.name)], + "foo": [Conflict(remote_cap, "adele")], }), ) - self.db.resolve_conflict("foo") @given( tahoe_lafs_immutable_dir_capabilities(), From dc582e23d3dae01854b61c0f3d8ed0e692c76100 Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 22 Oct 2023 01:14:44 -0600 Subject: [PATCH 04/67] test fix --- src/magic_folder/test/test_config.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/magic_folder/test/test_config.py b/src/magic_folder/test/test_config.py index 49a013050..b5ead3381 100644 --- a/src/magic_folder/test/test_config.py +++ b/src/magic_folder/test/test_config.py @@ -1376,6 +1376,11 @@ def test_add_list_multi_conflict(self, remote0_cap, remote1_cap, meta_cap, conte """ assume(remote0_cap != remote1_cap) + participants = static_participants( + names=["ursula", "le guin"], + my_files=[], + other_files=[list(), ], + ) snap0 = RemoteSnapshot( "foo", create_local_author(u"desktop"), @@ -1395,18 +1400,17 @@ def test_add_list_multi_conflict(self, remote0_cap, remote1_cap, meta_cap, conte meta_cap, ) - self.db.add_conflict(snap0) - self.db.add_conflict(snap1) + self.db.add_conflict(snap0, participants.list()[0]) + self.db.add_conflict(snap1, participants.list()[1]) self.assertThat( self.db.list_conflicts(), Equals({ "foo": [ - Conflict(remote0_cap, "desktop"), - Conflict(remote1_cap, "laptop"), + Conflict(remote0_cap, "ursula"), + Conflict(remote1_cap, "le guin"), ], }) ) - self.db.resolve_conflict("foo") @given( tahoe_lafs_immutable_dir_capabilities(), From a0010b1e6a1a2fb5016690b8d2f5671f0e84d898 Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 22 Oct 2023 01:21:01 -0600 Subject: [PATCH 05/67] test fix --- src/magic_folder/test/test_config.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/magic_folder/test/test_config.py b/src/magic_folder/test/test_config.py index b5ead3381..cb95c6b63 100644 --- a/src/magic_folder/test/test_config.py +++ b/src/magic_folder/test/test_config.py @@ -1422,6 +1422,11 @@ def test_delete_multi_conflict(self, remote0_cap, remote1_cap, immutable_cap): A multiple-conflict is successfully deleted """ + participants = static_participants( + names=["sarah", "connor"], + my_files=[], + other_files=[list(), ], + ) snap0 = RemoteSnapshot( "foo", create_local_author(u"laptop"), @@ -1441,14 +1446,14 @@ def test_delete_multi_conflict(self, remote0_cap, remote1_cap, immutable_cap): immutable_cap, ) - self.db.add_conflict(snap0) - self.db.add_conflict(snap1) + self.db.add_conflict(snap0, participants.list()[0]) + self.db.add_conflict(snap1, participants.list()[1]) self.assertThat( self.db.list_conflicts(), Equals({ "foo": [ - Conflict(remote0_cap, "laptop"), - Conflict(remote1_cap, "phone"), + Conflict(remote0_cap, "sarah"), + Conflict(remote1_cap, "connor"), ] }), ) From fda06dadc11c5cfd1e89b8b034726a51dd8f4475 Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 22 Oct 2023 01:59:08 -0600 Subject: [PATCH 06/67] incorrect params --- src/magic_folder/test/test_download.py | 5 ++++- src/magic_folder/test/test_magic_file.py | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/magic_folder/test/test_download.py b/src/magic_folder/test/test_download.py index fe866a642..63f49156c 100644 --- a/src/magic_folder/test/test_download.py +++ b/src/magic_folder/test/test_download.py @@ -1115,6 +1115,7 @@ def setUp(self): self.alice_magic_path = FilePath(self.mktemp()) self.alice_magic_path.makedirs() + self.author = create_local_author("alice") self.alice = MagicFolderNode.create( reactor, FilePath(self.mktemp()), @@ -1385,7 +1386,7 @@ def test_update_delete(self): child_cap = random_immutable(directory=True) child = RemoteSnapshot( relpath="foo", - author=self.alice, + author=self.author, metadata={"modification_time": 0}, capability=child_cap, parents_raw=[parent_cap.danger_real_capability_string()], @@ -1723,6 +1724,7 @@ class CancelTests(AsyncTestCase): def setUp(self): super(CancelTests, self).setUp() self.participants = static_participants() + self.author = create_local_author("diana") # XXX NOTE if this name gets longer, the resulting temp-paths can # become "too long" on windows causing failures @@ -1763,6 +1765,7 @@ def test_cancel0(self): class FakeRemoteSnapshot(object): content_cap = random_immutable(directory=True) relpath = "some_file" # match earlier relpath + author = self.author remote_snapshot = FakeRemoteSnapshot() mf = service.file_factory.magic_file_for(local) diff --git a/src/magic_folder/test/test_magic_file.py b/src/magic_folder/test/test_magic_file.py index 718b5747d..c07aaab5b 100644 --- a/src/magic_folder/test/test_magic_file.py +++ b/src/magic_folder/test/test_magic_file.py @@ -314,7 +314,7 @@ def test_multiple_remote_updates(self): cap0 = random_immutable(directory=True) remote0 = RemoteSnapshot( relpath=relpath, - author=self.author, + author=create_author("someone", VerifyKey(b"\xff" * 32)), metadata={"modification_time": 0}, capability=cap0, parents_raw=[], @@ -324,9 +324,9 @@ def test_multiple_remote_updates(self): self.remote_cache._cached_snapshots[cap0.danger_real_capability_string()] = remote0 abspath = self.config.magic_path.preauthChild(relpath) mf = self.magic_file_factory.magic_file_for(abspath) - self.participants.add(create_author("beth", VerifyKey(b"\xff" * 32)), random_dircap()) - self.participants.add(create_author("callum", VerifyKey(b"\xee" * 32)), random_dircap()) - self.participants.add(create_author("dawn", VerifyKey(b"\xee" * 32)), random_dircap()) + self.participants.add("beth", random_dircap()) + self.participants.add("callum", random_dircap()) + self.participants.add("dawn", random_dircap()) d0 = mf.found_new_remote(remote0, self.participants.participants[1]) d1 = mf.found_new_remote(remote0, self.participants.participants[2]) d2 = mf.found_new_remote(remote0, self.participants.participants[3]) From aacd22de73de08e91c1492e66213f285608601a8 Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 22 Oct 2023 02:14:19 -0600 Subject: [PATCH 07/67] more participants --- src/magic_folder/test/test_scanner.py | 5 ++++- src/magic_folder/test/test_upload.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/magic_folder/test/test_scanner.py b/src/magic_folder/test/test_scanner.py index a0e905a47..eb7e44679 100644 --- a/src/magic_folder/test/test_scanner.py +++ b/src/magic_folder/test/test_scanner.py @@ -29,6 +29,9 @@ from ..config import create_testing_configuration from ..magic_file import MagicFileFactory +from ..participants import ( + static_participants, +) from ..scanner import ( ScannerService, find_updated_files, @@ -604,7 +607,7 @@ def test_scan_conflicted_file(self, relpath): snap1, OLD_PATH_STATE, ) - self.config.add_conflict(snap1) + self.config.add_conflict(snap1, static_participants(names=["ada"]).list()[0]) # now it is conflicted, start a scanner service and let it # find an update. diff --git a/src/magic_folder/test/test_upload.py b/src/magic_folder/test/test_upload.py index cecc1d11b..83c2f8f17 100644 --- a/src/magic_folder/test/test_upload.py +++ b/src/magic_folder/test/test_upload.py @@ -281,7 +281,7 @@ def test_existing_conflict(self, upload_dircap): ) # mark it as a conflict - config.add_conflict(snap) + config.add_conflict(snap, static_participants(names=["existing"]).list()[0]) # create a MagicFile file for this relpath now mf = f.magic_file_factory.magic_file_for(local) From c8dc41346ca145d31595aba88a986a80b9c43f7a Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 22 Oct 2023 02:38:21 -0600 Subject: [PATCH 08/67] fixes --- src/magic_folder/test/test_web.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/magic_folder/test/test_web.py b/src/magic_folder/test/test_web.py index b958e199c..6636e11fe 100644 --- a/src/magic_folder/test/test_web.py +++ b/src/magic_folder/test/test_web.py @@ -132,6 +132,9 @@ RemoteSnapshot, create_local_author, ) +from ..participants import ( + static_participants, +) from .strategies import ( tahoe_lafs_readonly_dir_capabilities, tahoe_lafs_dir_capabilities, @@ -2103,12 +2106,12 @@ def test_one_conflict(self): random_immutable(), ) - mf_config.add_conflict(snap) + mf_config.add_conflict(snap, static_participants(names=["spider"]).list()[0]) # internal API self.assertThat( mf_config.list_conflicts_for("foo"), - Equals([Conflict(snap.capability, "nelli")]) + Equals([Conflict(snap.capability, "spider")]) ) # external API @@ -2125,7 +2128,7 @@ def test_one_conflict(self): body_matcher=AfterPreprocessing( loads, Equals({ - "foo": ["nelli"], + "foo": ["spider"], }), ) ), @@ -2173,7 +2176,7 @@ def test_resolve_conflict(self): random_immutable(), ) - mf_config.add_conflict(snap) + mf_config.add_conflict(snap, static_participants(names=["cavatica"]).list()[0]) # external API self.assertThat( @@ -2183,6 +2186,7 @@ def test_resolve_conflict(self): u"POST", self.url.child("default", "resolve-conflict"), dumps({ + "relpath": "foo", "take": "mine", # "use": ..., for multi-conflicts }).encode("utf8") @@ -2193,7 +2197,7 @@ def test_resolve_conflict(self): body_matcher=AfterPreprocessing( loads, Equals({ - "foo": ["nelli"], + "foo": ["cavatica"], }), ) ), From ad82e454e9179b3dcaef213178d012fe2bb44d4b Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 22 Oct 2023 02:43:23 -0600 Subject: [PATCH 09/67] update version --- src/magic_folder/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/magic_folder/config.py b/src/magic_folder/config.py index 5956949fa..10723fc6d 100644 --- a/src/magic_folder/config.py +++ b/src/magic_folder/config.py @@ -269,10 +269,10 @@ [upload_duration_ns] INTEGER -- nanoseconds the last upload took ) """, - # From version 23.6.0 until 23.7.0 we recorded "author name" + # From version 23.6.0 until 23.10.0 we recorded "author name" # in the database, but "participant name" on the filesystem. # - # ...from 23.7.0 onwards we record "participant name" in the + # ...from 23.10.0 onwards we record "participant name" in the # database (and filesystem) # # So (relpath, conflict_author) _can_ be counted on to be From a631344a8a601c587f2c97beae5bbad2b188aff5 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 2 Nov 2023 23:14:09 -0600 Subject: [PATCH 10/67] WIP: basic resolution stuff works --- src/magic_folder/downloader.py | 15 ++-- src/magic_folder/magic_file.py | 138 +++++++++++++++++++++++++++++++-- src/magic_folder/uploader.py | 26 +++++++ src/magic_folder/web.py | 22 ++++++ 4 files changed, 188 insertions(+), 13 deletions(-) diff --git a/src/magic_folder/downloader.py b/src/magic_folder/downloader.py index 53963126c..532b94064 100644 --- a/src/magic_folder/downloader.py +++ b/src/magic_folder/downloader.py @@ -251,8 +251,10 @@ def mark_not_conflicted(relpath, keep_path, rejected_paths): (i.e. that must end up at `relpath`) :param [FilePath] rejected_paths: the existing conflict files - which we should ensure no longer exist. Must contain at - least one item (or else this wasn't actually conflicted). + which we should ensure no longer exist. If we had one + conflict, and accepted "theirs" then this will be empty + (because we'll move the only conflict-marker over top of + the relpath). """ def mark_delete(relpath): @@ -436,11 +438,14 @@ def mark_not_conflicted(self, relpath, keep_path, rejected_paths): del_paths = [ self.magic_path.preauthChild(p) for p in rejected_paths - if p.exists() ] - src_path.moveTo(dest_path) + if dest_path != src_path: + src_path.moveTo(dest_path) for p in del_paths: - p.remove() + try: + p.remove() + except FileNotFoundError: + pass # it's already gone: good def mark_delete(self, relpath): """ diff --git a/src/magic_folder/magic_file.py b/src/magic_folder/magic_file.py index 133131f1d..359280ffa 100644 --- a/src/magic_folder/magic_file.py +++ b/src/magic_folder/magic_file.py @@ -85,6 +85,13 @@ class MagicFileFactory(object): _delays = attr.ib(default=attr.Factory(list)) _logger = attr.ib(default=None) # mainly for tests + def relpath_to_path(self, relpath): + """ + :returns: an absolute FilePath for the given relpath relative to + this folder's base. + """ + return self._config.magic_path.preauthChild(relpath) + def magic_file_for(self, path): """ :returns: the MagicFile instance for path. This may create one @@ -162,6 +169,41 @@ def finish(self): return DeferredList(idles) +class ResolutionMine(object): + """ + A conflict resolution accpting the local changes + """ + + +class ResolutionTheirs(object): + """ + A conflict resolution accpting the other participants' changes + """ + + +class ResolutionError(Exception): + """ + Any error related to the resolution of a conflict. + """ + + +class LocallyQueuedFiles(ResolutionError): + """ + We have locally queued changes when trying to apply a conflict + resolution. + """ + + def __str__(self): + return 'Cannot resolve "{}": we have {} local updates queued'.format(*self.args) + + +def conflict_to_marker(relpath, participant_name): + """ + Convert a relpath to a conflict-marker for the given participant + """ + return "{}.conflict-{}".format(relpath, participant_name) + + @attr.s class MagicFile(object): """ @@ -265,6 +307,51 @@ def resolve_conflict(self, resolution): """ This file is in conflict and we specify a resolution. """ + + # new thinking: + + # we do everything via "uploader" anyway -- so i guess what we + # want to do is to modify the filesystem to "match" the + # resolution specified, and the tell the uploader to + # upload. (that is, "the filesytem IS one API" so we have to + # handle the case when the human modifies the FS to resolve + # the conflict anyway -- so now it's this case too) + + conflicts = self._factory._config.list_conflicts_for(self._relpath) + if resolution is None: + keep_path = self._relpath + rejected = [ + conflict_to_marker(self._relpath, con.author_name) + for con in conflicts + ] + else: + if resolution not in conflicts: + print("BAD", resolution, conflicts) + raise ResolutionError( + "Resolution not found as existing conflict" + ) + keep_path = conflict_to_marker(self._relpath, resolution.author_name) + rejected = [ + conflict_to_marker(self._relpath, con.author_name) + for con in conflicts + if con != resolution + ] + print("KEEP", keep_path) + print("REJECT", rejected) + self._factory._magic_fs.mark_not_conflicted(self._relpath, keep_path, rejected) + # NOTE: the database NEEDS to retain the conflict markers -- + # when the uploader gets around to doing its thing, _it_ will + # remove the conflicts from the config once it creates a + # correct LocalSnapshot + return self._conflict_resolution() + + + # resolution == None means "mine" + # otherwise, it's a list of Conflict objects + + # .mark_not_conflicted (in MagicFolderConfig?) currently deletes files etc + # (self._factory._config.mark_not_conflicted) + # ultimately, we want to do this: # - make all local files "match" the resolution # - move the favored file to relpath (check first?) @@ -510,7 +597,7 @@ def _no_download_work(self, snapshot): """ @_machine.input() - def _conflict_resolution(self, snapshot): + def _conflict_resolution(self): """ A conflicted file has been resolved """ @@ -539,6 +626,12 @@ def _cancel(self, snapshot): We have been cancelled """ + @_machine.input() + def _resolved_remotely(self): + """ + A remote update successfully resolved a conflict + """ + @_machine.output() def _begin_download(self, snapshot, participant): """ @@ -901,10 +994,7 @@ def _mark_download_conflict(self, snapshot, staged_path, participant): """ Mark a conflict for this remote snapshot """ - conflict_path = "{}.conflict-{}".format( - self._relpath, - participant.name, - ) + conflict_path = conflict_to_marker(self._relpath, participant.name) self._factory._magic_fs.mark_conflict(self._relpath, conflict_path, staged_path) self._factory._config.add_conflict(snapshot, participant) @@ -1048,6 +1138,33 @@ def do_remote_update(done_d, snap, part): return self._call_later(self._no_download_work, None) + @_machine.output() + def _check_if_conflict_resolved(self, snapshot): + """ + We are conflicted and have seen a remote update -- chcek if update + is a fix for the conflict. + """ + # the conflict is 'resolved' if this remote-snapshot has + # multiple parents, and "our" Snapshot is one of them -- XXX + # actually isn't it more general like if _any_ ancestor has + # it. The remote could have: had a "resolve" Snapshot, then + # any number of "normal" snapshots after that (before we see + # anything) ... so we may have to cache them etc right? some + # async-work ... + if len(snapshot.parents_raw) > 1: + rs = self._factory._config.get_remotesnapshot(self._relpath) + # XXX don't we have to do an ancestor check, basically? + # like "are we in ANY of the ancestors of this remote?" + if rs.danger_real_capability_string() in snapshot.parents_raw: + conflicts = self._factory._config.list_conflicts_for(self._relpath) + rejected = [ + conflict_to_marker(self._relpath, conflict.author_name) + for conflict in conflicts + ] + self._factory._magic_fs.mark_not_conflicted(self._relpath, self._relpath, rejected) + self._factory._config.resolve_conflict(self._relpath) + self._call_later(self._resolved_remotely) + @_machine.output() def _working(self): """ @@ -1330,14 +1447,19 @@ def _done_working(self): _conflicted.upon( _conflict_resolution, - enter=_uploading, - outputs=[_begin_upload], + enter=_creating_snapshot, + outputs=[_working, _status_upload_queued, _create_local_snapshot], collector=_last_one, ) _conflicted.upon( _remote_update, enter=_conflicted, - outputs=[], # probably want to .. do something? remember it? + outputs=[_check_if_conflict_resolved], + ) + _conflicted.upon( + _resolved_remotely, + enter=_checking_for_local_work, + outputs=[_check_for_local_work], ) _conflicted.upon( _local_update, diff --git a/src/magic_folder/uploader.py b/src/magic_folder/uploader.py index 18186a243..f9b20dfa4 100644 --- a/src/magic_folder/uploader.py +++ b/src/magic_folder/uploader.py @@ -132,6 +132,32 @@ def create_local_snapshot(self, path): # when we handle conflicts we will have to handle multiple # parents here (or, somewhere) + # XXX if this file is currently conflicted, then: + # - check if the conflict-markers exist or not + # - if they exist: error + # - if they're missing: conflict is resolved -- so multiple parents! (all the conflicts we know about) + conflicts = self._db.list_conflicts_for(relpath) + print("CONFLICTS", relpath, conflicts) + if conflicts: + # XXX check for conflict marker files (or not)! + existing = 0 + from .magic_file import conflict_to_marker + for con in conflicts: + fn = conflict_to_marker(relpath, con.author_name) + if self._magic_dir.preauthChild(fn).exists(): + existing += 1 + print("EXIST", existing) + if existing: + raise RuntimeError("Tried to upload a file but we're conflicted") + else: + # we are marked as a conflict, but all our + # conflict-markers are gone -- so the user has + # resolved the conflict + for con in conflicts: + raw_remote.append(con.snapshot_cap.danger_real_capability_string()) + print("resolving conflict") + self._db.resolve_conflict(relpath) + action = SNAPSHOT_CREATOR_PROCESS_ITEM(relpath=relpath) with action: path_info = get_pathinfo(path) diff --git a/src/magic_folder/web.py b/src/magic_folder/web.py index 0a1cb3d7b..e6f0811dc 100644 --- a/src/magic_folder/web.py +++ b/src/magic_folder/web.py @@ -56,6 +56,9 @@ from .invite import ( InviteError, ) +from .magic_file import ( + ResolutionError, +) from .status import ( StatusFactory, ) @@ -222,6 +225,12 @@ def something_cancelled(request, failure): _application_json(request) return json.dumps({"reason": "cancelled"}).encode("utf8") + @app.handle_errors(ResolutionError) + def resolve_failed(request, failure): + request.setResponseCode(http.CONFLICT) + _application_json(request) + return json.dumps({"reason": str(failure.value)}).encode("utf8") + @app.handle_errors(Exception) def fallback_error(request, failure): """ @@ -585,6 +594,19 @@ def resolve_conflict(request, folder_name): else: raise _InputError('Must specify "take" or "use"') + folder_svc = global_service.get_folder_service(folder_name) + print(folder_svc) + print(dir(folder_svc)) + mf = folder_svc.file_factory.magic_file_for( + folder_svc.file_factory.relpath_to_path(resolution["relpath"]) + ) + if matching_conflicts is not None: + assert len(matching_conflicts) == 1, "Unexpected inconsistency" + resolution = matching_conflicts[0] + else: + resolution = None + mf.resolve_conflict(resolution) + # if "matching_conflicts" is None, we want "us" # ...otherwise, it contains a single Conflict which is the one we want From 6f003fa93416168bb3cf5605c83535806d8b2dbc Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 2 Nov 2023 23:58:05 -0600 Subject: [PATCH 11/67] WIP: fix resolutions --- src/magic_folder/config.py | 2 +- src/magic_folder/test/test_web.py | 6 +++++- src/magic_folder/uploader.py | 3 --- src/magic_folder/web.py | 26 ++++++++++---------------- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/magic_folder/config.py b/src/magic_folder/config.py index 10723fc6d..082b10488 100644 --- a/src/magic_folder/config.py +++ b/src/magic_folder/config.py @@ -824,7 +824,7 @@ class Conflict(object): snapshot_cap = attr.ib() # Tahoe URI author_name = attr.ib(validator=instance_of(str)) ## participant_name = attr.ib(validator=instance_of(str)) - +# should probably re-name to "participant_name" since that's what it is ...? @attr.s class MagicFolderConfig(object): diff --git a/src/magic_folder/test/test_web.py b/src/magic_folder/test/test_web.py index 6636e11fe..1e963e839 100644 --- a/src/magic_folder/test/test_web.py +++ b/src/magic_folder/test/test_web.py @@ -10,6 +10,10 @@ dumps, ) +from attr import ( + evolve, +) + from hyperlink import ( DecodedURL, ) @@ -2552,7 +2556,7 @@ def test_deleted_item(self, remote_snap): # make it a delete .. it's a little weird to have a delete # with no "content" parent (semantically) but for the purposes # of this test that is sufficient. - remote_snap.content_cap = None + remote_snap = evolve(remote_snap, content_cap=None) local_path = FilePath(self.mktemp()) local_path.makedirs() diff --git a/src/magic_folder/uploader.py b/src/magic_folder/uploader.py index f9b20dfa4..a97e69c78 100644 --- a/src/magic_folder/uploader.py +++ b/src/magic_folder/uploader.py @@ -137,7 +137,6 @@ def create_local_snapshot(self, path): # - if they exist: error # - if they're missing: conflict is resolved -- so multiple parents! (all the conflicts we know about) conflicts = self._db.list_conflicts_for(relpath) - print("CONFLICTS", relpath, conflicts) if conflicts: # XXX check for conflict marker files (or not)! existing = 0 @@ -146,7 +145,6 @@ def create_local_snapshot(self, path): fn = conflict_to_marker(relpath, con.author_name) if self._magic_dir.preauthChild(fn).exists(): existing += 1 - print("EXIST", existing) if existing: raise RuntimeError("Tried to upload a file but we're conflicted") else: @@ -155,7 +153,6 @@ def create_local_snapshot(self, path): # resolved the conflict for con in conflicts: raw_remote.append(con.snapshot_cap.danger_real_capability_string()) - print("resolving conflict") self._db.resolve_conflict(relpath) action = SNAPSHOT_CREATOR_PROCESS_ITEM(relpath=relpath) diff --git a/src/magic_folder/web.py b/src/magic_folder/web.py index e6f0811dc..6ffac5ee0 100644 --- a/src/magic_folder/web.py +++ b/src/magic_folder/web.py @@ -557,7 +557,8 @@ def resolve_conflict(request, folder_name): _application_json(request) # set reply headers folder_config = global_config.get_magic_folder(folder_name) resolution = _load_json(request.content.read()) - conflicts = folder_config.list_conflicts_for(resolution["relpath"]) + relpath = resolution["relpath"] + conflicts = folder_config.list_conflicts_for(relpath) if not conflicts: raise _InputError('No conflicts for "{relpath}"'.format(**resolution)) if "take" in resolution: @@ -583,7 +584,7 @@ def resolve_conflict(request, folder_name): matching_conflicts = [ conflict for conflict in conflicts - if conflict.name == participant_name + if conflict.author_name == participant_name ] if not matching_conflicts: raise _InputError('"{relpath}" is not conflicted with "{take}"'.format(**resolution)) @@ -598,7 +599,7 @@ def resolve_conflict(request, folder_name): print(folder_svc) print(dir(folder_svc)) mf = folder_svc.file_factory.magic_file_for( - folder_svc.file_factory.relpath_to_path(resolution["relpath"]) + folder_svc.file_factory.relpath_to_path(relpath) ) if matching_conflicts is not None: assert len(matching_conflicts) == 1, "Unexpected inconsistency" @@ -607,19 +608,12 @@ def resolve_conflict(request, folder_name): resolution = None mf.resolve_conflict(resolution) - # if "matching_conflicts" is None, we want "us" - # ...otherwise, it contains a single Conflict which is the one we want - - # XXX names coming in are "participants" (since 23.6.0) not - # authors -- is it even possible to map between them? (I think - # at least for GridSync, it'll always be "user" for - # author-name unfortunately) - - # XXX probably want to use "the state-machine" for resolving - # conflicts, since that produces a new (local) snapshot .. and - # they may be others in the queue...? - - return json.dumps({"hello": "foo"}).encode("utf8") + return json.dumps({ + relpath: [ + conflict.author_name + for conflict in conflicts + ] + }).encode("utf8") @app.route("/magic-folder//tahoe-objects", methods=['GET']) def folder_tahoe_objects(request, folder_name): From c71d1f49305045ba4c480f4e8f314c14236dba5c Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 3 Nov 2023 00:08:06 -0600 Subject: [PATCH 12/67] cleanup --- src/magic_folder/magic_file.py | 53 ---------------------------------- 1 file changed, 53 deletions(-) diff --git a/src/magic_folder/magic_file.py b/src/magic_folder/magic_file.py index 359280ffa..ca0568aee 100644 --- a/src/magic_folder/magic_file.py +++ b/src/magic_folder/magic_file.py @@ -169,18 +169,6 @@ def finish(self): return DeferredList(idles) -class ResolutionMine(object): - """ - A conflict resolution accpting the local changes - """ - - -class ResolutionTheirs(object): - """ - A conflict resolution accpting the other participants' changes - """ - - class ResolutionError(Exception): """ Any error related to the resolution of a conflict. @@ -326,7 +314,6 @@ def resolve_conflict(self, resolution): ] else: if resolution not in conflicts: - print("BAD", resolution, conflicts) raise ResolutionError( "Resolution not found as existing conflict" ) @@ -336,8 +323,6 @@ def resolve_conflict(self, resolution): for con in conflicts if con != resolution ] - print("KEEP", keep_path) - print("REJECT", rejected) self._factory._magic_fs.mark_not_conflicted(self._relpath, keep_path, rejected) # NOTE: the database NEEDS to retain the conflict markers -- # when the uploader gets around to doing its thing, _it_ will @@ -345,43 +330,6 @@ def resolve_conflict(self, resolution): # correct LocalSnapshot return self._conflict_resolution() - - # resolution == None means "mine" - # otherwise, it's a list of Conflict objects - - # .mark_not_conflicted (in MagicFolderConfig?) currently deletes files etc - # (self._factory._config.mark_not_conflicted) - - # ultimately, we want to do this: - # - make all local files "match" the resolution - # - move the favored file to relpath (check first?) - # - delete all (other) conflict-markers - # - make a new LocalSnapshot with multiplate parents - # - # XXX _WHEN_ do we "move files" and "delete markers"; if we - # have queued stuff for this file ... bail? Does it still / - # ever make sense to resolve a conflict if we're offline? - # - # If we say "mine", we take whatever is in relpath. - # - delete all conflict markers - # - # If we say "theirs", we: - # - move "their" content over top of relpath - # - delete any other conflict-marker files - # - # If we say "some participant name", we: - # - move that content over top of relpath - # - delete all (other) conflict-markers - # - # For all cases above, we _then_ create a new LocalSnapshot: - # - content is in relpath - # - parents are ALL the parents (at least 2!) - # - one (or more) from conflict database, one from remotesnapshots - # - # Delete all conflict entries from the database (last? first?) - # - ## self._factory._locate_snapshot_service. - def found_new_remote(self, remote_snapshot, participant): """ A RemoteSnapshot that doesn't match our existing database entry @@ -396,7 +344,6 @@ def found_new_remote(self, remote_snapshot, participant): self._known_remotes = {remote_snapshot} else: self._known_remotes.add(remote_snapshot) - print("{} --[known]--> {}".format(remote_snapshot.relpath, [r.author.name for r in self._known_remotes])) self._remote_update(remote_snapshot, participant) return self.when_idle() From 75583d244e824f81ec9fa4df16be4d9b99f719ae Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 3 Nov 2023 01:08:39 -0600 Subject: [PATCH 13/67] author -> participant_name --- src/magic_folder/config.py | 6 ++---- src/magic_folder/magic_file.py | 8 ++++---- src/magic_folder/web.py | 6 +++--- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/magic_folder/config.py b/src/magic_folder/config.py index 082b10488..40c2c64b0 100644 --- a/src/magic_folder/config.py +++ b/src/magic_folder/config.py @@ -822,9 +822,8 @@ class Conflict(object): Represents information about a particular conflict. """ snapshot_cap = attr.ib() # Tahoe URI - author_name = attr.ib(validator=instance_of(str)) -## participant_name = attr.ib(validator=instance_of(str)) -# should probably re-name to "participant_name" since that's what it is ...? + participant_name = attr.ib(validator=instance_of(str)) + @attr.s class MagicFolderConfig(object): @@ -1615,7 +1614,6 @@ def add_conflict(self, cursor, snapshot, participant): VALUES (?,?,?) """, - ##(snapshot.relpath, snapshot.author.name, snapshot.capability.danger_real_capability_string()), (snapshot.relpath, participant.name, snapshot.capability.danger_real_capability_string()), ) diff --git a/src/magic_folder/magic_file.py b/src/magic_folder/magic_file.py index ca0568aee..df7d92846 100644 --- a/src/magic_folder/magic_file.py +++ b/src/magic_folder/magic_file.py @@ -309,7 +309,7 @@ def resolve_conflict(self, resolution): if resolution is None: keep_path = self._relpath rejected = [ - conflict_to_marker(self._relpath, con.author_name) + conflict_to_marker(self._relpath, con.participant_name) for con in conflicts ] else: @@ -317,9 +317,9 @@ def resolve_conflict(self, resolution): raise ResolutionError( "Resolution not found as existing conflict" ) - keep_path = conflict_to_marker(self._relpath, resolution.author_name) + keep_path = conflict_to_marker(self._relpath, resolution.participant_name) rejected = [ - conflict_to_marker(self._relpath, con.author_name) + conflict_to_marker(self._relpath, con.participant_name) for con in conflicts if con != resolution ] @@ -1105,7 +1105,7 @@ def _check_if_conflict_resolved(self, snapshot): if rs.danger_real_capability_string() in snapshot.parents_raw: conflicts = self._factory._config.list_conflicts_for(self._relpath) rejected = [ - conflict_to_marker(self._relpath, conflict.author_name) + conflict_to_marker(self._relpath, conflict.participant_name) for conflict in conflicts ] self._factory._magic_fs.mark_not_conflicted(self._relpath, self._relpath, rejected) diff --git a/src/magic_folder/web.py b/src/magic_folder/web.py index 6ffac5ee0..624df43ff 100644 --- a/src/magic_folder/web.py +++ b/src/magic_folder/web.py @@ -537,7 +537,7 @@ def list_conflicts(request, folder_name): folder_config = global_config.get_magic_folder(folder_name) return json.dumps({ relpath: [ - conflict.author_name + conflict.participant_name for conflict in conflicts ] for relpath, conflicts in folder_config.list_conflicts().items() @@ -584,7 +584,7 @@ def resolve_conflict(request, folder_name): matching_conflicts = [ conflict for conflict in conflicts - if conflict.author_name == participant_name + if conflict.participant_name == participant_name ] if not matching_conflicts: raise _InputError('"{relpath}" is not conflicted with "{take}"'.format(**resolution)) @@ -610,7 +610,7 @@ def resolve_conflict(request, folder_name): return json.dumps({ relpath: [ - conflict.author_name + conflict.participant_name for conflict in conflicts ] }).encode("utf8") From 56b0c896cb88d54fbd18539672eb302e06f05f16 Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 3 Nov 2023 01:34:53 -0600 Subject: [PATCH 14/67] test api --- src/magic_folder/test/test_client.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/magic_folder/test/test_client.py b/src/magic_folder/test/test_client.py index 572b5d5b5..465e4dadd 100644 --- a/src/magic_folder/test/test_client.py +++ b/src/magic_folder/test/test_client.py @@ -201,5 +201,21 @@ def test_join_read_only(self): body, { b"Content-Length": ["{}".format(len(body)).encode("utf8")], + + def test_resolve_conflict(self): + """ + The .../resolve-conflict API works + """ + return self._client_method_request( + "resolve-conflict", + ("folder_name", "foo/bar", "mine", None), + b"POST", + "http://invalid./v1/magic-folder/folder_name/resolve-conflict", + body=json.dumps({ + "relpath": "foo/bar", + "take": "mine", + }).encode("utf-8"), + extra_headers={ + b"Content-Length": [b"38"] }, ) From 0eb6b90032e26bcdff5334667a50c3574cc3c24d Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 3 Nov 2023 01:53:55 -0600 Subject: [PATCH 15/67] rename --- src/magic_folder/magic_file.py | 22 ++++++---------------- src/magic_folder/test/test_client.py | 20 +++++++++++++++++++- src/magic_folder/uploader.py | 6 ++++-- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/magic_folder/magic_file.py b/src/magic_folder/magic_file.py index df7d92846..1b2705e17 100644 --- a/src/magic_folder/magic_file.py +++ b/src/magic_folder/magic_file.py @@ -175,17 +175,7 @@ class ResolutionError(Exception): """ -class LocallyQueuedFiles(ResolutionError): - """ - We have locally queued changes when trying to apply a conflict - resolution. - """ - - def __str__(self): - return 'Cannot resolve "{}": we have {} local updates queued'.format(*self.args) - - -def conflict_to_marker(relpath, participant_name): +def conflict_marker_filename(relpath, participant_name): """ Convert a relpath to a conflict-marker for the given participant """ @@ -309,7 +299,7 @@ def resolve_conflict(self, resolution): if resolution is None: keep_path = self._relpath rejected = [ - conflict_to_marker(self._relpath, con.participant_name) + conflict_marker_filename(self._relpath, con.participant_name) for con in conflicts ] else: @@ -317,9 +307,9 @@ def resolve_conflict(self, resolution): raise ResolutionError( "Resolution not found as existing conflict" ) - keep_path = conflict_to_marker(self._relpath, resolution.participant_name) + keep_path = conflict_marker_filename(self._relpath, resolution.participant_name) rejected = [ - conflict_to_marker(self._relpath, con.participant_name) + conflict_marker_filename(self._relpath, con.participant_name) for con in conflicts if con != resolution ] @@ -941,7 +931,7 @@ def _mark_download_conflict(self, snapshot, staged_path, participant): """ Mark a conflict for this remote snapshot """ - conflict_path = conflict_to_marker(self._relpath, participant.name) + conflict_path = conflict_marker_filename(self._relpath, participant.name) self._factory._magic_fs.mark_conflict(self._relpath, conflict_path, staged_path) self._factory._config.add_conflict(snapshot, participant) @@ -1105,7 +1095,7 @@ def _check_if_conflict_resolved(self, snapshot): if rs.danger_real_capability_string() in snapshot.parents_raw: conflicts = self._factory._config.list_conflicts_for(self._relpath) rejected = [ - conflict_to_marker(self._relpath, conflict.participant_name) + conflict_marker_filename(self._relpath, conflict.participant_name) for conflict in conflicts ] self._factory._magic_fs.mark_not_conflicted(self._relpath, self._relpath, rejected) diff --git a/src/magic_folder/test/test_client.py b/src/magic_folder/test/test_client.py index 465e4dadd..3ccb4f5a6 100644 --- a/src/magic_folder/test/test_client.py +++ b/src/magic_folder/test/test_client.py @@ -202,7 +202,7 @@ def test_join_read_only(self): { b"Content-Length": ["{}".format(len(body)).encode("utf8")], - def test_resolve_conflict(self): + def test_resolve_conflict_take(self): """ The .../resolve-conflict API works """ @@ -219,3 +219,21 @@ def test_resolve_conflict(self): b"Content-Length": [b"38"] }, ) + + def test_resolve_conflict_use(self): + """ + The .../resolve-conflict API works + """ + return self._client_method_request( + "resolve-conflict", + ("folder_name", "foo/bar", None, "theirs"), + b"POST", + "http://invalid./v1/magic-folder/folder_name/resolve-conflict", + body=json.dumps({ + "relpath": "foo/bar", + "take": "mine", + }).encode("utf-8"), + extra_headers={ + b"Content-Length": [b"38"] + }, + ) diff --git a/src/magic_folder/uploader.py b/src/magic_folder/uploader.py index a97e69c78..b4f6c0c4e 100644 --- a/src/magic_folder/uploader.py +++ b/src/magic_folder/uploader.py @@ -32,6 +32,9 @@ ABSPATH, log_call_deferred, ) +from .magic_file import ( + conflict_marker_filename, +) from .snapshot import ( LocalAuthor, LocalSnapshot, @@ -140,9 +143,8 @@ def create_local_snapshot(self, path): if conflicts: # XXX check for conflict marker files (or not)! existing = 0 - from .magic_file import conflict_to_marker for con in conflicts: - fn = conflict_to_marker(relpath, con.author_name) + fn = conflict_marker_filename(relpath, con.author_name) if self._magic_dir.preauthChild(fn).exists(): existing += 1 if existing: From 741662f1023f38bf5c8121023a8b59a43b022759 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 20 Nov 2023 16:12:08 -0700 Subject: [PATCH 16/67] author_name -> participant_name --- src/magic_folder/uploader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/magic_folder/uploader.py b/src/magic_folder/uploader.py index b4f6c0c4e..5d9cae55a 100644 --- a/src/magic_folder/uploader.py +++ b/src/magic_folder/uploader.py @@ -144,7 +144,7 @@ def create_local_snapshot(self, path): # XXX check for conflict marker files (or not)! existing = 0 for con in conflicts: - fn = conflict_marker_filename(relpath, con.author_name) + fn = conflict_marker_filename(relpath, con.participant_name) if self._magic_dir.preauthChild(fn).exists(): existing += 1 if existing: From 98ed98704c9add2fd75bb537fb0272a0835c52fe Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 4 Dec 2023 13:04:09 -0700 Subject: [PATCH 17/67] add 'resolve' to the local API helper --- integration/util.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/integration/util.py b/integration/util.py index 9fb1f30d5..26ba0c318 100644 --- a/integration/util.py +++ b/integration/util.py @@ -540,6 +540,22 @@ def add_snapshot(self, folder_name, relpath): ], ) + def resolve(self, folder_name, magic_file, resolution): + """ + magic-folder resolve + """ + if resolution not in ["theirs", "mine"]: + raise ValueError("Invalid resolution: {}".format(resolution)) + return _magic_folder_runner( + self.reactor, self.request, self.name, + [ + "--config", self.magic_config_directory, + "resolve", + "--theirs" if resolution == "theirs" else "--mine", + magic_file, + ], + ) + def scan_folder(self, folder_name): """ magic-folder-api scan-folder From 072e0e9f3a9176990fdad7836608bebc0efb3fb0 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 31 Jan 2024 03:42:22 -0700 Subject: [PATCH 18/67] fix test? --- src/magic_folder/test/test_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/magic_folder/test/test_client.py b/src/magic_folder/test/test_client.py index 3ccb4f5a6..54af71a0e 100644 --- a/src/magic_folder/test/test_client.py +++ b/src/magic_folder/test/test_client.py @@ -231,9 +231,9 @@ def test_resolve_conflict_use(self): "http://invalid./v1/magic-folder/folder_name/resolve-conflict", body=json.dumps({ "relpath": "foo/bar", - "take": "mine", + "use": "theirs", }).encode("utf-8"), extra_headers={ - b"Content-Length": [b"38"] + b"Content-Length": [b"39"] }, ) From 4cca0100ef78512e6ce5077b93d6699278207821 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 12 Feb 2024 18:04:11 -0700 Subject: [PATCH 19/67] add 'reactor' arg to CLI --- src/magic_folder/api_cli.py | 47 +++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/magic_folder/api_cli.py b/src/magic_folder/api_cli.py index 2753549f1..20745499a 100644 --- a/src/magic_folder/api_cli.py +++ b/src/magic_folder/api_cli.py @@ -51,7 +51,7 @@ def postOptions(self): @inlineCallbacks -def cancel_invite(options): +def cancel_invite(reactor, options): """ Cancel a pending invite in a folder """ @@ -74,7 +74,7 @@ def postOptions(self): @inlineCallbacks -def list_invites(options): +def list_invites(reactor, options): """ List all pending invites for a folder """ @@ -100,7 +100,7 @@ def postOptions(self): @inlineCallbacks -def create_invite(options): +def create_invite(reactor, options): """ Create a new invite for a folder """ @@ -125,7 +125,7 @@ def postOptions(self): @inlineCallbacks -def await_invite(options): +def await_invite(reactor, options): """ Await a new invite for a folder """ @@ -163,7 +163,7 @@ def postOptions(self): @inlineCallbacks -def accept_invite(options): +def accept_invite(reactor, options): """ Accept a new invite for a folder """ @@ -194,7 +194,7 @@ def postOptions(self): @inlineCallbacks -def add_snapshot(options): +def add_snapshot(reactor, options): """ Add one new Snapshot of a particular file in a particular magic-folder. @@ -218,7 +218,7 @@ def postOptions(self): @inlineCallbacks -def file_status(options): +def file_status(reactor, options): """ List the status of all files in a magic-folder """ @@ -239,7 +239,7 @@ def postOptions(self): raise usage.UsageError("--folder / -n is required") -def dump_state(options): +def dump_state(reactor, options): """ Dump the database / state for a particular folder """ @@ -312,7 +312,7 @@ def postOptions(self): @inlineCallbacks -def add_participant(options): +def add_participant(reactor, options): """ Add one new participant to an existing magic-folder """ @@ -339,7 +339,7 @@ def postOptions(self): @inlineCallbacks -def list_participants(options): +def list_participants(reactor, options): """ List all participants in a magic-folder """ @@ -364,7 +364,7 @@ def postOptions(self): @inlineCallbacks -def list_conflicts(options): +def list_conflicts(reactor, options): """ List all conflicts in a magic-folder """ @@ -387,7 +387,7 @@ def postOptions(self): if self[arg] is None: raise usage.UsageError(error) -def scan(options): +def scan(reactor, options): return options.parent.client.scan_folder_local( options['folder'], ) @@ -406,7 +406,7 @@ def postOptions(self): if self[arg] is None: raise usage.UsageError(error) -def poll(options): +def poll(reactor, options): return options.parent.client.poll_folder_remote( options['folder'], ) @@ -439,7 +439,7 @@ def onMessage(self, payload, is_binary): @inlineCallbacks -def monitor(options): +def monitor(reactor, options): """ Print out updates from the WebSocket status API """ @@ -447,7 +447,7 @@ def monitor(options): endpoint_str = options.parent.api_client_endpoint websocket_uri = "{}/v1/status".format(endpoint_str.replace("tcp:", "ws://")) - agent = options.parent.get_websocket_agent() + agent = options.parent.get_websocket_agent(reactor) proto = yield agent.open( websocket_uri, { @@ -470,9 +470,8 @@ class MagicFolderApiCommand(BaseOptions): """ _websocket_agent = None # initialized (at most once) in get_websocket_agent() - def get_websocket_agent(self): + def get_websocket_agent(self, reactor): if self._websocket_agent is None: - from twisted.internet import reactor self._websocket_agent = create_client_agent(reactor) return self._websocket_agent @@ -533,7 +532,7 @@ def getUsage(self, width=None): @inlineCallbacks -def dispatch_magic_folder_api_command(args, stdout=None, stderr=None, client=None, +def dispatch_magic_folder_api_command(reactor, args, stdout=None, stderr=None, client=None, websocket_agent=None, config=None): """ Run a magic-folder-api command with the given args @@ -581,14 +580,16 @@ def dispatch_magic_folder_api_command(args, stdout=None, stderr=None, client=Non print(options, file=options.stdout) raise SystemExit(1) - yield run_magic_folder_api_options(options) + yield run_magic_folder_api_options(reactor, options) @inlineCallbacks -def run_magic_folder_api_options(options): +def run_magic_folder_api_options(reactor, options): """ Runs a magic-folder-api subcommand with the provided options. + :param reactor: the reactor we're using + :param options: already-parsed options. :returns: a Deferred which fires with the result of doing this @@ -619,11 +620,11 @@ def run_magic_folder_api_options(options): # we want to let exceptions out to the top level if --debug is on # because this gives better stack-traces if options['debug']: - yield maybeDeferred(main_func, so) + yield maybeDeferred(main_func, reactor, so) else: try: - yield maybeDeferred(main_func, so) + yield maybeDeferred(main_func, reactor, so) except CannotAccessAPIError as e: # give user more information if we can't find the daemon at all @@ -649,7 +650,7 @@ def _entry(): """ def main(reactor): - return dispatch_magic_folder_api_command(sys.argv[1:]) + return dispatch_magic_folder_api_command(reactor, sys.argv[1:]) return react(main) From 6671994fe4dddf0f83d5ca181ead6ce72ab6a783 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 13 Feb 2024 14:49:41 -0700 Subject: [PATCH 20/67] document HTTP API ..../resolve-conflict --- docs/interface.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/interface.rst b/docs/interface.rst index 53248fe3d..cf5fe4934 100644 --- a/docs/interface.rst +++ b/docs/interface.rst @@ -263,6 +263,21 @@ The author-names correspond to the device that conflicts with this file. There will also be a file named like ``.conflict-`` in the magic-folder whose contents match those of the conflicting remote file. +POST ``/v1/magic-folder//resolve-conflict`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Ask for a particular resolution to a conflicted file. +The body is a JSON object containing the keys: + +* ``relpath``: the file name inside the folder +* ``take``: ``"mine"`` or ``"theirs"`` indicating whose version to take +* ``use``: a Participant name indicating whose version to take + +Note that you can use only one of ``take`` or ``use``, and that ``take="theirs"`` only works when there is exactly one other conflict. + +Returns a ``dict`` mapping ``relpath`` to a list of all Participant names that conflicted before the resolution. + + GET ``/v1/magic-folder//scan-local`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From c0ef9ae9c18669ca317f97902296dd7b7a6b974a Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 13 Feb 2024 14:50:14 -0700 Subject: [PATCH 21/67] begin documenting conflicts / resolution --- docs/conflicts.rst | 89 ++++++++++++++++++++++++++++++++++++++++++++++ docs/datamodel.rst | 5 ++- 2 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 docs/conflicts.rst diff --git a/docs/conflicts.rst b/docs/conflicts.rst new file mode 100644 index 000000000..611e760da --- /dev/null +++ b/docs/conflicts.rst @@ -0,0 +1,89 @@ +.. -*- coding: utf-8 -*- + +.. _conflicts: + +Magic Folder Conflicts and Resolution +===================================== + +When we have two or more participants updating a folder, it can happen that a file is modified "at the same time" by two or more participants. +More correctly, "at the same time" here means "between communications". + +Effectively, participants are speaking a protocol by posting messages to their mutable Capabilities. +Namely, updading which Snapshot object a particular (file) name points to. + +We do not know *when* another Participant has updated their notion of the current state. +However, what we *can* notice is when they "commit" to that state (that is, they update publically their current Snapshot pointer). + +Each file is considered individually, and is represented by a tree of Snapshots of the contents (plus metadata). +Included in the metadata are one or more "parent" Snapshots (a brand new file has zero parents). + +For more on this, the :ref:`datamodel` has a section on :ref:`datamodel_conflicts`. + + +Communication Between Folders +----------------------------- + +Consider a single file named ``"foo"``. +When this file is created (for example on the device called Laptop) that device uploads a Snapshot (``S0``) with no parent Snapshots. + +By "upload" we mean to push the content and metadata to the Tahoe-LAFS client, receiving ultimately a Capability for the Snapshot ``S0``. +The file entry for ``"foo"`` is then updated, visualized as: ``foo -> S0`` + +Now, all other Participants can (and, eventually, will) notice this update when they poll the magic-folder. + +Each of these is a "communication event", so we talk about the regions between these as a cohesive state of that client. +That is, anything that happens during that time is "at the same time" for the purposes of this protocol. + +Note that this time interval could be as short as a few seconds or as long as days (or more). + +Observe too that the "parents" of a particular Snapshot are a commitment as to the state visible to that client when creating the Snapshot. + + +Detecting Conflicts +------------------- + +A new update is communicated to us -- that is, we've downloaded a previously-unknown Snapshot from some other Participant's mutable Capbility. + +We now look at our own Snapshot for the corresponding file. +Either our Snapshot appears in some parents (including grandparents, etc) of the new Snapshot, or it doesn't. + +If it does appear, this is an "update" (and we simply replace the local content with the incoming content). + +Instead if our Snapshot does not appear as any ancestor of the incoming Snapshot, a commit is determined. + +This is because when the other device created their Snapshot, they didn't know about ours (or else it would appear as an ancestor) so we have made a change "at the same time". + +Note that unlike tools like Git, we do not examine the contents of the file or try to produce differences -- everything is determined from the metadata in the Snapshot. + + +Showing Conflicts Locally +------------------------- + +Once a conflict is detected, "conflict marker" files are put into the local magic folder location (our local file remains unmodified, and something like ``foo.conflict-desktop`` will appear. +The state database is also updated (conflicts can also be listed via the API and CLI) + + +Resolving Conflicts +------------------- + +We cannot "magically" solve a conflict: two devices produced new Snapshots while not communicating with each other. + +Thus, it is up to the humans using these devices to determine what happens. +Once appropriate changes are decided upon, a new Snapshot is produced with *two or more parents*: one parent for each of the Snapshots involved in the conflict (we've only talked about one other participant so far, but there could be more). + +Such a Snapshot (with two or more parents) indicates to the other clients a particular resoltion to the conflict has been decided. + +So there's another case when we see an incoming new Snapshot: it may in fact *resolve an existing* conflict. + + +Resolving via Filesystem +------------------------ + + +Resolving via the CLI +--------------------- + + +Resolving via the HTTP API +-------------------------- + diff --git a/docs/datamodel.rst b/docs/datamodel.rst index 8096def08..e1a65c9c2 100644 --- a/docs/datamodel.rst +++ b/docs/datamodel.rst @@ -123,7 +123,8 @@ If any Snapshot is different, it is downloaded and acted upon. For a full discussion of this process, see :ref:`downloader`. Ultimately, for normal updates or deletes, the change will be reflected (or "acknowledged" if you prefer) by updating our own Personal folder after making local changes. -In case of a "conflict" (e.g. two changes at "the same" time) we will not update the Personal folder until the user resolves the conflict (this part isn't possible yet, see `Issue 102 `_). +In case of a "conflict" (e.g. two or more changes at "the same" time) we will not update the Personal folder until some participant resolves the conflict. +See also :ref:`conflicts` Considered together, an abstract view of a two-Participant example: @@ -140,6 +141,8 @@ There is a single file (``grumpy-cat.jpeg``) which has been changed once (the or We can see that both Participants are up-to-date because both Personal folders point at the latest Snapshot. +.. _datamodel_conflicts: + Conflicts --------- From 343c91b1aae33141dee4f2153eb1e51f11eb8467 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 13 Feb 2024 15:03:12 -0700 Subject: [PATCH 22/67] doc refs --- docs/conflicts.rst | 1 + docs/interface.rst | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/conflicts.rst b/docs/conflicts.rst index 611e760da..75a62f156 100644 --- a/docs/conflicts.rst +++ b/docs/conflicts.rst @@ -87,3 +87,4 @@ Resolving via the CLI Resolving via the HTTP API -------------------------- +See :ref:`api_resolve_conflict` diff --git a/docs/interface.rst b/docs/interface.rst index cf5fe4934..3e717b113 100644 --- a/docs/interface.rst +++ b/docs/interface.rst @@ -262,6 +262,7 @@ Each item in the ``dict`` maps a relpath to a list of author-names. The author-names correspond to the device that conflicts with this file. There will also be a file named like ``.conflict-`` in the magic-folder whose contents match those of the conflicting remote file. +.. _api_resolve_conflict: POST ``/v1/magic-folder//resolve-conflict`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From d0ff3172b8f0ded30403476e4d4eaa285cf39d10 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 13 Feb 2024 17:50:44 -0700 Subject: [PATCH 23/67] fix assert --- src/magic_folder/downloader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/magic_folder/downloader.py b/src/magic_folder/downloader.py index 532b94064..8253a7df6 100644 --- a/src/magic_folder/downloader.py +++ b/src/magic_folder/downloader.py @@ -187,7 +187,7 @@ def is_ancestor_of(self, target_cap, child_cap): # only incrementally adds to the ancestors of the remote # - for checking in the other direction, we can skip checking parents of any ancestors that are # also ancestors of our remotesnapshot - assert child_cap.danger_real_capability_string() in self._cached_snapshots is not None, "Remote should be cached already" + assert child_cap.danger_real_capability_string() in self._cached_snapshots, "Remote should be cached already" snapshot = self._cached_snapshots[child_cap.danger_real_capability_string()] q = deque([snapshot]) From 5300aa4dd51cd516bca82c41c5613363456d8412 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 13 Feb 2024 17:51:48 -0700 Subject: [PATCH 24/67] working 3-party conflict removal --- src/magic_folder/magic_file.py | 51 ++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/src/magic_folder/magic_file.py b/src/magic_folder/magic_file.py index 1b2705e17..a4dacac71 100644 --- a/src/magic_folder/magic_file.py +++ b/src/magic_folder/magic_file.py @@ -334,7 +334,23 @@ def found_new_remote(self, remote_snapshot, participant): self._known_remotes = {remote_snapshot} else: self._known_remotes.add(remote_snapshot) - self._remote_update(remote_snapshot, participant) + + # if we're already conflicted, this will indeed not "match our + # existing database entry" (as per the docstring) -- but it + # may still be an "already known" update becaus we've already + # seen it and marked it as a conflict + found = False + for conflict in self._factory._config.list_conflicts_for(self._relpath): + if remote_snapshot.capability == conflict.snapshot_cap: + found = True + + # note that we'll emit this signal even when we're already + # conflicted, but detect a new conflict (in which case we + # produce a new conflict-marker or update existing) + + if not found: + print("found new remote", remote_snapshot) + self._remote_update(remote_snapshot, participant) return self.when_idle() def local_snapshot_exists(self, local_snapshot): @@ -672,6 +688,22 @@ def _check_local_update(self, snapshot, staged_path, participant): self._call_later(self._download_mismatch, snapshot, staged_path, participant) return + # this incoming snapshot might be the resolution to a remote + # conflict; if so we can remove the appropriate conflict + # entries and markers... + if len(snapshot.parents_raw) > 1: + rs = self._factory._config.get_remotesnapshot(self._relpath) + # XXX don't we have to do an ancestor check, basically? + # like "are we in ANY of the ancestors of this remote?" + if rs.danger_real_capability_string() in snapshot.parents_raw: + conflicts = self._factory._config.list_conflicts_for(self._relpath) + rejected = [ + conflict_marker_filename(self._relpath, conflict.participant_name) + for conflict in conflicts + ] + self._factory._magic_fs.mark_not_conflicted(self._relpath, self._relpath, rejected) + self._factory._config.resolve_conflict(self._relpath) + self._call_later(self._download_matches, snapshot, staged_path, local_pathinfo.state, participant) @_machine.output() @@ -1390,14 +1422,17 @@ def _done_working(self): ) _conflicted.upon( _remote_update, - enter=_conflicted, - outputs=[_check_if_conflict_resolved], - ) - _conflicted.upon( - _resolved_remotely, - enter=_checking_for_local_work, - outputs=[_check_for_local_work], + enter=_downloading, + outputs=[_working, _status_download_queued, _begin_download] ) + # XXX where do we? outputs=[_check_if_conflict_resolved_or_additional], + # (i guess what we want to do is ... do this in "_check_ancestor" or "_check_local_update") + +# _conflicted.upon( +# _resolved_remotely, +# enter=_checking_for_local_work, +# outputs=[_check_for_local_work], +# ) _conflicted.upon( _local_update, enter=_conflicted, From 9f42e58b31396a5a406bc9ee07c4d9deeb29a869 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 13 Feb 2024 17:52:31 -0700 Subject: [PATCH 25/67] more docs --- docs/conflicts.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/conflicts.rst b/docs/conflicts.rst index 75a62f156..f54ac1fe1 100644 --- a/docs/conflicts.rst +++ b/docs/conflicts.rst @@ -83,6 +83,18 @@ Resolving via Filesystem Resolving via the CLI --------------------- +The subcommand ``magic-folder resolve`` may be used to specify a resolution. +It allows you to choose ``--mine`` or ``--theirs``(if there is only one other conflict). +Otherwise, you must use ``--use `` to specify which version to keep. + +Currently there is no API for doing something more complex (e.g. simultaneuously replacing the latest version with new content). + +Complete example: + +.. code-block:: console + + magic-folder resolve --mine ~/Documents/Magic/foo + Resolving via the HTTP API -------------------------- From a63c1668459c61d7ec26798e5904792ccab08cb5 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 13 Feb 2024 17:58:22 -0700 Subject: [PATCH 26/67] unused --- src/magic_folder/magic_file.py | 35 ---------------------------------- 1 file changed, 35 deletions(-) diff --git a/src/magic_folder/magic_file.py b/src/magic_folder/magic_file.py index a4dacac71..b30331dbf 100644 --- a/src/magic_folder/magic_file.py +++ b/src/magic_folder/magic_file.py @@ -1107,33 +1107,6 @@ def do_remote_update(done_d, snap, part): return self._call_later(self._no_download_work, None) - @_machine.output() - def _check_if_conflict_resolved(self, snapshot): - """ - We are conflicted and have seen a remote update -- chcek if update - is a fix for the conflict. - """ - # the conflict is 'resolved' if this remote-snapshot has - # multiple parents, and "our" Snapshot is one of them -- XXX - # actually isn't it more general like if _any_ ancestor has - # it. The remote could have: had a "resolve" Snapshot, then - # any number of "normal" snapshots after that (before we see - # anything) ... so we may have to cache them etc right? some - # async-work ... - if len(snapshot.parents_raw) > 1: - rs = self._factory._config.get_remotesnapshot(self._relpath) - # XXX don't we have to do an ancestor check, basically? - # like "are we in ANY of the ancestors of this remote?" - if rs.danger_real_capability_string() in snapshot.parents_raw: - conflicts = self._factory._config.list_conflicts_for(self._relpath) - rejected = [ - conflict_marker_filename(self._relpath, conflict.participant_name) - for conflict in conflicts - ] - self._factory._magic_fs.mark_not_conflicted(self._relpath, self._relpath, rejected) - self._factory._config.resolve_conflict(self._relpath) - self._call_later(self._resolved_remotely) - @_machine.output() def _working(self): """ @@ -1425,14 +1398,6 @@ def _done_working(self): enter=_downloading, outputs=[_working, _status_download_queued, _begin_download] ) - # XXX where do we? outputs=[_check_if_conflict_resolved_or_additional], - # (i guess what we want to do is ... do this in "_check_ancestor" or "_check_local_update") - -# _conflicted.upon( -# _resolved_remotely, -# enter=_checking_for_local_work, -# outputs=[_check_for_local_work], -# ) _conflicted.upon( _local_update, enter=_conflicted, From 526c32f34d2d45f581477346135b72cad5a49b95 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 13 Feb 2024 18:29:48 -0700 Subject: [PATCH 27/67] ignore dupes for conflicts --- src/magic_folder/config.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/magic_folder/config.py b/src/magic_folder/config.py index 40c2c64b0..e0e057088 100644 --- a/src/magic_folder/config.py +++ b/src/magic_folder/config.py @@ -1600,7 +1600,8 @@ def list_conflicts_for(self, cursor, relpath): @with_cursor def add_conflict(self, cursor, snapshot, participant): """ - Add a new conflicting author + Add a new conflicting author. It is fine if the same conflict + already exists. :param RemoteSnapshot snapshot: the conflicting Snapshot @@ -1609,7 +1610,7 @@ def add_conflict(self, cursor, snapshot, participant): with start_action(action_type="config:state-db:add-conflict", relpath=snapshot.relpath): cursor.execute( """ - INSERT INTO + INSERT OR IGNORE INTO conflicted_files (relpath, conflict_author, snapshot_cap) VALUES (?,?,?) From 4df9c5328f01a8af199e559ad13f8e7426474fbe Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 14 Feb 2024 16:38:37 -0700 Subject: [PATCH 28/67] fixup: more places need reactor arg --- src/magic_folder/test/cli/common.py | 7 +++++-- src/magic_folder/test/cli/test_api_cli.py | 4 ++-- src/magic_folder/test/test_api_cli.py | 18 ++++++++++++++++-- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/magic_folder/test/cli/common.py b/src/magic_folder/test/cli/common.py index 779717b1b..8605cb073 100644 --- a/src/magic_folder/test/cli/common.py +++ b/src/magic_folder/test/cli/common.py @@ -1,5 +1,6 @@ from contextlib import contextmanager from io import StringIO +from functools import partial from twisted.python import usage from twisted.python.usage import ( @@ -155,6 +156,7 @@ def _run_cli(command, argv, global_config=None, http_client=None): result, )) + def cli(argv, global_config=None, http_client=None): """ Perform an in-process equivalent to the given magic-folder command. @@ -176,7 +178,8 @@ def cli(argv, global_config=None, http_client=None): command = Command(MagicFolderCommand, run_magic_folder_options) return _run_cli(command, argv, global_config, http_client) -def api_cli(argv, global_config=None, http_client=None): + +def api_cli(reactor, argv, global_config=None, http_client=None): """ Perform an in-process equivalent to the given magic-folder-api command. @@ -194,5 +197,5 @@ def api_cli(argv, global_config=None, http_client=None): :return Deferred[ProcessOutcome]: The side-effects and result of the process. """ - command = Command(MagicFolderApiCommand, run_magic_folder_api_options) + command = Command(MagicFolderApiCommand, partial(run_magic_folder_api_options, reactor)) return _run_cli(command, argv, global_config, http_client) diff --git a/src/magic_folder/test/cli/test_api_cli.py b/src/magic_folder/test/cli/test_api_cli.py index 828ceabb8..1e0c007d1 100644 --- a/src/magic_folder/test/cli/test_api_cli.py +++ b/src/magic_folder/test/cli/test_api_cli.py @@ -17,10 +17,10 @@ class ScanMagicFolder(AsyncTestCase): def api_cli(self, argv): - return api_cli(argv, self.node.global_config, self.node.http_client) + return api_cli(self.reactor, argv, self.node.global_config, self.node.http_client) def cli(self, argv): - return cli(argv, self.node.global_config, self.node.http_client) + return cli(self.reactor, argv, self.node.global_config, self.node.http_client) @inline_callbacks def setUp(self): diff --git a/src/magic_folder/test/test_api_cli.py b/src/magic_folder/test/test_api_cli.py index 3cf531be4..45c1590f4 100644 --- a/src/magic_folder/test/test_api_cli.py +++ b/src/magic_folder/test/test_api_cli.py @@ -134,6 +134,7 @@ def test_happy(self): ) with request_sequence.consume(self.fail): yield dispatch_magic_folder_api_command( + Clock(), ["--config", self.magic_config.path, "add-snapshot", "--file", "foo", "--folder", "default"], @@ -190,6 +191,7 @@ def test_bad_file(self): with self.assertRaises(SystemExit): with request_sequence.consume(self.fail): yield dispatch_magic_folder_api_command( + Clock(), ["--config", self.magic_config.path, "add-snapshot", "--file", "../../../foo", "--folder", "default"], @@ -270,7 +272,7 @@ def test_empty_command_prints_help(self): """ stdout = StringIO() with self.assertRaises(SystemExit): - yield dispatch_magic_folder_api_command([], stdout=stdout) + yield dispatch_magic_folder_api_command(Clock(), [], stdout=stdout) self.assertThat( stdout.getvalue(), @@ -290,6 +292,7 @@ def test_version(self): self.assertThat( dispatch_magic_folder_api_command( + Clock(), ["--version"], stdout=stdout, stderr=stderr, @@ -317,6 +320,7 @@ def test_no_file_arg(self): self.assertThat( dispatch_magic_folder_api_command( + Clock(), ["add-snapshot"], stdout=stdout, stderr=stderr, @@ -343,6 +347,7 @@ def test_no_folder_arg(self): self.assertThat( dispatch_magic_folder_api_command( + Clock(), ["add-snapshot", "--file", "foo"], stdout=stdout, stderr=stderr, @@ -392,6 +397,7 @@ def error(*args, **kw): with self.assertRaises(SystemExit): yield dispatch_magic_folder_api_command( + Clock(), ["--config", basedir.path, "add-snapshot", "--file", "foo", "--folder", "default"], @@ -460,6 +466,7 @@ def test_api_error(self): with self.assertRaises(SystemExit): with request_sequence.consume(self.fail): yield dispatch_magic_folder_api_command( + Clock(), ["--config", basedir.path, "add-snapshot", "--file", "foo", "--folder", "default"], @@ -507,6 +514,7 @@ def error(*args, **kw): with self.assertRaises(SystemExit): yield dispatch_magic_folder_api_command( + Clock(), ["--config", basedir.path, "add-snapshot", "--file", "foo", "--folder", "default"], @@ -545,6 +553,7 @@ def test_happy(self): author = create_local_author("zara") magic_path = FilePath(self.mktemp()) + self.reactor = MemoryReactorClockResolver() magic_path.makedirs() config = self.global_config.create_magic_folder( name="test", @@ -596,7 +605,7 @@ def test_happy(self): "--folder", "test", ]) options._config = self.global_config - yield run_magic_folder_api_options(options) + yield run_magic_folder_api_options(self.reactor, options) self.assertThat( options.stderr.getvalue(), @@ -654,6 +663,7 @@ def test_add_participant_missing_arg(self): # missing --personal-dmd with self.assertRaises(SystemExit): yield dispatch_magic_folder_api_command( + Clock(), ["--config", self.magic_config.path, "add-participant", "--folder", "default", "--author-name", "amaya", @@ -718,6 +728,7 @@ def XXXtest_add_participant(self): ) with request_sequence.consume(self.fail): yield dispatch_magic_folder_api_command( + reactor, ["--config", self.magic_config.path, "add-participant", "--folder", "default", "--author-name", "amaya", @@ -747,6 +758,7 @@ def test_list_participants_missing_arg(self): # --folder missing with self.assertRaises(SystemExit): yield dispatch_magic_folder_api_command( + Clock(), ["--config", self.magic_config.path, "list-participants", ], stdout=stdout, @@ -802,6 +814,7 @@ def test_list_participants(self): ) with request_sequence.consume(self.fail): yield dispatch_magic_folder_api_command( + Clock(), ["--config", self.magic_config.path, "list-participants", "--folder", "default", ], @@ -859,6 +872,7 @@ def test_once(self): stderr = StringIO() yield dispatch_magic_folder_api_command( + self.reactor, ["--config", self.magic_config.path, "monitor", "--once", ], From b6256405eb343b01127cf029b2fb431fab18fe97 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 14 Feb 2024 16:51:08 -0700 Subject: [PATCH 29/67] conflicted state tests/notes about handling --- src/magic_folder/magic_file.py | 13 +++++++++++++ src/magic_folder/test/test_config.py | 5 ++--- src/magic_folder/test/test_upload.py | 26 ++++++++------------------ 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/magic_folder/magic_file.py b/src/magic_folder/magic_file.py index b30331dbf..96b3afbc2 100644 --- a/src/magic_folder/magic_file.py +++ b/src/magic_folder/magic_file.py @@ -1398,6 +1398,19 @@ def _done_working(self): enter=_downloading, outputs=[_working, _status_download_queued, _begin_download] ) + # XXX this is trix-y .. the user could be changing the local + # contents because they'll ultimately be doing a "resolve" (with + # those contents) __OR__ they could be naively simply editing the + # file more. + # + # ...so I think we _do_ want to queue a local-update: an incoming + # resolution may happen, and if we have no local change: great, + # apply it. But if we _do_ have a local change, we'd want to + # .. conflict again? + # + # need integration test here: have conflict; A changes local file + # (but does not "resolve"); B uploads resolution; A shouldn't lose + # user data (i.e. delete local-only changes with the resolution) _conflicted.upon( _local_update, enter=_conflicted, diff --git a/src/magic_folder/test/test_config.py b/src/magic_folder/test/test_config.py index cb95c6b63..2944a1abf 100644 --- a/src/magic_folder/test/test_config.py +++ b/src/magic_folder/test/test_config.py @@ -1342,7 +1342,7 @@ def test_add_list_conflict(self, remote_cap, meta_cap, content_cap): ) def test_add_conflict_twice(self, remote_cap, meta_cap, content_cap): """ - It's an error to add the same conflict twice + It is not an error to add the same conflict twice """ participants = static_participants( names=["marlyn"], @@ -1360,8 +1360,7 @@ def test_add_conflict_twice(self, remote_cap, meta_cap, content_cap): ) self.db.add_conflict(snap, participants.list()[0]) - with self.assertRaises(sqlite3.IntegrityError): - self.db.add_conflict(snap, participants.list()[0]) + self.db.add_conflict(snap, participants.list()[0]) self.db.resolve_conflict("foo") @given( diff --git a/src/magic_folder/test/test_upload.py b/src/magic_folder/test/test_upload.py index 83c2f8f17..92ae6078f 100644 --- a/src/magic_folder/test/test_upload.py +++ b/src/magic_folder/test/test_upload.py @@ -283,37 +283,27 @@ def test_existing_conflict(self, upload_dircap): # mark it as a conflict config.add_conflict(snap, static_participants(names=["existing"]).list()[0]) - # create a MagicFile file for this relpath now + # create a MagicFile file for this relpath now (i.e. _after_ + # marking the conflict) mf = f.magic_file_factory.magic_file_for(local) # we can't know the current state, but we can see what it does + # ... set ourselves as the trace function transitions = [] def trace(*args): transitions.append(args) mf.set_trace(trace) - # send in a remote update; if we were already conflicted it'll - # loop into that state and stay conflicted .. otherwise it'll - # try to upload - child = RemoteSnapshot( - relpath, - author, - metadata={ - "modification_time": int(1234), - }, - capability=random_immutable(directory=True), - parents_raw=[snap.capability.danger_real_capability_string()], - content_cap=random_immutable(), - metadata_cap=random_immutable(), - ) - participants = static_participants() - mf.found_new_remote(child, participants.participants[0]) + # if we do a local_update while in _conflicted, we should stay + # in _conflicted + # (...although .. shouldn't we queue a local update?) + mf.create_update() self.assertThat( transitions, Equals([ - ('_conflicted', '_remote_update', '_conflicted'), + ('_conflicted', '_local_update', '_conflicted'), ]) ) From 9926ee66fdfd28bac0888dba4b1d80957084f72a Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Feb 2024 19:21:35 -0700 Subject: [PATCH 30/67] incorrect merge --- src/magic_folder/test/test_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/magic_folder/test/test_client.py b/src/magic_folder/test/test_client.py index 54af71a0e..8a24cc2c4 100644 --- a/src/magic_folder/test/test_client.py +++ b/src/magic_folder/test/test_client.py @@ -235,5 +235,5 @@ def test_resolve_conflict_use(self): }).encode("utf-8"), extra_headers={ b"Content-Length": [b"39"] - }, + } ) From ce31aa0baf42f87cbab4bfd97e52c351c62e3b4d Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Feb 2024 22:03:34 -0700 Subject: [PATCH 31/67] linting --- src/magic_folder/test/test_api_cli.py | 2 +- src/magic_folder/test/test_config.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/magic_folder/test/test_api_cli.py b/src/magic_folder/test/test_api_cli.py index 45c1590f4..5cd949104 100644 --- a/src/magic_folder/test/test_api_cli.py +++ b/src/magic_folder/test/test_api_cli.py @@ -728,7 +728,7 @@ def XXXtest_add_participant(self): ) with request_sequence.consume(self.fail): yield dispatch_magic_folder_api_command( - reactor, + Clock(), ["--config", self.magic_config.path, "add-participant", "--folder", "default", "--author-name", "amaya", diff --git a/src/magic_folder/test/test_config.py b/src/magic_folder/test/test_config.py index 2944a1abf..091e171eb 100644 --- a/src/magic_folder/test/test_config.py +++ b/src/magic_folder/test/test_config.py @@ -1,4 +1,3 @@ -import sqlite3 import itertools from io import ( BytesIO, From 029575b81e2b1d0d18693a78b0e1147f99c5b48d Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 18 Feb 2024 21:25:54 -0700 Subject: [PATCH 32/67] not really valid test --- integration/test_multiuser.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/integration/test_multiuser.py b/integration/test_multiuser.py index 153a8aa68..6c8407b66 100644 --- a/integration/test_multiuser.py +++ b/integration/test_multiuser.py @@ -207,16 +207,12 @@ async def test_conflicted_users(request, reactor, temp_filepath, alice, bob, edm # now, wait for updates all_updates = await DeferredList([ - alice.status_monitor(how_long=20), - bob.status_monitor(how_long=20), - edmond.status_monitor(how_long=20), + alice.status_monitor(how_long=5), + bob.status_monitor(how_long=5), + edmond.status_monitor(how_long=5), ]) - # ensure we don't "keep downloading" when there's a conflict - for st, updates in all_updates: - assert st, "status streaming failed" - assert len([e for e in updates if e["kind"] == "download-queued"]) < 2, "too many downloads queued" - # everyone should have a conflict though... + # everyone should have a conflict... assert find_conflicts(magic) != [], "alice should have conflicts" assert find_conflicts(magic_bob) != [], "bob should have conflicts" assert find_conflicts(magic_ed) != [], "edmond should have conflicts" From de9bfbfd3e00f3a47332108bb18f653253915ddc Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 18 Feb 2024 21:31:54 -0700 Subject: [PATCH 33/67] lint --- integration/test_multiuser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/test_multiuser.py b/integration/test_multiuser.py index 6c8407b66..5cab732be 100644 --- a/integration/test_multiuser.py +++ b/integration/test_multiuser.py @@ -206,7 +206,7 @@ async def test_conflicted_users(request, reactor, temp_filepath, alice, bob, edm # shouldn't _keep_ trying to download conflicted updates. # now, wait for updates - all_updates = await DeferredList([ + await DeferredList([ alice.status_monitor(how_long=5), bob.status_monitor(how_long=5), edmond.status_monitor(how_long=5), From fb13dcc3e83907c76169a8138b97bc508b439415 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 27 Mar 2024 11:49:55 -0600 Subject: [PATCH 34/67] copyedit --- docs/conflicts.rst | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/conflicts.rst b/docs/conflicts.rst index f54ac1fe1..defedc7c3 100644 --- a/docs/conflicts.rst +++ b/docs/conflicts.rst @@ -6,10 +6,10 @@ Magic Folder Conflicts and Resolution ===================================== When we have two or more participants updating a folder, it can happen that a file is modified "at the same time" by two or more participants. -More correctly, "at the same time" here means "between communications". +More correctly, "at the same time" here means "between communications events". -Effectively, participants are speaking a protocol by posting messages to their mutable Capabilities. -Namely, updading which Snapshot object a particular (file) name points to. +Effectively, participants are speaking a protocol by posting updates to their mutable Capabilities. +Namely, updating which Snapshot object a particular (file) name points to (including adding a new name, or removing one). We do not know *when* another Participant has updated their notion of the current state. However, what we *can* notice is when they "commit" to that state (that is, they update publically their current Snapshot pointer). @@ -49,11 +49,12 @@ Either our Snapshot appears in some parents (including grandparents, etc) of the If it does appear, this is an "update" (and we simply replace the local content with the incoming content). -Instead if our Snapshot does not appear as any ancestor of the incoming Snapshot, a commit is determined. +Instead if our Snapshot does not appear as any ancestor of the incoming Snapshot, a conflict is determined. This is because when the other device created their Snapshot, they didn't know about ours (or else it would appear as an ancestor) so we have made a change "at the same time". Note that unlike tools like Git, we do not examine the contents of the file or try to produce differences -- everything is determined from the metadata in the Snapshot. +This means that even if we happened to make the very same edits "at the same time" it would still be a conflict. Showing Conflicts Locally @@ -84,7 +85,7 @@ Resolving via the CLI --------------------- The subcommand ``magic-folder resolve`` may be used to specify a resolution. -It allows you to choose ``--mine`` or ``--theirs``(if there is only one other conflict). +It allows you to choose ``--mine`` or ``--theirs`` (if there is only one other conflict). Otherwise, you must use ``--use `` to specify which version to keep. Currently there is no API for doing something more complex (e.g. simultaneuously replacing the latest version with new content). From 063c6274f1a3a6625eb7adfc22f7955475415889 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 27 Mar 2024 12:27:01 -0600 Subject: [PATCH 35/67] rebase resolution was wrong --- src/magic_folder/test/test_client.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/magic_folder/test/test_client.py b/src/magic_folder/test/test_client.py index 8a24cc2c4..893f5eea6 100644 --- a/src/magic_folder/test/test_client.py +++ b/src/magic_folder/test/test_client.py @@ -201,6 +201,8 @@ def test_join_read_only(self): body, { b"Content-Length": ["{}".format(len(body)).encode("utf8")], + } + ) def test_resolve_conflict_take(self): """ From 9fcec34d92088dd8393d3332c38169a9035530f9 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 27 Mar 2024 12:27:46 -0600 Subject: [PATCH 36/67] news --- newsfragments/725.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/725.feature diff --git a/newsfragments/725.feature b/newsfragments/725.feature new file mode 100644 index 000000000..edf933076 --- /dev/null +++ b/newsfragments/725.feature @@ -0,0 +1 @@ +Ability to resolve conflicted files. \ No newline at end of file From b5354d3827ae620a74845d99eead2bcb1a19fe5a Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 27 Mar 2024 13:02:35 -0600 Subject: [PATCH 37/67] Windows can't move onto existing file --- src/magic_folder/downloader.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/magic_folder/downloader.py b/src/magic_folder/downloader.py index 8253a7df6..7de3ae68a 100644 --- a/src/magic_folder/downloader.py +++ b/src/magic_folder/downloader.py @@ -427,6 +427,14 @@ def mark_conflict(self, relpath, conflict_path, staged_content): content. """ local_path = self.magic_path.preauthChild(conflict_path) + # on windows, it's an error to "os.rename()" on top of an + # existing file so we delete it first if it exits (because we + # want the content to be "the newest conflict" in case it + # already existed) + try: + local_path.remove() + except OSError: + pass staged_content.moveTo(local_path) From d69b3c684129a327a37db69c4a1d9e89cad7dd88 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 28 Mar 2024 00:00:31 -0600 Subject: [PATCH 38/67] integration test for conflict-resolution --- integration/test_resolve_conflict.py | 103 +++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 integration/test_resolve_conflict.py diff --git a/integration/test_resolve_conflict.py b/integration/test_resolve_conflict.py new file mode 100644 index 000000000..fe89c98bc --- /dev/null +++ b/integration/test_resolve_conflict.py @@ -0,0 +1,103 @@ +""" +Testing synchronizing files between 3 or more participants +""" + +from eliot.twisted import ( + inline_callbacks, +) +import pytest_twisted + +from .util import ( + find_conflicts, +) +from twisted.internet.defer import DeferredList + + +def non_lit_content(s): + # type: (str) -> bytes + """ + Pad the given string so it is long enough to not fit in a tahoe literal + URI. + """ + # The max size of data that will be stored in a literal tahoe cap is 55. + # See allmydata.immutable.upload.Uploader.URI_LIT_SIZE_THRESHOLD + # We don't need to be exactly longer than that threshold, as long as we + # are over it. + return "{} {}\n".format(s, "." * max(55 - len(s), 0)).encode("utf8") + + +async def perform_invite(request, folder_name, inviter, invitee_name, invitee, invitee_magic_fp, read_only=False): + invitee_magic_fp.makedirs() + + code, magic_proto, process_transport = await inviter.invite(folder_name, invitee_name) + await invitee.join( + code, + folder_name, + invitee_magic_fp.path, + invitee_name, + poll_interval=1, + scan_interval=1, + read_only=read_only, + ) + + def cleanup_invitee(): + pytest_twisted.blockon(invitee.leave(folder_name)) + request.addfinalizer(cleanup_invitee) + + await magic_proto.exited + print(f"{invitee_name} successfully invited to {folder_name}") + + +@inline_callbacks +@pytest_twisted.ensureDeferred +async def test_conflicted_users(request, reactor, temp_filepath, alice, bob): + """ + Two users both add the same file at the same time, producing conflicts. + + One user resolves the conflict. + """ + + magic = temp_filepath.child("magic-alice") + magic.makedirs() + + await alice.add(request, "conflict", magic.path) + + # invite some friends + magic_bob = temp_filepath.child("magic-bob") + await perform_invite(request, "conflict", alice, "robert", bob, magic_bob) + + # add the same file at "the same" time + content0 = non_lit_content("very-secret") + magic.child("summertime.txt").setContent(content0) + magic_bob.child("summertime.txt").setContent(content0) + + await DeferredList([ + alice.add_snapshot("conflict", "summertime.txt"), + bob.add_snapshot("conflict", "summertime.txt"), + ]) + # we've added all the files on all participants .. they _should_ conflict, but also we + # shouldn't _keep_ trying to download conflicted updates. + + # now, wait for updates + await DeferredList([ + alice.status_monitor(how_long=20), + bob.status_monitor(how_long=20), + ]) + + # everyone should have a conflict... + assert find_conflicts(magic) != [], "alice should have conflicts" + assert find_conflicts(magic_bob) != [], "bob should have conflicts" + + # resolve the conflict + alice.resolve("conflict", magic.child("summertime.txt").path, "theirs") + + # wait for updates + x = await DeferredList([ + alice.status_monitor(how_long=20), + bob.status_monitor(how_long=20), + ]) + + # no more conflicts + assert find_conflicts(magic) == [], "alice has conflicts" + assert find_conflicts(magic_bob) == [], "bob has conflicts" + From 00b9586c9aefe6b667b94959fbd0a8368044bc09 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 10:47:21 -0600 Subject: [PATCH 39/67] improve documentation --- docs/conflicts.rst | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/docs/conflicts.rst b/docs/conflicts.rst index defedc7c3..f94b53fe4 100644 --- a/docs/conflicts.rst +++ b/docs/conflicts.rst @@ -28,15 +28,16 @@ When this file is created (for example on the device called Laptop) that device By "upload" we mean to push the content and metadata to the Tahoe-LAFS client, receiving ultimately a Capability for the Snapshot ``S0``. The file entry for ``"foo"`` is then updated, visualized as: ``foo -> S0`` +Here, "updated" meanst that we use Tahoe-LAFS to edit to contents of our Personal directory to change "foo" to point at Snapshot ``S0``. Now, all other Participants can (and, eventually, will) notice this update when they poll the magic-folder. -Each of these is a "communication event", so we talk about the regions between these as a cohesive state of that client. +Each of these updates to our Personal directory is a "communication event", so we talk about the regions between these as a cohesive state of that client. That is, anything that happens during that time is "at the same time" for the purposes of this protocol. Note that this time interval could be as short as a few seconds or as long as days (or more). -Observe too that the "parents" of a particular Snapshot are a commitment as to the state visible to that client when creating the Snapshot. +Observe too that the "parents" of a particular Snapshot are a commitment to the state visible by a particular client when creating any new Snapshot. Detecting Conflicts @@ -61,7 +62,9 @@ Showing Conflicts Locally ------------------------- Once a conflict is detected, "conflict marker" files are put into the local magic folder location (our local file remains unmodified, and something like ``foo.conflict-desktop`` will appear. -The state database is also updated (conflicts can also be listed via the API and CLI) +The state database is also updated (conflicts can also be listed via the API and CLI). + +Although magic-folder itself doesn't try to examine the contents, you can now use any ``diff`` or similar tools you prefer to look at what is different between your copy and other participant(s) copies. Resolving Conflicts @@ -74,19 +77,25 @@ Once appropriate changes are decided upon, a new Snapshot is produced with *two Such a Snapshot (with two or more parents) indicates to the other clients a particular resoltion to the conflict has been decided. -So there's another case when we see an incoming new Snapshot: it may in fact *resolve an existing* conflict. +So there's actually another case when we see an incoming new Snapshot: it may in fact *resolve an existing* conflict. +If this is the case, conflict markers are removed and the local database is updated (i.e. removing the conflict). + +It is a human problem if this resolution is not to your particular liking; you can produce an edit again or talk to the human who runs the other computer(s) involved. +The history of these changes *is available* in the parent (or grandparent) Snapshots if the UX you're using can view or restore these. Resolving via Filesystem ------------------------ +Not currently possible. + Resolving via the CLI --------------------- The subcommand ``magic-folder resolve`` may be used to specify a resolution. It allows you to choose ``--mine`` or ``--theirs`` (if there is only one other conflict). -Otherwise, you must use ``--use `` to specify which version to keep. +Otherwise, you must apply the ``--use `` option to specify which version to keep. Currently there is no API for doing something more complex (e.g. simultaneuously replacing the latest version with new content). @@ -101,3 +110,22 @@ Resolving via the HTTP API -------------------------- See :ref:`api_resolve_conflict` + + +Future Directions +----------------- + +We do not consider the current conflict functionality "done". +There are other features required to make this more robust and have a nicer user experience. + +*Viewing old data*: While it is currently possible in the datamodel to view past versions of the files, we do not know of any UI that does this (and the CLI currently cannot). + +*Restore old version*: Similarly, it is possible to produce a new Snapshot that effectively restores an older version of the same file. +We do not know of any UI that can do this. + +*Completely new content*: As hinted above, it might be nice to be able to produce a resolution that is some combination of multiple versions (like one sometimes does with Git conflicts, for example). +While this isn't directly possible currently, you can always take the "closest" one via the existin conflict-resolution API and then immediately produce an edit that has the desired new content. + +*Resolution via file manipulation*: Currently, filesystem manipulation is one API (e.g. you just change a file and new Snapshots are produced). +Similarly, conflict-marker files are used to indicate a conflict via the filesystem. +It would be nice if you could use a similar mechanism to *eliminate* conflicts -- one way to design this could be to notice that the user has deleted all the conflict-markers and take this as a sign that the remaining file is in fact the desired resolution. From 17c2cb8da7db3ebb0df25a303eda158bfd1c3dc7 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 10:47:54 -0600 Subject: [PATCH 40/67] only show individual uploads if there's less than 20 of them --- src/magic_folder/cli.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/magic_folder/cli.py b/src/magic_folder/cli.py index d5cc9d7a7..b85d28b52 100644 --- a/src/magic_folder/cli.py +++ b/src/magic_folder/cli.py @@ -506,10 +506,11 @@ def message(payload, is_binary=False): file=out, ) print(" uploads: {}".format(len(folder["uploads"])), file=out) - for relpath, u in folder["uploads"].items(): - queue = humanize.naturaldelta(now - u["queued-at"]) - start = " (started {} ago)".format(humanize.naturaldelta(now - u["started-at"])) if "started-at" in u else "" - print(" {}: queued {} ago{}".format(relpath, queue, start), file=out) + if len(folder["uploads"]) < 20: + for relpath, u in folder["uploads"].items(): + queue = humanize.naturaldelta(now - u["queued-at"]) + start = " (started {} ago)".format(humanize.naturaldelta(now - u["started-at"])) if "started-at" in u else "" + print(" {}: queued {} ago{}".format(relpath, queue, start), file=out) if folder["errors"]: print("Errors:", file=out) From bd199b90ff0d3eb0a435eb0eec5e694a56fe9836 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 11:08:20 -0600 Subject: [PATCH 41/67] test invalid resolution --- src/magic_folder/test/test_magic_file.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/magic_folder/test/test_magic_file.py b/src/magic_folder/test/test_magic_file.py index c07aaab5b..b87c1a94c 100644 --- a/src/magic_folder/test/test_magic_file.py +++ b/src/magic_folder/test/test_magic_file.py @@ -61,6 +61,7 @@ from ..magic_file import ( maybe_update_personal_dmd_to_local, MagicFileFactory, + ResolutionError, ) from ..magic_folder import ( MagicFolder, @@ -331,3 +332,13 @@ def test_multiple_remote_updates(self): d1 = mf.found_new_remote(remote0, self.participants.participants[2]) d2 = mf.found_new_remote(remote0, self.participants.participants[3]) yield DeferredList([d0, d1, d2]) + + @inlineCallbacks + def test_not_conflicted(self): + """ + """ + relpath = "not-conflict" + abspath = self.config.magic_path.preauthChild(relpath) + mf = self.magic_file_factory.magic_file_for(abspath) + with self.assertRaises(ResolutionError): + yield mf.resolve_conflict("foo") From dbeef66cb4d7f93571edced465a2ee8b17054b60 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 11:56:27 -0600 Subject: [PATCH 42/67] in-memory resolve --- src/magic_folder/downloader.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/magic_folder/downloader.py b/src/magic_folder/downloader.py index 7de3ae68a..c4755cc48 100644 --- a/src/magic_folder/downloader.py +++ b/src/magic_folder/downloader.py @@ -480,6 +480,7 @@ class InMemoryMagicFolderFilesystem(object): def __init__(self): self.actions = [] self._staged_content = {} + self._conflicted_paths = set() def download_content_to_staging(self, relpath, file_cap, tahoe_client): self.actions.append( @@ -506,6 +507,14 @@ def mark_conflict(self, relpath, conflict_path, staged_content): self.actions.append( ("conflict", relpath, conflict_path, self._staged_content[staged_content]) ) + self._conflicted_paths.add(relpath) + + def mark_not_conflicted(self, relpath, keep_path, rejected_paths): + assert relpath in self._conflicted_paths, "Resolved something not conflicted" + self._conflicted_paths.remove(relpath) + self.actions.append( + ("resolve", relpath, keep_path, rejected_paths) + ) def mark_delete(self, relpath): self.actions.append( From d4deb913cec9df29561a80e47fe8bec08765bf52 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 11:56:50 -0600 Subject: [PATCH 43/67] test conflict cases --- src/magic_folder/test/test_magic_file.py | 55 +++++++++++++++++++++--- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/src/magic_folder/test/test_magic_file.py b/src/magic_folder/test/test_magic_file.py index b87c1a94c..bc3f3875c 100644 --- a/src/magic_folder/test/test_magic_file.py +++ b/src/magic_folder/test/test_magic_file.py @@ -29,6 +29,7 @@ from ..config import ( create_testing_configuration, + Conflict, ) from ..testing.web import ( create_tahoe_treq_client, @@ -235,7 +236,7 @@ def setUp(self): ) tahoe_client = object() - uploader = InMemoryUploaderService(["a-file-name", "a-file-name"]) + self.uploader = InMemoryUploaderService(["a-file-name", "a-file-name"]) status_service = EventsWebSocketStatusService(self.reactor, self._global_config) folder_status = FolderStatus("folder-name", status_service) self.stash_path = FilePath(self.mktemp()) @@ -251,7 +252,7 @@ def setUp(self): ), folder_status, ) - filesystem = InMemoryMagicFolderFilesystem() + self.filesystem = InMemoryMagicFolderFilesystem() self.tahoe_client = create_tahoe_treq_client() self.remote_cache = RemoteSnapshotCacheService.from_config(self.config, self.tahoe_client) @@ -261,10 +262,10 @@ def setUp(self): tahoe_client, folder_status, self.local_snapshot_service, - uploader, + self.uploader, self.participants.writer, self.remote_cache, - filesystem, + self.filesystem, ) self.magic_folder = MagicFolder( client=tahoe_client, @@ -275,7 +276,7 @@ def setUp(self): folder_status=folder_status, remote_snapshot_cache=self.remote_cache, downloader=MultiService(), - uploader=uploader, + uploader=self.uploader, participants=self.participants, scanner_service=Service(), clock=self.reactor, @@ -342,3 +343,47 @@ def test_not_conflicted(self): mf = self.magic_file_factory.magic_file_for(abspath) with self.assertRaises(ResolutionError): yield mf.resolve_conflict("foo") + + @inlineCallbacks + def test_remote_conflict(self): + """ + """ + relpath = "dual-conflict" + cap0 = random_immutable(directory=True) + cap1 = random_immutable(directory=True) + remote0 = RemoteSnapshot( + relpath=relpath, + author=create_author("someone", VerifyKey(b"\xff" * 32)), + metadata={"modification_time": 0}, + capability=cap0, + parents_raw=[], + content_cap=random_immutable(), + metadata_cap=random_immutable(), + ) + remote1 = RemoteSnapshot( + relpath=relpath, + author=create_author("someone_else", VerifyKey(b"\xff" * 32)), + metadata={"modification_time": 0}, + capability=cap1, + parents_raw=[], + content_cap=random_immutable(), + metadata_cap=random_immutable(), + ) + self.remote_cache._cached_snapshots[cap0.danger_real_capability_string()] = remote0 + self.remote_cache._cached_snapshots[cap1.danger_real_capability_string()] = remote1 + + abspath = self.config.magic_path.preauthChild(relpath) + mf = self.magic_file_factory.magic_file_for(abspath) + self.participants.add("beth", random_dircap()) + d0 = mf.found_new_remote(remote0, self.participants.participants[0]) + d1 = mf.found_new_remote(remote1, self.participants.participants[1]) + x = yield DeferredList([d0, d1]) + + self.assertEquals( + self.config.list_conflicts_for("dual-conflict"), + [Conflict(snapshot_cap=cap1, participant_name="beth")] + ) + + self.uploader._uploads = ["dual-conflict"] + snap = yield mf.resolve_conflict(Conflict(snapshot_cap=cap1, participant_name="beth")) + print("XXX", snap) From df9564de1e160b14cae5c5399242d21a0fe0a7a5 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 12:07:51 -0600 Subject: [PATCH 44/67] refactor and add error-case test --- src/magic_folder/test/test_web.py | 122 +++++++++++++++++------------- 1 file changed, 71 insertions(+), 51 deletions(-) diff --git a/src/magic_folder/test/test_web.py b/src/magic_folder/test/test_web.py index 1e963e839..9c1030a66 100644 --- a/src/magic_folder/test/test_web.py +++ b/src/magic_folder/test/test_web.py @@ -2022,24 +2022,36 @@ class ConflictStatusTests(SyncTestCase): """ url = DecodedURL.from_text(u"http://example.invalid./v1/magic-folder") + def setUp(self): + super(ConflictStatusTests, self).setUp() + self.local_path = FilePath(self.mktemp()) + self.local_path.makedirs() + self.folder_config = magic_folder_config( + "louise", + self.local_path, + ) + self.node = MagicFolderNode.create( + Clock(), + FilePath(self.mktemp()), + AUTH_TOKEN, + { + "default": self.folder_config, + }, + start_folder_services=False, + ) + self.node.global_service.get_folder_service("default").file_factory._synchronous = True + + def test_empty(self): """ A folder with no conflicts reflects that in the status """ - local_path = FilePath(self.mktemp()) - local_path.makedirs() - - folder_config = magic_folder_config( - "louise", - local_path, - ) - treq = treq_for_folders( Clock(), FilePath(self.mktemp()), AUTH_TOKEN, { - "default": folder_config, + "default": self.folder_config, }, start_folder_services=False, ) @@ -2068,26 +2080,7 @@ def test_one_conflict(self): Appropriate information is returned when we have a conflict with one author """ - local_path = FilePath(self.mktemp()) - local_path.makedirs() - - folder_config = magic_folder_config( - "marta", - local_path, - ) - - node = MagicFolderNode.create( - Clock(), - FilePath(self.mktemp()), - AUTH_TOKEN, - { - "default": folder_config, - }, - start_folder_services=False, - ) - node.global_service.get_folder_service("default").file_factory._synchronous = True - - mf_config = node.global_config.get_magic_folder("default") + mf_config = self.node.global_config.get_magic_folder("default") mf_config._get_current_timestamp = lambda: 42.0 mf_config.store_currentsnapshot_state( "foo", @@ -2121,7 +2114,7 @@ def test_one_conflict(self): # external API self.assertThat( authorized_request( - node.http_client, + self.node.http_client, AUTH_TOKEN, u"GET", self.url.child("default", "conflicts"), @@ -2139,31 +2132,60 @@ def test_one_conflict(self): ) ) - def test_resolve_conflict(self): """ We can resolve a conflict """ - local_path = FilePath(self.mktemp()) - local_path.makedirs() + mf_config = self.node.global_config.get_magic_folder("default") + mf_config._get_current_timestamp = lambda: 42.0 + mf_config.store_currentsnapshot_state( + "foo", + PathState(123, seconds_to_ns(1), seconds_to_ns(2)), + ) - folder_config = magic_folder_config( - "marta", - local_path, + snap = RemoteSnapshot( + "foo", + create_local_author("nelli"), + {"relpath": "foo", "modification_time": 1234}, + random_immutable(directory=True), + [], + random_immutable(), + random_immutable(), ) - node = MagicFolderNode.create( - Clock(), - FilePath(self.mktemp()), - AUTH_TOKEN, - { - "default": folder_config, - }, - start_folder_services=False, + mf_config.add_conflict(snap, static_participants(names=["cavatica"]).list()[0]) + + # external API + self.assertThat( + authorized_request( + self.node.http_client, + AUTH_TOKEN, + u"POST", + self.url.child("default", "resolve-conflict"), + dumps({ + "relpath": "foo", + "take": "mine", + # "use": ..., for multi-conflicts + }).encode("utf8") + ), + succeeded( + matches_response( + code_matcher=Equals(200), + body_matcher=AfterPreprocessing( + loads, + Equals({ + "foo": ["cavatica"], + }), + ) + ), + ) ) - node.global_service.get_folder_service("default").file_factory._synchronous = True - mf_config = node.global_config.get_magic_folder("default") + def test_resolve_conflict_non_exist(self): + """ + It is an error to resolve a non-conflict + """ + mf_config = self.node.global_config.get_magic_folder("default") mf_config._get_current_timestamp = lambda: 42.0 mf_config.store_currentsnapshot_state( "foo", @@ -2180,12 +2202,10 @@ def test_resolve_conflict(self): random_immutable(), ) - mf_config.add_conflict(snap, static_participants(names=["cavatica"]).list()[0]) - # external API self.assertThat( authorized_request( - node.http_client, + self.node.http_client, AUTH_TOKEN, u"POST", self.url.child("default", "resolve-conflict"), @@ -2197,11 +2217,11 @@ def test_resolve_conflict(self): ), succeeded( matches_response( - code_matcher=Equals(200), + code_matcher=Equals(400), body_matcher=AfterPreprocessing( loads, Equals({ - "foo": ["cavatica"], + "reason": 'No conflicts for "foo"', }), ) ), From efdf878452438f3bda5cb63ab07db3d8f3accd04 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 12:15:29 -0600 Subject: [PATCH 45/67] test for take/use at once --- src/magic_folder/test/test_web.py | 48 +++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/magic_folder/test/test_web.py b/src/magic_folder/test/test_web.py index 9c1030a66..88cc37c30 100644 --- a/src/magic_folder/test/test_web.py +++ b/src/magic_folder/test/test_web.py @@ -2228,6 +2228,54 @@ def test_resolve_conflict_non_exist(self): ) ) + def test_resolve_conflict_use_and_take(self): + """ + It is an error to specify both "use" and "take" + """ + mf_config = self.node.global_config.get_magic_folder("default") + mf_config._get_current_timestamp = lambda: 42.0 + mf_config.store_currentsnapshot_state( + "foo", + PathState(123, seconds_to_ns(1), seconds_to_ns(2)), + ) + + snap = RemoteSnapshot( + "foo", + create_local_author("nelli"), + {"relpath": "foo", "modification_time": 1234}, + random_immutable(directory=True), + [], + random_immutable(), + random_immutable(), + ) + mf_config.add_conflict(snap, static_participants(names=["marie"]).list()[0]) + + # external API + self.assertThat( + authorized_request( + self.node.http_client, + AUTH_TOKEN, + u"POST", + self.url.child("default", "resolve-conflict"), + dumps({ + "relpath": "foo", + "take": "mine", + "use": "margaret", + }).encode("utf8") + ), + succeeded( + matches_response( + code_matcher=Equals(400), + body_matcher=AfterPreprocessing( + loads, + Equals({ + "reason": 'Cannot specify "take" and "use" at once', + }), + ) + ), + ) + ) + class InviteTests(SyncTestCase): """ From 18358e74102d850808c440687224e34841f430e5 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 12:18:54 -0600 Subject: [PATCH 46/67] more error-case Web API tests --- src/magic_folder/test/test_web.py | 95 +++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/src/magic_folder/test/test_web.py b/src/magic_folder/test/test_web.py index 88cc37c30..e667a751a 100644 --- a/src/magic_folder/test/test_web.py +++ b/src/magic_folder/test/test_web.py @@ -2276,6 +2276,101 @@ def test_resolve_conflict_use_and_take(self): ) ) + def test_resolve_conflict_take_invalid(self): + """ + It is an error for take to be a weird value + """ + mf_config = self.node.global_config.get_magic_folder("default") + mf_config._get_current_timestamp = lambda: 42.0 + mf_config.store_currentsnapshot_state( + "foo", + PathState(123, seconds_to_ns(1), seconds_to_ns(2)), + ) + + snap = RemoteSnapshot( + "foo", + create_local_author("marie"), + {"relpath": "foo", "modification_time": 1234}, + random_immutable(directory=True), + [], + random_immutable(), + random_immutable(), + ) + mf_config.add_conflict(snap, static_participants(names=["marie"]).list()[0]) + + # external API + self.assertThat( + authorized_request( + self.node.http_client, + AUTH_TOKEN, + u"POST", + self.url.child("default", "resolve-conflict"), + dumps({ + "relpath": "foo", + "take": "a definitely invalid string", + }).encode("utf8") + ), + succeeded( + matches_response( + code_matcher=Equals(400), + body_matcher=AfterPreprocessing( + loads, + Equals({ + "reason": '"take" must be "mine" or "theirs"', + }), + ) + ), + ) + ) + + def test_resolve_conflict_theirs_invalid(self): + """ + It is an error to use "theirs" with >1 conflict + """ + mf_config = self.node.global_config.get_magic_folder("default") + mf_config._get_current_timestamp = lambda: 42.0 + mf_config.store_currentsnapshot_state( + "foo", + PathState(123, seconds_to_ns(1), seconds_to_ns(2)), + ) + + snap = RemoteSnapshot( + "foo", + create_local_author("ada"), + {"relpath": "foo", "modification_time": 1234}, + random_immutable(directory=True), + [], + random_immutable(), + random_immutable(), + ) + mf_config.add_conflict(snap, static_participants(names=["ada"]).list()[0]) + mf_config.add_conflict(snap, static_participants(names=["margaret"]).list()[0]) + + # external API + self.assertThat( + authorized_request( + self.node.http_client, + AUTH_TOKEN, + u"POST", + self.url.child("default", "resolve-conflict"), + dumps({ + "relpath": "foo", + "take": "theirs", + }).encode("utf8") + ), + succeeded( + matches_response( + code_matcher=Equals(400), + body_matcher=AfterPreprocessing( + loads, + Equals({ + "reason": 'Cannot use "theirs" with 2 conflicts', + }), + ) + ), + ) + ) + class InviteTests(SyncTestCase): """ From b42ca105b77355d24ae7c6be093c1adcd379c5a8 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 12:21:20 -0600 Subject: [PATCH 47/67] cleanup --- src/magic_folder/web.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/magic_folder/web.py b/src/magic_folder/web.py index 624df43ff..aea8c09d9 100644 --- a/src/magic_folder/web.py +++ b/src/magic_folder/web.py @@ -596,8 +596,6 @@ def resolve_conflict(request, folder_name): raise _InputError('Must specify "take" or "use"') folder_svc = global_service.get_folder_service(folder_name) - print(folder_svc) - print(dir(folder_svc)) mf = folder_svc.file_factory.magic_file_for( folder_svc.file_factory.relpath_to_path(relpath) ) From deb02947bec0075b51ccac3f401390b840e0fac9 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 14:59:12 -0600 Subject: [PATCH 48/67] test a remote-resolve --- src/magic_folder/test/test_magic_file.py | 73 ++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/src/magic_folder/test/test_magic_file.py b/src/magic_folder/test/test_magic_file.py index bc3f3875c..11cbce6dc 100644 --- a/src/magic_folder/test/test_magic_file.py +++ b/src/magic_folder/test/test_magic_file.py @@ -347,6 +347,7 @@ def test_not_conflicted(self): @inlineCallbacks def test_remote_conflict(self): """ + We have a conflict """ relpath = "dual-conflict" cap0 = random_immutable(directory=True) @@ -387,3 +388,75 @@ def test_remote_conflict(self): self.uploader._uploads = ["dual-conflict"] snap = yield mf.resolve_conflict(Conflict(snapshot_cap=cap1, participant_name="beth")) print("XXX", snap) + + @inlineCallbacks + def test_remote_conflict_resolution(self): + """ + an incoming update is actually a resolution to an existing conflict + """ + relpath = "fire" + with self.magic_path.child(relpath).open("wb") as f: + f.write(b"testdata") + + from magic_folder.util.file import get_pathinfo + cap0 = random_immutable(directory=True) + cap1 = random_immutable(directory=True) + cap2 = random_immutable(directory=True) + cap3 = random_immutable(directory=True) + remote0 = RemoteSnapshot( + relpath=relpath, + author=create_author("me", VerifyKey(b"\xff" * 32)), + metadata={"modification_time": 0}, + capability=cap0, + parents_raw=[], + content_cap=random_immutable(), + metadata_cap=random_immutable(), + ) + remote1 = RemoteSnapshot( + relpath=relpath, + author=create_author("zara", VerifyKey(b"\xff" * 32)), + metadata={"modification_time": 0}, + capability=cap1, + parents_raw=[], + content_cap=random_immutable(), + metadata_cap=random_immutable(), + ) + remote2 = RemoteSnapshot( + relpath=relpath, + author=create_author("yidris", VerifyKey(b"\xff" * 32)), + metadata={"modification_time": 0}, + capability=cap2, + parents_raw=[remote0.capability.danger_real_capability_string(), remote1.capability.danger_real_capability_string()], + content_cap=random_immutable(), + metadata_cap=random_immutable(), + ) + assert cap0 == remote0.capability + assert cap1 == remote1.capability + assert cap2 == remote2.capability + self.remote_cache._cached_snapshots[cap0.danger_real_capability_string()] = remote0 + self.remote_cache._cached_snapshots[cap1.danger_real_capability_string()] = remote1 + self.remote_cache._cached_snapshots[cap2.danger_real_capability_string()] = remote2 + + self.participants.add("zara", random_dircap()) + self.participants.add("yidris", random_dircap()) + + self.config.store_currentsnapshot_state(relpath, get_pathinfo(self.magic_path.child(relpath)).state) + self.config.store_uploaded_snapshot(relpath, remote0, 0.0) + self.filesystem._conflicted_paths.add(relpath) + self.config.add_conflict(remote1, self.participants.participants[1]) + + abspath = self.config.magic_path.preauthChild(relpath) + mf = self.magic_file_factory.magic_file_for(abspath) + + self.assertEquals( + self.config.list_conflicts_for(relpath), + [Conflict(snapshot_cap=cap1, participant_name="zara")] + ) + + # resolve the conflict via a remote + yield mf.found_new_remote(remote2, self.participants.participants[2]) + + self.assertEquals( + self.config.list_conflicts_for(relpath), + [] + ) From 62979f477b9f7598418756403f9b87200c88f032 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 15:39:43 -0600 Subject: [PATCH 49/67] linter --- integration/test_resolve_conflict.py | 3 +-- src/magic_folder/test/test_magic_file.py | 3 +-- src/magic_folder/test/test_web.py | 10 ---------- 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/integration/test_resolve_conflict.py b/integration/test_resolve_conflict.py index fe89c98bc..22e3a5dfb 100644 --- a/integration/test_resolve_conflict.py +++ b/integration/test_resolve_conflict.py @@ -92,7 +92,7 @@ async def test_conflicted_users(request, reactor, temp_filepath, alice, bob): alice.resolve("conflict", magic.child("summertime.txt").path, "theirs") # wait for updates - x = await DeferredList([ + await DeferredList([ alice.status_monitor(how_long=20), bob.status_monitor(how_long=20), ]) @@ -100,4 +100,3 @@ async def test_conflicted_users(request, reactor, temp_filepath, alice, bob): # no more conflicts assert find_conflicts(magic) == [], "alice has conflicts" assert find_conflicts(magic_bob) == [], "bob has conflicts" - diff --git a/src/magic_folder/test/test_magic_file.py b/src/magic_folder/test/test_magic_file.py index 11cbce6dc..3e7fa176b 100644 --- a/src/magic_folder/test/test_magic_file.py +++ b/src/magic_folder/test/test_magic_file.py @@ -378,7 +378,7 @@ def test_remote_conflict(self): self.participants.add("beth", random_dircap()) d0 = mf.found_new_remote(remote0, self.participants.participants[0]) d1 = mf.found_new_remote(remote1, self.participants.participants[1]) - x = yield DeferredList([d0, d1]) + yield DeferredList([d0, d1]) self.assertEquals( self.config.list_conflicts_for("dual-conflict"), @@ -402,7 +402,6 @@ def test_remote_conflict_resolution(self): cap0 = random_immutable(directory=True) cap1 = random_immutable(directory=True) cap2 = random_immutable(directory=True) - cap3 = random_immutable(directory=True) remote0 = RemoteSnapshot( relpath=relpath, author=create_author("me", VerifyKey(b"\xff" * 32)), diff --git a/src/magic_folder/test/test_web.py b/src/magic_folder/test/test_web.py index e667a751a..8406c6990 100644 --- a/src/magic_folder/test/test_web.py +++ b/src/magic_folder/test/test_web.py @@ -2192,16 +2192,6 @@ def test_resolve_conflict_non_exist(self): PathState(123, seconds_to_ns(1), seconds_to_ns(2)), ) - snap = RemoteSnapshot( - "foo", - create_local_author("nelli"), - {"relpath": "foo", "modification_time": 1234}, - random_immutable(directory=True), - [], - random_immutable(), - random_immutable(), - ) - # external API self.assertThat( authorized_request( From a4dedc20479a7f3f68a17f12d471746583675596 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 May 2024 17:49:47 -0600 Subject: [PATCH 50/67] windows cares about tests-names, kind of --- integration/test_resolve_conflict.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/test_resolve_conflict.py b/integration/test_resolve_conflict.py index 22e3a5dfb..9bbc46db4 100644 --- a/integration/test_resolve_conflict.py +++ b/integration/test_resolve_conflict.py @@ -50,7 +50,7 @@ def cleanup_invitee(): @inline_callbacks @pytest_twisted.ensureDeferred -async def test_conflicted_users(request, reactor, temp_filepath, alice, bob): +async def test_resolve_two_users(request, reactor, temp_filepath, alice, bob): """ Two users both add the same file at the same time, producing conflicts. From 5e7c4c99e51dde0277a4835f43bb8c1459d0b8f0 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 3 Jun 2024 13:36:09 -0600 Subject: [PATCH 51/67] properly overwrite, even on windows --- src/magic_folder/downloader.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/magic_folder/downloader.py b/src/magic_folder/downloader.py index c4755cc48..5ba800dd3 100644 --- a/src/magic_folder/downloader.py +++ b/src/magic_folder/downloader.py @@ -448,7 +448,13 @@ def mark_not_conflicted(self, relpath, keep_path, rejected_paths): for p in rejected_paths ] if dest_path != src_path: - src_path.moveTo(dest_path) + try: + src_path.moveTo(dest_path) + except FileExistsError: + # at least on Windows, it's an error to write to a + # file that's already there .. + src_path.remove() + src_path.moveTo(dest_path) for p in del_paths: try: p.remove() From 861042410eeca1de18494bf3e3223b5e7b0317de Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 3 Jun 2024 14:55:42 -0600 Subject: [PATCH 52/67] fixup; delete the dest not source --- src/magic_folder/downloader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/magic_folder/downloader.py b/src/magic_folder/downloader.py index 5ba800dd3..7df3dd8ec 100644 --- a/src/magic_folder/downloader.py +++ b/src/magic_folder/downloader.py @@ -453,7 +453,7 @@ def mark_not_conflicted(self, relpath, keep_path, rejected_paths): except FileExistsError: # at least on Windows, it's an error to write to a # file that's already there .. - src_path.remove() + dest_path.remove() src_path.moveTo(dest_path) for p in del_paths: try: From 0342420c64418d4056723e96df134be5e75b64ab Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 3 Jun 2024 15:57:15 -0600 Subject: [PATCH 53/67] reword --- integration/test_resolve_conflict.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/integration/test_resolve_conflict.py b/integration/test_resolve_conflict.py index 9bbc46db4..dcec0e95e 100644 --- a/integration/test_resolve_conflict.py +++ b/integration/test_resolve_conflict.py @@ -75,10 +75,9 @@ async def test_resolve_two_users(request, reactor, temp_filepath, alice, bob): alice.add_snapshot("conflict", "summertime.txt"), bob.add_snapshot("conflict", "summertime.txt"), ]) - # we've added all the files on all participants .. they _should_ conflict, but also we - # shouldn't _keep_ trying to download conflicted updates. + # we've added all the files on both participants - # now, wait for updates + # wait for updates await DeferredList([ alice.status_monitor(how_long=20), bob.status_monitor(how_long=20), From 3fc14ea2c6ac764485b30a5cab0e1de62d922371 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 3 Jun 2024 15:57:20 -0600 Subject: [PATCH 54/67] debug --- integration/test_resolve_conflict.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integration/test_resolve_conflict.py b/integration/test_resolve_conflict.py index dcec0e95e..6c9c5aa6e 100644 --- a/integration/test_resolve_conflict.py +++ b/integration/test_resolve_conflict.py @@ -91,10 +91,11 @@ async def test_resolve_two_users(request, reactor, temp_filepath, alice, bob): alice.resolve("conflict", magic.child("summertime.txt").path, "theirs") # wait for updates - await DeferredList([ + x = await DeferredList([ alice.status_monitor(how_long=20), bob.status_monitor(how_long=20), ]) + print("X", x) # no more conflicts assert find_conflicts(magic) == [], "alice has conflicts" From ba0151031fa5d085ded23b66f00238eec103e308 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 3 Jun 2024 16:17:20 -0600 Subject: [PATCH 55/67] missing await --- integration/test_resolve_conflict.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/test_resolve_conflict.py b/integration/test_resolve_conflict.py index 6c9c5aa6e..d5c87410b 100644 --- a/integration/test_resolve_conflict.py +++ b/integration/test_resolve_conflict.py @@ -88,7 +88,7 @@ async def test_resolve_two_users(request, reactor, temp_filepath, alice, bob): assert find_conflicts(magic_bob) != [], "bob should have conflicts" # resolve the conflict - alice.resolve("conflict", magic.child("summertime.txt").path, "theirs") + await alice.resolve("conflict", magic.child("summertime.txt").path, "theirs") # wait for updates x = await DeferredList([ From d4d1b9ad40209cc601eef7214f59b440be588c17 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 3 Jun 2024 16:17:28 -0600 Subject: [PATCH 56/67] undo debug --- integration/test_resolve_conflict.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/integration/test_resolve_conflict.py b/integration/test_resolve_conflict.py index d5c87410b..135434d0e 100644 --- a/integration/test_resolve_conflict.py +++ b/integration/test_resolve_conflict.py @@ -91,11 +91,10 @@ async def test_resolve_two_users(request, reactor, temp_filepath, alice, bob): await alice.resolve("conflict", magic.child("summertime.txt").path, "theirs") # wait for updates - x = await DeferredList([ + await DeferredList([ alice.status_monitor(how_long=20), bob.status_monitor(how_long=20), ]) - print("X", x) # no more conflicts assert find_conflicts(magic) == [], "alice has conflicts" From 3adf6c891082285e99aaceb482bd1c091288a5ac Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 11 Jun 2024 14:37:18 -0700 Subject: [PATCH 57/67] spell --- docs/conflicts.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conflicts.rst b/docs/conflicts.rst index f94b53fe4..db93f5b7e 100644 --- a/docs/conflicts.rst +++ b/docs/conflicts.rst @@ -28,7 +28,7 @@ When this file is created (for example on the device called Laptop) that device By "upload" we mean to push the content and metadata to the Tahoe-LAFS client, receiving ultimately a Capability for the Snapshot ``S0``. The file entry for ``"foo"`` is then updated, visualized as: ``foo -> S0`` -Here, "updated" meanst that we use Tahoe-LAFS to edit to contents of our Personal directory to change "foo" to point at Snapshot ``S0``. +Here, "updated" means that we use Tahoe-LAFS to edit to contents of our Personal directory to change "foo" to point at Snapshot ``S0``. Now, all other Participants can (and, eventually, will) notice this update when they poll the magic-folder. From e6f48d589bb11fcb640d3cadda0cccbd3baaeb58 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 11 Jun 2024 14:37:38 -0700 Subject: [PATCH 58/67] spell --- docs/conflicts.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conflicts.rst b/docs/conflicts.rst index db93f5b7e..2c87bb3ed 100644 --- a/docs/conflicts.rst +++ b/docs/conflicts.rst @@ -43,7 +43,7 @@ Observe too that the "parents" of a particular Snapshot are a commitment to the Detecting Conflicts ------------------- -A new update is communicated to us -- that is, we've downloaded a previously-unknown Snapshot from some other Participant's mutable Capbility. +A new update is communicated to us -- that is, we've downloaded a previously-unknown Snapshot from some other Participant's mutable Capability. We now look at our own Snapshot for the corresponding file. Either our Snapshot appears in some parents (including grandparents, etc) of the new Snapshot, or it doesn't. From 35e1996ada3601d315246ab9ee8470b6388a587f Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 11 Jun 2024 14:40:19 -0700 Subject: [PATCH 59/67] add a ref --- docs/conflicts.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/conflicts.rst b/docs/conflicts.rst index 2c87bb3ed..e0cc9cc4e 100644 --- a/docs/conflicts.rst +++ b/docs/conflicts.rst @@ -98,6 +98,7 @@ It allows you to choose ``--mine`` or ``--theirs`` (if there is only one other c Otherwise, you must apply the ``--use `` option to specify which version to keep. Currently there is no API for doing something more complex (e.g. simultaneuously replacing the latest version with new content). +As with everything else on the CLI, the :ref:`http_api` may be used to accomplish the same simple resolution tasks as above. Complete example: From b0b65057cf60139d9ef9eca3a22dfad15d43f484 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 11 Jun 2024 14:41:05 -0700 Subject: [PATCH 60/67] set up the list --- docs/conflicts.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/conflicts.rst b/docs/conflicts.rst index e0cc9cc4e..eae007b9f 100644 --- a/docs/conflicts.rst +++ b/docs/conflicts.rst @@ -118,6 +118,7 @@ Future Directions We do not consider the current conflict functionality "done". There are other features required to make this more robust and have a nicer user experience. +Some of those features are: *Viewing old data*: While it is currently possible in the datamodel to view past versions of the files, we do not know of any UI that does this (and the CLI currently cannot). From 86cb4789b260355cda37d2094f39d7c388cd8176 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 11 Jun 2024 14:47:11 -0700 Subject: [PATCH 61/67] add some notes on error-handling --- docs/interface.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/interface.rst b/docs/interface.rst index 3e717b113..49d7a076b 100644 --- a/docs/interface.rst +++ b/docs/interface.rst @@ -49,6 +49,17 @@ A mechanism to add deprecation of APIs will be added in a future release. - **Version 1 (``/v1``)**: initial version of the API (not yet considered 100% stable). +Error Handling +~~~~~~~~~~~~~~ + +Various sorts of errors may occur while using the API. + +Most input-validation errors (e.g. nonsensical argument, missing arugments, parsing errors, etc) will be answered with a "400 Bad Request". +Sometimes, 500-level errors may occur if something actually "internally bad" has happened. +A "502 Bad Gateway" will result from incorrect interactions with Magic Wormhole servers. +A "409 Conflict" results when a conflict-resolution attempt fails. + + .. _`daemon configuration`: :ref:`config` ``GET /v1/magic-folder`` From be5b04fe8743f7918e9db5f1b002ccf350fb6a86 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 11 Jun 2024 16:01:37 -0700 Subject: [PATCH 62/67] redirect --- docs/conflicts.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conflicts.rst b/docs/conflicts.rst index eae007b9f..b561eedf8 100644 --- a/docs/conflicts.rst +++ b/docs/conflicts.rst @@ -87,7 +87,7 @@ The history of these changes *is available* in the parent (or grandparent) Snaps Resolving via Filesystem ------------------------ -Not currently possible. +Not currently possible (see `https://github.com/tahoe-lafs/magic-folder/issues/754 `_). Resolving via the CLI From c3db63262cab3d1c5c959f4abe263feb44250674 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 12 Jun 2024 16:46:16 -0600 Subject: [PATCH 63/67] try coveralls --- .github/workflows/linux.yml | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 3808cabff..8ae987f13 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -86,12 +86,11 @@ jobs: name: "unit-test" path: "eliot*" - - name: Upload coverage report - uses: codecov/codecov-action@v2 + - name: Coveralls + uses: coverallsapp/github-action@v2 with: - token: "322d708d-8283-4827-b605-ccf02bfecf70" - file: "./coverage.xml" - + - parallel: true + - flag-name: unit integration-tests: @@ -162,3 +161,10 @@ jobs: token: "322d708d-8283-4827-b605-ccf02bfecf70" file: "./coverage.xml" flags: "integration" + + - name: Coveralls + uses: coverallsapp/github-action@v2 + with: + - parallel: true + - flag-name: integration + - parallel-finished: true From a35af310cc4036b3aaaf360096b8e445d48c4c6f Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 12 Jun 2024 16:48:18 -0600 Subject: [PATCH 64/67] syntax? --- .github/workflows/linux.yml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 8ae987f13..9c4d18b0b 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -90,7 +90,7 @@ jobs: uses: coverallsapp/github-action@v2 with: - parallel: true - - flag-name: unit + - flag-name: "unit" integration-tests: @@ -156,15 +156,9 @@ jobs: name: "integration" path: "eliot*" - - uses: codecov/codecov-action@v2 - with: - token: "322d708d-8283-4827-b605-ccf02bfecf70" - file: "./coverage.xml" - flags: "integration" - - name: Coveralls uses: coverallsapp/github-action@v2 with: - parallel: true - - flag-name: integration + - flag-name: "integration" - parallel-finished: true From aa6934576468c084e0697f1c920f1354b709ccb3 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 12 Jun 2024 16:50:01 -0600 Subject: [PATCH 65/67] syntax --- .github/workflows/linux.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 9c4d18b0b..954b6f93f 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -89,8 +89,8 @@ jobs: - name: Coveralls uses: coverallsapp/github-action@v2 with: - - parallel: true - - flag-name: "unit" + parallel: true + flag-name: "unit" integration-tests: @@ -159,6 +159,6 @@ jobs: - name: Coveralls uses: coverallsapp/github-action@v2 with: - - parallel: true - - flag-name: "integration" - - parallel-finished: true + parallel: true + flag-name: "integration" + parallel-finished: true From 5bba085a9bd8f60c10b4ec25cbece9a669cfb738 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 17 Jun 2024 16:21:35 -0600 Subject: [PATCH 66/67] circular-import attrib --- src/magic_folder/magic_file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/magic_folder/magic_file.py b/src/magic_folder/magic_file.py index 96b3afbc2..ebf707ec1 100644 --- a/src/magic_folder/magic_file.py +++ b/src/magic_folder/magic_file.py @@ -73,7 +73,7 @@ class MagicFileFactory(object): _config = attr.ib() # MagicFolderConfig _tahoe_client = attr.ib() _folder_status = attr.ib() - _local_snapshot_service = attr.ib()#validator=attr.validators.instance_of(LocalSnapshotService)) + _local_snapshot_service = attr.ib() # LocalSnapshotService but circular import _uploader = attr.ib() _write_participant = attr.ib() _remote_cache = attr.ib() From e68849b1893dc91b614dd33c36cfe7eabb6cdbfc Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 17 Jun 2024 16:21:53 -0600 Subject: [PATCH 67/67] dead code --- src/magic_folder/magic_file.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/magic_folder/magic_file.py b/src/magic_folder/magic_file.py index ebf707ec1..82bc087fa 100644 --- a/src/magic_folder/magic_file.py +++ b/src/magic_folder/magic_file.py @@ -214,9 +214,6 @@ class MagicFile(object): _queue_remote = attr.ib(default=attr.Factory(list)) _is_working = attr.ib(default=None) - # XXX - _known_remotes = attr.ib(default=None) - # to facilitate testing, sometimes we _don't_ want to use the real # reactor to wait one turn, or to delay for retry purposes. _call_later = attr.ib(default=None) # Callable to schedule work at least one reactor turn later @@ -330,14 +327,9 @@ def found_new_remote(self, remote_snapshot, participant): :param IParticipant participant: the participant we found this snapshot via """ - if self._known_remotes is None: - self._known_remotes = {remote_snapshot} - else: - self._known_remotes.add(remote_snapshot) - # if we're already conflicted, this will indeed not "match our # existing database entry" (as per the docstring) -- but it - # may still be an "already known" update becaus we've already + # may still be an "already known" update because we've already # seen it and marked it as a conflict found = False for conflict in self._factory._config.list_conflicts_for(self._relpath):