Hi,
Using 1.10.1 we are seeing different results when enabling HttpV2 for a test that should trigger a commit conflict. With HttpV2, the commit completes despite the base revision being outdated.
The test uses our CmsCommit abstraction but I hope it is readable as pseudo-code anyway.
The repo has a single commit with 1 file “/dir/t2.txt” containing “c\nd\n”.
CmsCommit c = new CmsCommitSvnkitEditor(repo, repo.getSvnkitProvider());
RepoRevision baseRev = new RepoRevision(1, new Date(1));
CmsItemPath path = new CmsItemPath("/dir/t2.txt");
CmsPatchset change = new CmsPatchset(repo, baseRev);
InputStream base = new ByteArrayInputStream("c\nd\n".getBytes());
InputStream work = new ByteArrayInputStream("c\nd\ne\n".getBytes());
change.add(new FileModification(path, base, work));
RepoRevision r1 = c.run(change);
assertEquals(2L, r1.getNumber());
CmsPatchset change2 = new CmsPatchset(repo, baseRev); // Using the outdated baseRev
InputStream work2 = new ByteArrayInputStream("c\nX\ne\n".getBytes());
change2.add(new FileModification(path, base, work2)); // Using the outdated base content.
try {
RepoRevision r2 = c.run(change2);
assertEquals("This commit should not be allowed...", 3L, r2.getNumber());
fail("Should have detected conflict and reported a dedicated exception. Test httpv2 setting: " + httpv2);
} catch (CmsItemConflictException e) {
assertEquals(new CmsItemPath("/dir/t2.txt"), e.getPath());
assertEquals("Detected conflict at '/dir/t2.txt'.", e.getMessage());
}
Hello Thomas,
thanks for the report and the pseudo-code. Even though the idea of the code was pretty clear something was not, so let me ask you for the details.
Are you using SVNRepository-level API or are you using higher-level SvnOperationFactory-level API (or maybe its outdated analogue SVNClientManager-based API which is the same level API but was replaced with SvnOperationFactory and is supported only for backward compatibility and may lack some features)? In other words, do you have a working copy and commit from it or do you create the commit “on the fly”?
Another question: do you check your checksums? If you’re using higher-level API working with working copy, the checksums are checked automatically while when using low level SVNRepository-based API, there’re ways to disable checksums checks by passing ‘null’ instead of checksum at certain places.
Also, if you use low-level API, are you sending the changes as deltas using SVNDeltaGenerator or as full content (regardless of base content).
I’m asking these questions because I was able to reproduce the behaviour you observe (no “409 Conflict” response from the server) but I couldn’t get the new revision created. The first problem was “checksum mismatch” when checking the base checksum. When I artificially disabled checksums checks, it was “500 Internal error” because after that it couldn’t apply delta to the wrong base, so I’m curious how you’ve managed to create a new revision.
The new JAR with the fix can be found at: “./svnkit/build/libs/” subdirectory after building. Could you try it? If it fixes your problem, we’ll include the fix into the next SVNKit release.
Thanks for the quick reply and commit. I have not yet tested with your commit so the below applies to SVNKit 1.10.1.
We are driving an ISVNEditor “on the fly” using an SVNDeltaGenerator.
You are correct to suspect that this test is executed with null base checksum. Our API supports both with and without base checksum and this test verifies that a conflict is detected even when using the API the “sloppy way”. I will add an additional test with base checksum.
I verified the repository content and log again, without base checksum:
HttpV2 disabled: 409 from Apache
HttpV2 enables: success from Apache, there is r3 with content:
c
X
e
When setting the base checksum:
HttpV2 disabled: 409 from Apache
HttpV2 enables: 403 (PUT) and 500 from Apache, based on Apache log. E200014 Base checksum mismatch
Looked at the diff from your recent commit. That seems very promising. I am building now and will test.
Thanks for the feedback. Indeed, the whole idea of HttpV2 is that there’re a bit different HTTP requests and their number is lower so one shouldn’t expect absolutely the same behaviour from HTTP level.
In particular, HTTPv2 dpesn’t fail with “Could not fetch resource information.” just because it doesn’t fetch this information in the first place and this is why it is supposed to be faster.
At SVN level the behaviour shouldn’t change (by the way, it’s not defined at SVN level at which moment the commit should fail, e.g. if you’re changing a file without having permissions: on openFile() or on closeFile() — this may depend on certain protocol — but it should fail eventually regardless).
By the way I wouldn’t recommend you to rely on certain error messages but use SVNErrorCode instead (whenever possible). This SVNErrorCode is defined for every SVNException and is independent of localization (though SVNKit is not localized).
We are using the error codes, a number of statements like this convert into our Exception classes (we have just 4 different exceptions we want to differentiate).
// Commit conflict E160024
if (SVNErrorCode.FS_CONFLICT.equals(errorMessage.getErrorCode())) {
throw new CmsItemConflictException(repository, path, null);
}
A tricky part is to consistently get the path to the problematic file/folder from the SVNException. It is sometimes available as on object to the message but that is often not a user-friendly path (path to a txn or similar). I have sorted it out in most situations by keeping track of which item was being processed when the exception occurred.
I have another edge case where I am parsing the 423 HTTP code from the message. When attempting to delete a file that another user has locked. The error code is the generic RA_DAV_REQUEST_FAILED.