Summary

Add a generic WebAPI app to Djblets

Review Request #221 — Created Feb. 7, 2008 and submitted

Information

Owner

chipx86

Repository

Navi (deprecated)

Branch

trunk

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Moved much of our Review Board JSON code to Djblets and changed it to be more generic. While it only outputs JSON right now, it will be updated in an upcoming change to output XML files, if specified. We'll be able to include our webapi urls.py twice, once with a JSON path and once with an XML path, and we should be able to access the data both ways.

To make this more generic, we no longer hard-code an object encoder. Now we use the ones specified in the project's settings.py file, using the WEB_API_ENCODERS variable.

There's a base WebAPIEncoder class that can be overridden to provide custom encoders. We provide a BasicAPIEncoder, but projects can include their own and add it to the WEB_API_ENCODERS list. The serializer code will iterate through all registered encoders, trying to find a suitable encoder for the particular object.

Testing Done

Updated Review Board to use these new classes. It appears to be working so far.

/trunk/djblets/djblets/webapi/auth.py (Diff revision 1)

1 blank line between standard libraries and 3rd-party libraries.

chipx86 Feb. 9, 2008, 9:43 a.m.
```
Done.
```

/trunk/djblets/djblets/webapi/core.py (Diff revision 1)

This should probably be named something like BasicJSONEncoder

chipx86 Feb. 9, 2008, 9:36 a.m.

While it's deriving from a JSONEncoder, we're going to use it for the XML stuff as well. There's nothing JSON-specific in it. I could call it BasicAPIEncoder.

david Feb. 9, 2008, 1:54 p.m.

I'd like an explanation of how it's not JSON-specific in a comment.

/trunk/djblets/djblets/webapi/core.py (Diff revision 1)

I don't like this going into settings.  We really ought to try to keep settings entirely focused on actual user deployments, and not let implementation details slip in.

How about you construct a WebAPIResponse with an encoder?

chipx86 Feb. 9, 2008, 9:38 a.m.

Then you'd have to do it with every single WebAPIResponse. That's *really* ugly and a pain to do. I'd rather define it in one place. Though I agree that settings may not be the best place, I can't think of another option that I'm okay with.

I see this as being no different from TEMPLATE_LOADERS, MIDDLEWARE_CLASSES, TEMPLATE_CONTEXT_PROCESSORS, INSTALLED_APPS, AUTH_PROFILE_MODULE, or TEST_RUNNER.

david Feb. 9, 2008, 1:57 p.m.

I guess.  It makes me kind of sad, but oh well.

/trunk/djblets/djblets/webapi/core.py (Diff revision 1)

Either remove this or explain why it's commented out.

/trunk/djblets/djblets/webapi/core.py (Diff revision 1)

404 isn't a useful error here, and won't help anyone trying to implement something other than JSON.

chipx86 Feb. 9, 2008, 9:42 a.m.

The idea is that you'll include a site's api's urls.py file and have api_format set to "json" or "xml" or something. It'll be part of the URL, so if it's an unknown type, the URL for it shouldn't exist. So you'd have, say, /api/json/* and /api/xml/*. Thus a /api/fdshkjshfa/* would be invalid, a 404.

This infrastructure isn't designed for people to implement whatever API format they want on top of this. I'll be putting in JSON and XML, but beyond that I don't consider it very important to let users define some custom format. If I'm convinced otherwise, I can try making it more scalable, but it's not a priority for me, as this is going to get us the 90%.

david Feb. 9, 2008, 1:54 p.m.
```
OK.
```

Ship it!

Looks good.

I wonder if we could maybe move some of the encoder logic into the models themselves.  Do something like admin/meta and have a "class WebAPI" that defined a method to encode itself.  This way the basic encoder could check that, and you'd keep model-related smarts closer to the model.

chipx86 Feb. 11, 2008, 5:35 p.m.

Thanks for the quick review.

Yeah, we could do that. It's nice having a separation so that you can encode models from third party apps, but we could make BasicAPIEncoder check for the WebAPI class in the model and call encode on it. I'll get to that in a later change though.