[Corona] Corona explore endpoint and web-interface?

[Geert Josten]

As said, the endpoint code can't check unless you relax the configuration
enough to get it matched when mistakes are made (yes, Corona is using BOTH
halves of the REST library ;). And as you said, not much left to check if
it did match in rewrite..

So, presuming you want the endpoint configurations to be accurate (like I
would prefer because that allows accurate WADL generating, and such) that
leaves fiddling around in rewrite, or add fall-back cases like you
suggested (interesting idea by the way!)..

I happened to have tried a few things with the rewrite. I added a check to
distinguish the case where no matches were found (just like you
suggested), gathering raised-errors, and return a reasonably descriptive
message based on the returned errors (by checking them in a certain
order). It is not as hard as you might think, but it does get a bit tricky
when there are multiple possible matches (as you predicted below). But
even for the tricky cases it should be possible to get the library return
at least some information.

I committed a few changes to my Corona branch to give you an impression of
where it could go. It involves changes to rest-impl:matching-request, and
you need to catch erors in the rewriter. Still work in progress by the
way..

Kind regards,
Geert

-----Oorspronkelijk bericht-----
Van: Norman Walsh [mailto:Norman.Walsh at marklogic.com]
Verzonden: dinsdag 13 december 2011 16:43
Aan: Geert Josten
CC: Colleen Whitney; Ryan Grimm; Corona Email List
Onderwerp: Re: [Corona] Corona explore endpoint and web-interface?

Geert Josten <geert.josten at dayon.nl> writes:
> The thing is that it makes a lot of sense to _do_ use both halves. They
> seem to be there for a purpose. But to me it is giving the impression
that
> it is designed such that step 1 never complains, just return a matching
> endpoint or not. So it looks like you would have to rely on step 2 to do
> the complaining part. But as said, if step 1 already filters out any
> mismatches, then there is nothing left to complain about.

Yes, it is the case that the two halves were designed to work together
and I mostly expect them to be used together. And you're right that if
each half uses the same consistent set of endpoint descriptions, there
can never be anything for the endpoint to complain about.

The status quo is this: the rewriter never complains and the endpoint
mostly doesn't check for errors. The case of a required-but-absent
parameter is the one error condition that the endpoint code does
notice. It just so happens that in the process of building the map,
it's easy to detect a required-but-absent parameter and *not* raising
an error in that case seems like the worst possible option.

What could be changed?

In the rewriter:

Nothing. Missing parameters, different methods, etc. are not errors,
they simply mean that some other endpoint matches (or may match). If
*no* endpoints match, I suppose it's possible that the rewriter could
(under some circumstances, maybe) work out that one of the endpoints
would have matched if some parameter had been different, but (1) that
would be really hard and (2) I think there would often be more than
one possible correction. So I don't think anything can be done in the
rewriter.

In the endpoint:

Check all the conditions again. This is the safest thing to do and I
think it should be possible to request this. Maybe it should even be
the default. But it's going to have a performance impact so I'm not
sure.

Ignore the fact that a required parameter is absent. In my mind this
would diminish the value of the library somewhat. If my description
says that "foo" is a required parameter, I want to be able to access
map:get($params, "foo") with the certain knowledge that I won't get
back an empty sequence. Moving the parameter checking into the
process-request method centralizes the error handling. If the test was
expensive, if the process-request method was doing a bunch of extra
work to determine that this error occurred, I'd be inclined to change
it, but the fact is that the code has to walk through the parameters
anyway, so the error is "right there".

At first glance, you might think that the process-request code could
skip looking at the request parameters entirely and simply return
all the xdmp:get-request-field-names directly. But it can't, because
if the type of the parameter is, say "decimal" then the value has to
be cast from a string to a decimal when it goes in the map.

In short: process-request has to do a certain amount of work. As a
consequence of doing that work, it is only a tiny bit more expensive
to detect the required-but-absent parameter error. Since I view that
as a severe error, I'm not in favor of removing the check.

So I think the status quo strikes a good balance.

> About Corona: to me it would make sense if the endpoint configuration is
> accurate, but that results in endpoints not being matched, and Corona
> bailing out with a non-informative error.

I'm not familiar enough with Corona to understand what you mean, exactly,
but see below.

> Also, the endpoint code is doing
> a lot of checks the REST library is doing as well. Sounds pity to me..

Well, not a lot, just the easy thing.

> I am just looking for a way to get 'the best of both', having the REST
> library doing the majority of the checking, and still getting sensible
> messages. It can be achieved by making the rewrite throw errors, but not
> sure that fits the architecture and recommended use of the REST
library..

If you wish there were better error messages, you can generally
arrange that.

Consider, for example

   <rest:request uri="^/manage/v1/list/package/edit/database$"
                 endpoint="/packaging/edit.xqy"
                 user-params="forbid">
     <rest:http method="POST"/>
     <rest:param name="pkgid" required="true" as="decimal"/>
     <rest:param name="database" required="true"/>
     <rest:param name="format" default="json"/>
     <rest:param name="chk" required="true" as="boolean"/>
   </rest:request>

Suppose that leaving out the pkgid is a common mistake:


http://localhost:8000/manage/v1/list/package/edit/database?database=Docume
nts&chk=true

If there are no other endpoints that match that URI, then you're going
to get the more-or-less uninformative "404" message from the rewriter.

So add this endpoint at the end of the list of endpoints:

   <rest:request uri="^/manage/v1/list/package/edit/database$"
                 endpoint="/packaging/edit-errors.xqy"
                 user-params="allow">
     <rest:http method="POST"/>
     <rest:http method="PUT"/>
     <rest:http method="GET"/>
     <rest:http method="DELETE"/>
   </rest:request>

Then write edit-errors.xqy with full knowledge that it could only have
been called if someone messed up the call to .../edit/databases. This
module can check for the wrong method, missing parameters, the phase
of the moon, whatever, and give comprehensive, useful error messages.

                                        Be seeing you,
                                          norm

-- 
Norman Walsh
Lead Engineer
MarkLogic Corporation
Phone: +1 413 624 6676
www.marklogic.com