Community
Participate
Working Groups
Tagging with a tag name that already exists should not end up with an Internal Server Error. It should not matter if you tried to tag a commit that already had been tagged with the tag or you tried to tag a different commit (move a tag?).
https://github.com/pjanik/orion.server/tree/bug346285 I wrote all this code and have the rights to contribute it to Eclipse under the eclipse.org web site terms of use. Forced update solves all the problems. 500 is not thrown and it also moves a tag when it's needed.
Nice try, but this is not what we want ;) When tagging the user will not be aware that he's replacing an existing tag. Moreover, he needs to refresh the page to figure out which commit he has just tagged. What we need here is a user friendly message saying why tagging failed. This makes this bug somehow similar to bug 353135.
So, do you want to move tag or return error message? I can see some inconsistency. ;)
When did I say I want to move the tag? What I meant in comment 0 is that both actions (tagging a commit with a tag which was already used to on that commit and tagging a different commit with an existing tag) should have the same output. An output different than 500.
Updated. https://github.com/pjanik/orion.server/tree/bug346285 https://github.com/pjanik/orion.client/tree/bug346285 500 is still thrown as this action causes JGitInternalException and I cannot distinguish a cause (i.e. that tag already exist etc.). It's JGit specific problem. I've added in client showing error on the status panel. Now, detailed message from JGit is show (it isn't beautiful). Other option is to show just "An error occured when tagging.", but we can miss some useful information in other error case, when JGitInternalException is thrown and detailed message is more descriptive.
Do we have a test case covering this? Also, once you've modified GitCommitHandlerV1.tag(HttpServletRequest, HttpServletResponse, Repository, String, String, boolean) I think you should do the same with GitTagHandlerV1.handlePost(HttpServletRequest, HttpServletResponse, String) even though it's not called by the client. Test cases for both ways of tagging would be required to make sure they both fail in the same manner.
(In reply to comment #6) > GitCommitHandlerV1.tag(HttpServletRequest, HttpServletResponse, Repository, String, String, boolean) btw, the signature can be cleaned up in terms of thrown exceptions.
Fixed, test added.
Fixed with http://git.eclipse.org/c/orion/org.eclipse.orion.server.git/commit/?id=5c2cad5db4ed8c089e341bed295d24a220e23484 and http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=ff8867ca40bc0b21e482681c16ccf80cca47873d.
