Summary

Create a mechanism to fetch SVN repository info via JSON

Review Request #220 — Created Feb. 6, 2008 and submitted

Information

Owner

plumpy

Repository

Review Board SVN (deprecated)

Branch

trunk

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

I'm writing a plugin for IntelliJ IDEA (a Java IDE) that will publish code reviews to Review Board (http://code.google.com/p/idea-reviewboard/).  Currently our company has a similar plugin for Code Striker, so getting this working is the last thing we need before everyone will migrate to Review Board.

What I'd like to be able to do is have the plugin inspect the local checkout and then automatically determine the correct repository and base diff path.  This is complicated by the fact that Subversion checkouts might not use the same repository URL as the server.  For example, one person might be using "http://svn/repos/" while another uses "http://svn.scm/repos" while another has an NFS mount and uses "file:///mnt/svn/repos".  The only sure way to identify same repositories is by the repository UUID.  Similarly, to determine the correct base diff path, I need to know the repository root (in case Review Board is configured with "http://svn/repos/trunk" or something).

So this is some code that lets me get that information from Review Board via JSON.  I'm not sure if this is the best way to go about it, but it serves my purposes.  It's perhaps a little weird to throw a method in there that only works for Subversion repositories.  Maybe you can come up with something better?  But I'd love it if this could get checked in.  Otherwise I'll have to tell people to apply a patch to their server if they want to use my plugin.

Let me know what you think.  Thanks!

Testing Done

I've used this to submit reviews.  It works for me.

I'm curious how you make use of this in your IDE. Got any sample code (pseudo-code would be fine).

Also, would you mind updating this for the new API changes that went in? It's basically s/JSON/WebAPI/g and s/json/webapi/g/

plumpy Feb. 21, 2008, 4:27 a.m.

Okay, I updated the diff so that it works with trunk svn.

I took the (ugly, static-typed) Java and kinda converted it into prettier pseudo-Python (my Python is sadly kinda rusty, so it might not be perfect):

# The second argument is a list of all the repositories on the ReviewBoard 
# server.
def findRepository( project, repositories ):
    vcs = project.vcs
    localInfo = vcs.getInfo( project.baseDir )
    localUuid = localInfo.repositoryUUID
    # This gives you the directory of the repository you've checked out.  So if
    # the repository is "http://svn/repos" and you checked out
    # "http://svn/repos/trunk/reviewboard", this will return "trunk/reviewboard"
    localRelative = getRelativePath( localInfo.URL, 
                                     localInfo.repositoryRootURL )

    client = ReviewBoardPlugin.getClient( project )

    svnRepositories = filterSvnRepositories( repositories )

    for repository in svnRepositories:
        try:
            info = client.getRepositoryInfo( repository.id )
            remoteUuid = info.repositoryUUID
            if localUuid != remoteUuid:
                continue;

            remoteRelative = getRelativePath( info.URL, info.repositoryRootURL )

            # Now, if the remote relative path is "trunk" and we have
            # "trunk/reviewboard", this will return "reviewboard".  This is the
            # path we need to hand off to reviewboard as the base diff path.
            relative = getRelativePath( localRelative, remoteRelative )

            # This is the same repository, but they're different paths.  The
            # local checkout might be "trunk/reviewboard" and the one on the
            # server might be "branches/2.0".
            if not relative:
                continue;

            return ( repository, relative );
        except Exception:
            pass

    return None;

trunk/reviewboard/webapi/json.py (Diff revision 4)

This constant name doesn't really match up with its meaning.  Perhaps REPO_NOT_IMPLEMENTED ?

plumpy March 12, 2008, 2:58 a.m.

Yeah, good point.  I'll fix that in a new patch once I understand what you mean on the below comment.

trunk/reviewboard/webapi/json.py (Diff revision 4)

This should probably return a generic error instead of just dropping the request.

plumpy March 12, 2008, 2:57 a.m.

I'm sorry; I don't understand what you mean here.  Do you think there should be more info in the JSON error besides just the error type?

david March 14, 2008, 3:31 p.m.
```
I meant in cases of other exceptions.
```

chipx86 March 31, 2008, 7:33 p.m.

I want to see this get in, so to check with what David meant, did you want something like:

try:
    return WebAPIResponse(...)
except NotImplementedError:
    return WebAPIResponseError(request, INVALID_REPO_TYPE)
except Exception, e:
    return WebAPIResponseError(request, something else?)

david April 1, 2008, 2:35 a.m.
```
Yes.
```

trunk/reviewboard/webapi/json.py (Diff revision 5)

Maybe this should just catch SCMError?  I dunno.  Is this more along the lines of what you were thinking?

david April 4, 2008, 4:10 a.m.

Yes, this is what I meant.  In future, you can just do "except:".  I'll give this one more look-through, and then probably commit it.  Thanks!

Ship it!

```
Committed as r1263.  Thanks!
```