RE: Response data should always be in a top-level "value" property.

I’m also in favor of Simon’s response. Just from a “how I think about things” perspective, being specific in that the “value” property always has the response you seek would make it easier to understand from a local end viewpoint. This completely depends on how you structure your code (and what language you’re writing in) but it could add the potential for something like delegates to handle the processing and overall be more DRY (one less branch in code).

From: Simon Stewart [mailto:simon.m.stewart@gmail.com]
Sent: Wednesday, October 5, 2016 10:58 AM
To: Andreas Tolfsen <ato@mozilla.com>
Cc: public-browser-tools-testing <public-browser-tools-testing@w3.org>
Subject: Re: Response data should always be in a top-level "value" property.

On Wed, Oct 5, 2016 at 6:02 PM, Andreas Tolfsen <ato@mozilla.com<mailto:ato@mozilla.com>> wrote:
Sorry I’m late to the party (-;

Simon Stewart <simon.m.stewart@gmail.com<mailto:simon.m.stewart@gmail.com>> writes:

> Proposal: any response from an end node that contains data should put
> that data in a "value" property of a JSON object.

Most importantly, I think this is a very late and untimely change to
make if we want to see the specification in CR-state by end of January.

It seems like a large change, but it's not massive when you audit the actual commands. Indeed, most of the existing implementations already always send the response data in a "value" property.

I was under the impression that the fundamental design of the protocol
would not change; especially not on a decision that was made—at a F2F
none the less—over two years ago.

It'd be nice not to have to change anything, but in this case, as I explained in a previous email, I think that this is a change worth making. Again, there are currently 11 commands, mostly around window sizing, that this impacts.

Mozilla has put a lot of engineering resources into adapting our
implementation to the current definition.  Changing it now will require
a non-insignificant investment on our end.

I'd be happy to help.

I’ve done the backwards compatibility dance around this before, and I’m
hesitant to do it again for what I perceive as only aesthetic changes.

If this was merely aesthetics, I'd not propose the change.


> 1) Processing the result cannot be a two stage process of dumbly
> extracting the return value and then coercing to a type. Instead,
> local ends must know the command being responded to (information not
> included in the response) and switch behaviour as appropriate. This
> additional complexity is needless.

A client will always need to have some level of knowledge of the data
structure that is returned.  If they blindly accept whatever the
server sends, they may break the API contract they give to consumers.

I'm thinking of the two step process that most of the selenium local ends do. They take the returned response, deserialise the data, add the session id, figure out whether it's an exception, and then return the deserialised response as something like a Map, Array, or primitive to the next level up. That next level pulls out the required data from the response.

There's no "blindly" going on, but there is standard software layering.

I buy the argument that it is easier for intermediaries to deal with an
object where the return value is always encoded in the same place
because they may not care about the data itself.  But on the other
hand I think it is entirely reasonable for the local end to know more
about the protocol it is implementing.

See above. The local ends know everything, but they still benefit from a simpler processing model.

For the ‘Set Timeouts’ and ‘Get Element Rect’ commands, any local end
will inevitably have to look up specified keys in the returned JSON
Object before it presents it to the user.  Whether you do this through
some automatic JSON decoder codec (for which you could have one for W3C
and one for Selenium) or manually is besides the point, because at some
level it needs to know something about the protocol data structure.

See above.


> 2) It's inconsistent. Generally, inconsistency is where bugs live.

I think the idea of wrapping everything inside a ‘value’ field has a
whiff of structured XMLness over it.

I don't understand. Also, XML was designed as a human readable data interchange language. From a historical perspective, if hating XML wasn't such a popular thing, I'd have used that for the json wire protocol.

It might make the task of writing the W3C WebDriver-to-Selenium
WebDriver shim easier, but at some level a client and a server needs to
know certain details about what structures and types are returned so I
don’t think it changes anything other than perhaps giving the code in
Selenium a somewhat nicer appearance.

It also opens the door to us being able to extend the protocol if we need to. If the response data can only be the value returned, without decoration, we've shut the door on a path to move the spec forward in later versions should we need to. Decoration also allows annotation of responses with (e.g.) security data, or information about the customer (such as their ID)

My tl;dr on this is that I’m _not strongly opposed_ to the change as
such, but I’d like us to think very carefully about whether we think it
is necessary.  I agree with James that because it only creates another
level of indirection, I too fail to see what it earns us apart from a
certain aesthetic symmetry.

I strongly believe that this isn't just aesthetics.

Moving a specification to Rec requires at least two conforming
implementations, and it will take time to change geckodriver and
potentially the Marionette automation protocol.  Because of this I think
there is a distinct chance that the January CR deadline will slip.

geckodriver is most of the way there already, because of the way that the spec suggests most commands send responses attached to a "value" property. I'm happy to help throw some engineering effort into modifying the remaining commands in geckodriver.

Simon

Received on Wednesday, 5 October 2016 19:01:47 UTC