diff --git a/hgitaly/branch.py b/hgitaly/branch.py index 901e292c6365edfe3e54b8d2e2f10b39f2038dcf_aGdpdGFseS9icmFuY2gucHk=..85ed3117f2e6531518b66d140bb6ab86f0319cc9_aGdpdGFseS9icmFuY2gucHk= 100644 --- a/hgitaly/branch.py +++ b/hgitaly/branch.py @@ -78,6 +78,9 @@ :return: a :class:`changectx` or ``None`` if there's no changeset for the given ``gl_branch``. """ + if gl_branch is None: + return None + gl_branches = gitlab_branches(repo) if gl_branches is not GITLAB_BRANCHES_MISSING: sha = gl_branches.get(gl_branch) diff --git a/hgitaly/gitlab_ref.py b/hgitaly/gitlab_ref.py index 901e292c6365edfe3e54b8d2e2f10b39f2038dcf_aGdpdGFseS9naXRsYWJfcmVmLnB5..85ed3117f2e6531518b66d140bb6ab86f0319cc9_aGdpdGFseS9naXRsYWJfcmVmLnB5 100644 --- a/hgitaly/gitlab_ref.py +++ b/hgitaly/gitlab_ref.py @@ -28,6 +28,8 @@ :returns: a :class:`changectx` instance, for the unfiltered version of ``repo``. The changeset itself may be obsolete. """ + if ref_path is None: + return None name = parse_special_ref(ref_path) if name is None: return None diff --git a/hgitaly/service/repository_service.py b/hgitaly/service/repository_service.py index 901e292c6365edfe3e54b8d2e2f10b39f2038dcf_aGdpdGFseS9zZXJ2aWNlL3JlcG9zaXRvcnlfc2VydmljZS5weQ==..85ed3117f2e6531518b66d140bb6ab86f0319cc9_aGdpdGFseS9zZXJ2aWNlL3JlcG9zaXRvcnlfc2VydmljZS5weQ== 100644 --- a/hgitaly/service/repository_service.py +++ b/hgitaly/service/repository_service.py @@ -8,6 +8,7 @@ import logging import tempfile +from hgitaly.git import NULL_COMMIT_ID from mercurial import ( archival, scmutil, @@ -23,7 +24,7 @@ from ..errors import ( invalid_argument, - not_implemented, + not_implemented, internal_error, ) from ..branch import ( iter_gitlab_branches, @@ -164,6 +165,27 @@ repo = self.load_repo(request.repository, context) try: + expected_old_rev = request.old_revision + old_rev = gitlab_revision_changeset(repo, ref) + if old_rev is not None: + old_rev = old_rev.hex() + + NULL_COMMIT_ID_BYTES = NULL_COMMIT_ID.encode('ascii') + is_null_rev = expected_old_rev == NULL_COMMIT_ID_BYTES + # FIXME probably not correct + expected_no_old_rev = (is_null_rev or len(expected_old_rev) == 0) + + if expected_no_old_rev: + if old_rev is not None and old_rev != NULL_COMMIT_ID_BYTES: + msg = "Expected old rev to not exist, got %r" % old_rev + return invalid_argument(context, WriteRefResponse, msg) + elif expected_old_rev != old_rev: + return invalid_argument( + context, + WriteRefResponse, + "Expected old rev %r, got %r" % (expected_old_rev, old_rev) + ) + special_ref_name = parse_special_ref(ref) if special_ref_name is not None: target_sha = gitlab_revision_changeset(repo, target) @@ -185,6 +207,10 @@ create_keep_around(repo, target) return WriteRefResponse() except Exception: + # TODO this is a stop-gap measure to prevent repository breakage + # until we get confident enough with `ensure_special_refs` for all + # existing repositories. This goes against all of our principles, + # but it's more prudent this way. logger.exception( "WriteRef failed for Repository %r on storage %r", request.repository.relative_path, diff --git a/hgitaly/service/tests/test_repository_service.py b/hgitaly/service/tests/test_repository_service.py index 901e292c6365edfe3e54b8d2e2f10b39f2038dcf_aGdpdGFseS9zZXJ2aWNlL3Rlc3RzL3Rlc3RfcmVwb3NpdG9yeV9zZXJ2aWNlLnB5..85ed3117f2e6531518b66d140bb6ab86f0319cc9_aGdpdGFseS9zZXJ2aWNlL3Rlc3RzL3Rlc3RfcmVwb3NpdG9yeV9zZXJ2aWNlLnB5 100644 --- a/hgitaly/service/tests/test_repository_service.py +++ b/hgitaly/service/tests/test_repository_service.py @@ -28,6 +28,8 @@ ) from hgitaly.stub.repository_service_pb2_grpc import RepositoryServiceStub +from hgitaly.git import NULL_COMMIT_ID + def test_repository_exists(grpc_channel, server_repos_root): repo_stub = RepositoryServiceStub(grpc_channel) @@ -122,9 +124,11 @@ (wrapper.path / '.hg' / 'store' / 'gitlab.special-refs').write_text( 'invalid') + # TODO handle exception once we've started raising them. See big + # `try ... except` block in `repository_service`. repo_stub.WriteRef(WriteRefRequest( repository=grpc_repo, ref=b'refs/merge-requests/12/head', revision=b'76ac23fe' * 5)) @@ -125,9 +129,57 @@ repo_stub.WriteRef(WriteRefRequest( repository=grpc_repo, ref=b'refs/merge-requests/12/head', revision=b'76ac23fe' * 5)) +def test_write_ref_with_old_value(grpc_channel, server_repos_root): + repo_stub = RepositoryServiceStub(grpc_channel) + wrapper, grpc_repo = make_empty_repo(server_repos_root) + change = wrapper.commit_file('foo') + change2 = wrapper.commit_file('foo2') + + repo_stub.WriteRef(WriteRefRequest( + repository=grpc_repo, + ref=b'refs/merge-requests/12/head', + revision=change.hex()) + ) + # Pass the right old_revision + repo_stub.WriteRef(WriteRefRequest( + repository=grpc_repo, + ref=b'refs/merge-requests/12/head', + revision=change2.hex(), + old_revision=change.hex()) + ) + # Check that the old rev does not exist + repo_stub.WriteRef(WriteRefRequest( + repository=grpc_repo, + ref=b'refs/merge-requests/10/head', + revision=change2.hex(), + old_revision=NULL_COMMIT_ID.encode('ascii')) + ) + with pytest.raises(grpc.RpcError) as exc_info: + # Pass the wrong old_revision + repo_stub.WriteRef(WriteRefRequest( + repository=grpc_repo, + ref=b'refs/merge-requests/42/head', + revision=change.hex(), + old_revision=change.hex()) + ) + assert exc_info.value.code() == grpc.StatusCode.INVALID_ARGUMENT + assert "Expected old rev %r" % change.hex() in exc_info.value.details() + + with pytest.raises(grpc.RpcError) as exc_info: + # Wrongly expect old revision to not exist + repo_stub.WriteRef(WriteRefRequest( + repository=grpc_repo, + ref=b'refs/merge-requests/10/head', + revision=change.hex(), + old_revision=NULL_COMMIT_ID.encode('ascii')) + ) + assert exc_info.value.code() == grpc.StatusCode.INVALID_ARGUMENT + assert "Expected old rev to not exist" in exc_info.value.details() + + @contextmanager def get_archive_tarfile(stub, grpc_repo, commit_id, path=b''): with BytesIO() as fobj: