Re: Extensions to REST-API
"John P. Rouillard" <[email protected]>
| Newsgroups | gmane.comp.bug-tracking.roundup.devel |
|---|---|
| Message-ID | <[email protected]> |
Hi Ralf: In message <[email protected]>, Ralf Schlatterbeck writes: >On Thu, Mar 28, 2019 at 09:05:22AM -0400, John P. Rouillard wrote: >> Hi Ralf: >> >> In message <[email protected]>, >> Ralf Schlatterbeck writes: >> >I'm currently missing some more things in the REST API before it becomes >> >really useful: >> >- The ability to look up keys of items in the database, e.g., say I want >> > to find the status with the name 'closed'. ... >> > The string search is a case insensitive substring search and >> > will find stati where the name includes the string 'closed'. So if we >> > have another status with the name 'almost closed' it will also be >> > found (because it contains the substring 'closed'). >> >> My thought was: >> >> rest/data/issue?@q=title:string >> >> replaces >> >> title=string >> >> so title=string is an absolute match while @q... is the substring >> match. So rest/data/status?@q=name:closed would be a substring search >> and rest/data/status?name=closed would be the absolute name. This >> would also mean that @q would play the role of the all text search >> capability on the web interface. > >Sounds nice. Needs an update to filter/filter_iter methods in the >backends. Does filter/filter_iter perform a case insensitive substring search? >For now I've solved that as indicated with > >rest/data/status/closed >or more verbose >rest/data/status/name=closed > >Note that the name= variant is only needed if the keys can be numeric >(in which case we would not be able to distinguish a key from an ID) Yup that works. >But absolute string matching is definitely something we should >implement before the next release. The @q parameter for the substring >searching sounds nice. I remember @q as query for text. >If you want to test my recent changes, be sure to update, I've pushed a >bug-fix (I had reused a variable). Done. >Also note that I've remove a lower() for the path, this lead to keyword >searches failing if the keys included uppercase letters (path includes >url parameters which are case sensitive!) How did that sneak in? URL paths are case sensitive right? >> Speaking of all text search, I also did this: >> >> rest/data/msg?content=testing >> >> I gave up after 3 minutes 8-). I think the content is being searched >> by grabbing each item and searching content without using the FTS. So >> some mechanism to tie into the full text search index is probably >> needed. > >Yes, maybe just document that text matching (being it absolute or >substring) on a content prop will not work. And maybe add index searches >some time in the future. Fair enough. How does the index search get done currently? Searching for a word in "all text" searches uses the index so it should be available. I expect we would use the same code. >Concerning index: I'm not very happy with the current state of affairs >of indexing binary files, as far as I understand the indexer currently >indexes everything. Joseph gave a more detailed answer, but only text is indexed. >> > IMO this should be extended to allow key attributes directly in the >> > URL, e.g. .../data/status/closed >> > Now this would work only for classes that have a key attribute (so we >> > can't search for issue title this way) but would be nice for finding a >> > certain status or a user (.../data/user/example_username) >> >> Agreed, id or key field should work. One question if closed is a >> retired status, what gets returned? I don't think 200 is correct as a >> retired state indicates it's not supposed to be used. I would claim at >> minimum 404 (not found), but 410 Gone may be better. At least 410 >> indicates that a action PATCH request to restore/unretire the status >> would be accepted. > >Currently this would return a 404. The retired status will not show up >in searches. Note that I implemented a retired parameter to the filter >method in hyperdb recently, so we *could* search for retired items. This >however would require two searches or iterating over several returned >items of which one may not be retired. So not something we want to do in >the general case: There can be multiple retired items for a single key >prop! Hmm, I could have sworn I retired an issue and could still do a get on the issue. I just tested: curl -X DELETE -s -u amin:amin --header \ 'if-match: "ff974f63e4ebfba1ecf29aa4cc508c15"' .../demo/rest/data/status/9 got a 200 ok. Then I did a: curl -s -u amin:amin \ https://rouilj.dynamic-dns.net/demo/rest/data/status/9 and got back a 200 status and the normal info with the same etag. Doing a: curl -s -u amin:amin \ https://rouilj.dynamic-dns.net/demo/rest/data/status does not show the deleted item. So I am going to claim that a retired item is not 404'ed by the rest interface. I also tried restoring the status with: curl -X PATCH -s -u amin:amin --header \ 'if-match: "ff974f63e4ebfba1ecf29aa4cc508c15"'\ '.../rest/data/status/9?@op=action&@action_name=restore' I got back: { "data": { "type": "status", "link": ".../rest/data/status/9", "id": "9", "attribute": {} } } but it doesn't show up in rest/data/status. I finally was able to restore it using roundup-admin. >> >- The query on a class should directly yield the attributes if >> > requested, so if we specify, say, @verbose=3 we would get all >> > attributes (except for content) along with the query, e.g. >> > /data/status?@verbose=3 would yield >> > 'data' : >> > '@total_size': '3' >> > , 'collection': >> > [ { 'link': ... >> > , 'id': '1' >> > , 'name': 'open' >> > , 'attributes' : >> > { 'name' : ... >> > , ... >> > } >> > } >> > , ... >> > ] >> > If this is not possible we're wasting a lot of roundtrip time to >> > request each item of which we only have the id (and the label property >> > if @verbose is >1 according to Johns latest update) in turn. Similar >> > to the same problem in XMLRPC and in the native roundup API (which >> > does a separate database request for each item). >> >> I have code enabling @embed=propname:propname:propname so the client >> can request any props they want at the class level. I am testing and >> can push it (maybe to a branch) this evening but it's live at: >> >> https://rouilj.dyn amic-dns.net/demo/rest/data/ >> >> (just remove the space). > >Sounds nice. How about @attributes instead of @embed, or is embed >something other interfaces are using? Maybe @attr as it's shorter? Embed is a term: https://developer.wordpress.org/rest-api/using-the-rest-api/linking-and-embedding/ uses _embed. Search for: rest embed turns up more links. https://stackoverflow.com/questions/19991232/include-embed-vs-link-in-restful-apis discusses the use of the term embed in HAL. Also it is seen here: https://evertpot.com/rest-embedding-hal-http2/ However it looks like embed is used to include other resources in the response. so more like: { data: { ... status: { link: "rest/data/status/2" } ... } _embedded: { status: { link: "rest/data/status/2", name: ... , id: ... , order: ... , help: ... , other_props.... } } so it's not quite the same. >I'm also thinking about using @verbose here similar to querying a single >item and with the same semantics as with a single item, how about: > >@verbose single item class >0 only IDs for links No attributes per item What would this look like for class? Just an id? >1 only IDs for links Attributes Why would just attributes be useful for a class? Without id or link there is no way to identify/retrieve the entries in the class. When using verbose level 0 or 1 with a single item, I assume the attributes json dict can be used as the payload to PUT or POST. Because at level 2 or above, the values for links/multilinks can't be posted to the single item. >2 id + link Attributes with id + link >3 id + link + key Attributes with id + link + key >4 + content + content? > >Note that I've renumbered the @verbose setting for single items, the new >default would be 2 (same behaviour as now with @verbose=1). >I don't think having verbose=0 only returning the id for the single-item >case is sane but we could do it for consitency. If the json returned by verbose=0 is supposed to be used for a POST/PUT then returning: "attributes": { ... "assignedto": "3" } makes perfect sense. I think verbose=0 should always return json that could be used to PUT to the same resource which will produce no change/update. This does raise the question, what to do about read only fields like: activity, creator when they are included in a POST or PUT? (Adding @protected=true to the query will include them.) I claim in a PUT, the values of the protected fields should be compared to the existing values. If they are the same (i.e. unchanged), they are ignored. If they are different then a 400 (or maybe a 409) gets returned with a suitable error report ("Error: can not set read only parameter 'creator'". In a POST, do we ignore the values, or require the client to remove them? Ignoring them may be nicer, but I think the presence of protected items should cause a failure (again 400 or 409). >This could be orthogonal to a @embed / @attributes setting, the @embed >would override the @verbose. I actually implemented part of @verbose in terms of embed. if verbose > 1: label = class_obj.labelprop() embed_fields.append(class_obj.labelprop()) then if embed_fields: for embed in embed_fields: id = obj['id'] if self.db.security.hasPermission( 'View', uid, class_name, property=embed, itemid=id): obj[embed] = class_obj.get(id, embed) so the labelprop data is a special case of embed in my code. basically it is like @embed=name or @embed=title where the name/title is magically correct for the class you are querying. One question I do have is should be have: "name": "..." or "title": "..." in the json output? How is the client supposed to know that value is the label for the item? Should we include labelprop in the output: { data: { "labelprop": "name", "attributes": { "name": "..." } } } { data: { "labelprop": "title", "attributes": { "title": "..." } } } in @verbose=2 or higher output? >And the @embed could also be specified for single items in which case >we could reduce the shown properties to only non-multilinks for >performance reasons. if @attr (or @embed) is used for a non-link item, then only those items need to be returned (plus labelprop if @verbose > 2). If one of the items is a link/multilink, then the @verbose setting is consulted to see how it should be represented: an id or list of id's an object or list of objects with id and link an object or list of objects with id, link and labelprop There needs to be some way to get the labelprop as there is no way to retrieve that info. I like the @verbose way to enable that, but having an @labelprop query parameter to enable it would work as well. This makes @verbose strictly control the display of link and multilink items which makes it orthoginal. >Speaking of multilinks, I've thought about a property @multilink which >by default is 1 (show them) and could be set to 0. Especially useful for >fast display of index lists (say in a responsive interface) where >normally multilinks don't make much sense anyway. And of course for all >sorts of searches where multilinks aren't needed. I would just let the user specify @attr and @verbose. They can just not put the multilink field in the @attr list. >> >One of the problems I'm seeing is different values of @verbose on >> >/data/issue and /data/issue/1 have different meaning with this >> >implementation. So maybe we never put the label property in there >> >(currently as implemented with @verbose>1), use @verbose=1 for the >> >output example above, make @verbose=1 the default (as in /data/issue/1), >> >provide the old output with @verbose=0 and add links to other items with >> >@verbose=2 and content with @verbose=3 like when requesting a single >> >issue? We could also allow pruning the attributes by specifying an >> >explicit @attributes parameter (also proposed by John some time ago). >> >> I think @attributes pruning was replaced by @embed in a later discussion. > >OK, I didn't notice, why (I'm fine with either variant) > >> However there is a more subtle issue with @embed. If I go to >> >> curl -u demo:demo 'https://rouilj.dyn amic-dns.net/demo/rest/data/status?@embed=transitions&@verbose=2' >> >> I get an object like: >> >> { >> "id": "9", >> "link": ".../rest/data/status/9", >> "name": "delete", >> "transitions": [ >> "1", >> "2", >> "4" >> ] >> } >> >> Note the format of the transitions. It is the same as though >> @verbose=0 was used to access rest/data/status/9 even though I have >> @verbose=2. > >See my discussion of verbose above: Verbose=1 would embed all attributes >(unless modified with @embed or @attributes or however we'll call it). >And higher values would specify more expansion Agreed, what I was showing is that the query used verbose=2, but the transitions property is displayed as though verbose=0. >Note that @verbose as currently implemented only works for single items >unless you've implemented something new there. Using @verbose>1 on a collection endpoint adds the data on the labelprop to a collection as of 1fa59181ce58. >> There is no way to tell what you can do with a transition from this >> output. They are statuses and can be retrieved via rest/data/status/1 >> etc., but you have to go to rest/data/status/9 with @verbose>0 to see >> that. If I do a get on rest/data/status/9 with no @verbose, I get the >> transitions expanded: >> >> "attributes": { >> "name": "delete", >> "requiredpermissions": null, >> "order": 9, >> "abbreviation": "d", >> "transitions": [ >> { >> "id": "1", >> "link": ".../rest/data/status/1", >> "name": "new" >> }, >> { >> "id": "2", >> "link": ".../rest/data/status/2", >> "name": "open" >> }, >> { >> "id": "4", >> "link": ".../rest/data/status/4", >> "name": "hold" >> } >> ], >> "help": "Ticket opened by mistake or in error. Considered closed." >> }, >> "type": "status" >> >> I was considering refactoring the class and class/id (or key name) >> endpoint code into a single function. So when I asked for: >> >> curl -u demo:demo '.../rest/data/status?@embed=transitions&@verbose=2' >> >> I would get the embedded fields (e.g. transitions) for each element of >> the collection expressed as though I had hit each rest/data/status/1, >> rest/data/status/2, rest/data/status/3... with @verbose=2. >> >> So the code re-arrangement would be: >[...] > >Yes! > >> This would unify the representation of the objects regardless of which >> way they were accessed (class (+ optional embed), class/id (+ optional >> embed)) for a given verbose level. >Yes! > >> If the property is requested/shown (either by an explicit @embed, or >> implicitly by requesting the class/id) it always has the exact same >> form based on the @verbose level. >Yes! > Hmm, seems we have agreement here 8-). >But see my discussion above for the verbosity: We'll want to use 0 for >not showing/embedding attributes when listing a class. so @verbose=0 means @embed is ignored? >> One note, I was discussing this with a developer over the weekend and >> he noted that: >> >> @verbose >> >> might better be named: >> >> @detail or @detailevel >> >> In most Unix commands enabling verbose (-v) doesn't change the command >> output. It produces info about how the output was obtained. In our >> case, it may add a field at the same level as "data" with: >> >> "perfdata": { "totalscanneditems": 2100, >> "real_time": "2.4s", >> "cputime": "1.2s", >> "cachemisses": 900 >> "cachehits": 3210 >> } >> >> Not sure I agree with him, but thought I would mention it. >> >> Thoughts? > >I'm not really religious about this naming. I do have a customer testing >(and implementing a client) for this now, so I'm inclined to leave it >as-is... but if you want it changed I'm in favor of the shorter detail >not detaillevel (two 'l'?) Yeah probably two l's. If @verbose only changes how links and multilinks are represented, then maybe @linkrepr or @linkfmt is a more descriptive (but longer) name. >Another thought: We currently can't attach files with binary data via >the REST-API, can we? I am not sure. Using Content-Type: application/json, I would say no. Using Contet-Type: application/x-www-form-urlencoded I think would work. It uses exactly the same code to parse the input data that the web interface uses. Now that I say that, I am trying to retrieve file/2 which is a jpeg file. All I am getting is: { "error": { "status": 409, "msg": "'utf-8' codec can't decode byte 0xe2 in position 10: invalid continuation byte" } So something is wrong there. Even getting file/2/type, which should work, returns the same error. Have a great week. -- -- rouilj John Rouillard =========================================================================== My employers don't acknowledge my existence much less my opinions.