New XML-RPC Methods (editcircle, getcircle, gettrustgroups)

[murklins]

Back in April, and again in June, catness did a fantastic job of writing new methods for the XML-RPC Protocol that replace the LJ-inherited friends methods. They provide ways of editing user relationships, both on the watch and trust edges, and ways to create/edit/delete access filters and subscription filters.

Not many people commented on the bug, though, so catness didn't get a ton of feedback on the proposed modifications, and none at all as far as I can tell from other people who actually use the API to develop client applications. I recently began moving over the old LJ Server Manual's XML-RPC chapter into the DW wiki, so I read through the patch in an effort to put together some pages for the new getcircle, editcircle and gettrustgroups methods. When I looked through the code, though, I found myself thinking of a few things that should maybe be changed before the new methods get pushed live. Once an API starts getting used by client applications, it becomes much harder to modify, and I thought it was worth having another look before the new methods got put into use.

I first detailed my thoughts in a comment on the bug, but denise suggested I open a discussion here, so I'm going to repost it all and hopefully provoke a few more people into looking over the patch and talking about whether anything would benefit from some tweaks.

It is worth noting here, as I did in my bug comment, that I am almost totally new to DW development, so maybe there's stuff going on that I am not fully grasping yet -- if that proves to be the case, please do not hesitate to set me straight!

1. The editcircle add key seems really awkward to me, as I attempt to document it. For one thing, it is called "add", but it is also the only way to remove watch and trust edges, so the add terminology is counter-intuitive. Probably "edit" is more appropriate. (Actually, I think that the trust group/content filter editing stuff should be moved out of editcircle and into a new editfilters method, then editcircle doesn't need an "add" or "edit" key at all since it will only be used for editing users' watch/trust edges, but that may be too radical a change. Ahahaha, too radical a change, she says! Like I do not proceed to go on and on about radical changes for the next hundred paragraphs.)

   Also, though, if I'm reading this right, the way to add/remove the watch and trust edges is coded such that you can't just modify one or the other. If you want to, say, start trusting a user, you can't just add the trust edge -- you first have to find out if you already watch that user, and then supply the edge key with both with the new trust value and the current watch value. But then again, if you want to change the user's trust mask, then you can do that... but you can't also change their watch edge at the same time. Maybe instead of having a single "edge" key for watch/trust, we could have an "edittrust" key and and "editwatch" key?

2. I wonder a little about the lack of symmetry in terms of how trust groups and content filters are dealt with. I know that on the backend they are very different, but I feel those differences should be hidden as much as possible by the API. Maybe that's just me? Anyway, here's where things are different, from what I can tell:

   • getfriendgroups is deprecated, and gettrustgroups was added, so why not also have getcontentfilters? catness asked about this upthread, and mark said implementation of methods could be piecemeal, but since getcircle already implements the functionality for returning content filters, it would be minimal effort.
   • The includecontentfilters key of getcircle is like the includetrustgroups key, except that is also includes the list of users in the filter. Any reason we can't match that when we get trust groups? I know that it's more calls on the backend, but that doesn't seem like a deal-breaker?

   • The way a user's trust groups are set with the add key but a user's content filters are set with separate keys is probably not going to seem intuitive for devs using the API, even though it makes sense from an implementation perspective. catness brought this up in her initial description of the addtocontentfilters key: 'Maybe it's better not to make a special keyword for it but to have an additional keyword in "add"?' So yeah, that's one option, to move it into add in some fashion. Or maybe it's not even worth trying to bring it more into line with how we edit trust groups, since those use masks and content filters don't... Or maybe we should hide the whole mask aspect of trust groups from the API level anyway, and change the add key accordingly. So then you might have (going with my replacement of the edge key with editwatch and edittrust from (1) above):

     • username - the user to edit
     • editwatch - 0 to remove watch, 1 to add watch, or not included at all to leave as is
     • edittrust - 0 to remove trust, 1 to add trust, or not included at all to leave as is
     • addtocontentfilters - list of filters to add user to
     • deletefromcontentfilters - list of filters to remove user from
     • addtotrustgroups - list of trust groups to add user to
     • deletefromtrustgroups - list of trust groups to remove user from

     Hmm, the groupmask key could also be kept around to facilitate LJ clients supporting DW. I don't know, I'm just spewing out all my thoughts now! Too many thoughts.

   • When you create a content filter, it is included in the API response, but the same is not true when you add a trust group. Presumably, this is because when you specify a non-existent id to edit_trust_groups, it creates a new one with that id so no return struct is needed to inform the client of the new group's id. In contrast, create_content_filter generates a new id for you, so you need to return that id to the client. Maybe it's not worth messing with that for symmetry's sake? But I want to float the idea of changing it so that if settrustgroups receives a non-existent id, it generates the new group's id for you and returns it in a struct, just like setcontentfilters.
3. This patch exposes terms like "edge" and "trust groups" but wouldn't it be better for the API to use terms that are closer to the front-end experience? I wouldn't expect every dev using the API to read the backend code, so they'll probably expect to see terminology more like "editaccess", "editsubscription", "setaccessfilters" and "setsubscriptionfilters". Or maybe I'm alone on this?

Comments

[denise]

If that method doesn't exist, we should probably create it, yes. *G*

htdocs/manage/circle/add.bml has a bit about getting the content filters a user's in, and it does look like it's looking it up based on the filters, not on the users that are in them. We might want to do something about that maybe.

[murklins]

Oh, so that was the page cesy was talking about. Yeah, that looks less than ideally efficient.