Re: Extensions to REST-API

Hi Ralf:

In message <[email protected]>,
Ralf Schlatterbeck writes:
>On Thu, Mar 28, 2019 at 09:53:25PM -0400, John P. Rouillard wrote:
>> >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.
>
>In which context? I don't think we currently have an exact string match
>method that can search for other attributes. (There is a stringFind
>method only for rdbms backends that does an exact string match but only
>for String properties)

String search only. I intended it as the query param for all/full text
search.

>> >> 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.
>
>Currently the index search is performed first. It yields a list of
>issues matching. These are then put into the filter method. Normally
>you'd call filter with an empty list of IDs, i.e., filter (None, ...
>but instead of None you can put a list of IDs from a previous fulltext
>search. So, yes, this is suboptimal in terms of performance :-)

Ah gotcha. So the full text search eliminates issues that don't have a
title or message that matches the search terms. Only matching issues
are passed to the filter to do an additional search on other params
like status, nosy etc.

>> 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.
>
>Ah: I was under the impression you were *searching* for retired items.
>These are not found via the filter method (unless you use the new
>'retired' parameter I added recently). But, yes, retired items are
>currently displayed as-is when you specify them via the item ID.
>
>We could change this and return a 404 or something else but this would
>be different from all the other roundup interfaces.

Ugh. I claim a retired item should not be accessible. Basically it
should be like delete but without breaking referential intergity.

>> 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.
>
>Yes, they're not found in a search but when you enter the URL of a
>retired issue it is shown. This is the same btw with the normal web
>interface.

Yup. I misunderstood the impact of retiring things. I though it was
only to keep referential integrity in the db while removing any
attempt to use it in the future.

>> 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.
>
>OK, this is worth a bug-report I guess.
>Looks like restore isn't working as expected yet.

That's my claim.

>> >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?
>
>Yes only link and 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.
>
>With attributes I mean also the link+id  +attributes, e.g.
>
>data
>    { id
>      link
>      attributes {
>      ...
>      }
>    }
>    ...

Ok, that makes sense. For @verbose=2 you had "Attributes with id + link"
which confused me.

>> 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.
>
>Yes, should be fine.
>
>> If the json returned by verbose=0 is supposed to be used for a
>> POST/PUT then returning:
>> 
>> "attributes": { ...
>>                 "assignedto": "3"
>>               }
>> 
>> makes perfect sense.
>Yes.
>
>> I think verbose=0 should always return json that could be used to PUT
>> to the same resource which will produce no change/update.
>ok
>
>> 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'".
>
>OK, fine
>
>> 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).
>
>I don't think they should be sent by the client

That is fine by me. I think they only show up in a get if
@protected=true, so a get with @verbose=0 shouldn't show them and
should be usable as a POST payload.

>> I actually implemented part of @verbose in terms of 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": "..."
>
>Definitely the real name of the label property, so for issue this should
>be 'title', for status 'name'.
>
>> 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": "..."
>>             }
>>      }
>>    }
>...
>> in @verbose=2 or higher output?
>
>Yes, that would be nice. I've also wondered how to document some of the
>special things about the roundup interface, e.g., label prop and order
>prop.

Yeah, I wonder if an @schema or @description at every level where a
GET makes sense should be supported for this.

A GET on /rest, and /rest/data does have any real meaning. So we can
just return schema/discovery data.

For other endpoints, /rest/data/issue/@schema or
/rest/data/issue/24/@schema or /rest/data/issue/24/nosy/@schema could
return something useful. I suspect /rest/data/issue/@schema and
/rest/data/issue/24/@schema would return the same thing.

  { schema: {
     issue: [
	{@link: .../rest/demo/issue}
	{name:title, type: string, },
	{name: nosy, type: multilink, required: false,
	      item_type: .../rest/data/user },
	{name: "id",  type: string, format: numeric },
	{name: creator, readonly=true, type=link .... }
	 ...
     ]
    }
   meta: { issue: { labelprop: "title", orderprop: "id",
           required: ["title", "status"], keyprop: id } }
    }
  }

Note the nosy item has an item_type value. This is a link to a url
reference so .../rest/data/user/@schema could be used to find info
about the things in nosy. Using .../rest/data/user/@schema to find the
name of the keyprop will let the client set the values in the
multilink.

Another way to do this is to add required, labelprop, orderprop
etc. to the items themselves:

{ schema: {
  issue: [
       {name:title, type: string, required: true, labelprop: true },
       {name: nosy, type: list, required: false, item_type: integer_string
        },
       { name: id, type=string, orderprop: true, keyprop: true }
       ...
      ]
}

Maybe than the name field, make the name the key of the dict
like:

  { schema: {
    issue: {
       title: {type: string, required: true, labelprop: true },
       nosy: type: list, item_type: integer_string ...},
       id: {type=string, orderprop: true, keyprop: true }
       ...
      }
  }


I would suggest readonly, labelprop, orderprop, required etc. only
show up where they would have the value true. I like this idea better,
but finding the labelprop, required items etc. for each class would
require iterating over all the properties. Parsing that info on page
reloads could be expensive.

Property schema like /rest/data/issue/24/title/@schema could return:

    { schema: {name:title, type: string, required: true, labelprop: true } }
  
There are a few proposals that attempt to represent schemas in XML and/or
in json. I don't have any real preference/experience in that area.

  https://json-schema.org/
  https://frictionlessdata.io/specs/table-schema/
  https://docs.oasis-open.org/odata/odata-csdl-json/v4.01/cs01/odata-csdl-json-v4.01-cs01.html

and there are proposals offer a way to represent link data.

  https://www.w3.org/2018/json-ld-wg/

>> >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
>
>Yes, but the list of IDs is already a join (or worse, a second database
>lookup per item per multilink).

I just looked. I didn't realise there was a join/lookup there. Makes
sense that's how it would be done in RDBMS.

>So I would like to be able to generally
>remove multilinks from the attributes with some option. This would be
>nice for displays similar to the current index display where multilinks
>do not make much sense (although they're currently allowed).

Does @fields=-multilink make sense? We have an inclusion syntax for
fields but no exclusion. A "-word" value could be interpreted as a
request to disable display of the "word" field. If the schema identifies
the multilinks, the client could just exclude those fields.

>> 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.
>
>This maybe leads back to the discussion if we should have a way of
>retrieving the whole schema, which should include information about key
>properties, label properties and probably order properties. Note that
>the order prop is used in hyperdb.filter if no ordering/grouping is
>specified. So this may also be relevant.

Maybe make the whole schema available at /rest/@schema:

{ schema: {
   issue: [
      {@link: .../rest/demo/issue}
      {name:title, type: string, required: true },
      { name: nosy, type: multilink, required: false,
            item_type: .../rest/data/user
        },
      { name: "id",  type: string, format: numeric },
      {name: creator, readonly=true, type=link .... }
       ...
   ],
   user: [
      { @link: .../rest/data/user }, ...
   ]
  }

(or some other representation). Then a get on /rest can list the
@schema link.

>> >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.
>
>Yes I've thought that, too, probably enough if we can provide a schema.

@multilink=false or 0 could work as well. Another possibility is to
combine @fields with this so: @fields=-@multilink will shut off
multilinks. What happens if we have something like:

  ?@fields=nosy&@multilink=0

I would claim all multilnks except nosy are excluded.

>> >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?
>
>I would interpret @embed to override @verbose but might be convinced
>otherwise :-)

Ok, so a get on rest/data/issue?@verbose=0&@fields=nosy would return
the nosy values. I think that makes sense. It also aligns well with my
interpretation of ?@fields=nosy&@multilink=0.

>> >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 Content-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.
>
>Ah, ok, I have to try this (maybe I'm already using this in my tests
>when specifying data= using the requests library).

Also I think:

  curl -X PUT -H "content-type: image/jpeg"  .../rest/data/file/23/content

should work. Not sure that it does however.

>> 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.
>
>This is because JSON can't represent binary data as Joseph Myers has
>noted in a reply in a different thread

But it shouldn't be trying to represent any binary data. I am not
asking for content at all. If I did want content and I asked for it as:

     curl -X GET --header "Accept: application/json" file/2/content

I would expect a 406 not acceptable. I would also expect a payload like:

  { error: { status: 406, msg: "bad content type",
             @links: { content-type: image-jpg, url: .../file/2/content }
           }
  }

or similar that would allow the client to figure out what content
type is allowed.

Then a:

   curl -X GET --header "Accept: image/jpeg" file/2/content

should work.

>[Reply in different thread:]
>Date: Fri, 22 Mar 2019 13:46:35 +0000 (UTC)
>Message-ID: <[email protected]>
>
>On Fri, Mar 22, 2019 at 01:46:35PM +0000, Joseph Myers wrote:
>>> On Fri, 22 Mar 2019, Ralf Schlatterbeck wrote:
>> 
>> > I honestly don't know what would happen if we attach the file as a byte
>> > string in python3, if the json serializer can handle this.
>> 
>> JSON (the format) does not support binary data (there are workarounds
>> people use such as base64-encoding binaries, but you need to know which
>> content properties are text and which are binary - which is information
>> the Roundup schema doesn't give you at present).
>
>This also leads to the question if we want to display file contents (as
>opposed to message contents) if @verbose is some level. Because it will
>lead to an error in many cases. Or maybe just include the content only
>if we have a certain type (text/plain or text/somethingelse) *and* can
>decode it as utf-8

I would say content is included in json payload only if it is text/*
and can be represented in utf-8. If the content isn't representable,
return a content field that is a link structure with content-type, uri
etc.

Maybe "content" should always be a link structure. If @verbose=3 we
also return a (pseudo) field: content-data, content-raw, @content with
the json utf-8 representation of the text if it can be represented.

Ok, enough musing for the night.
Have a great day.

--
				-- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.