[mythtv] Services API and Backwards Compatibility

[David Engel]

On Sat, Dec 28, 2019 at 11:54:55AM -0500, Peter Bennett wrote:
> 
> 
> On 12/26/19 3:36 PM, David Engel wrote:
> > This is a follow up to the thread I started back in January entitled
> > "Channel Management Ideas and Plans".  I might be slow but I'm
> > hopefully persistent. :)
> > 
> > http://lists.mythtv.org/pipermail/mythtv-dev/2019-January/077527.html
> > 
> > I have a patch ready which implements the "never" and "always" values
> > for channel.visible.  I intend for this change to be included in
> > MythTV version 31.  I also intend to provide backwards compatibility
> > in the services API for version 30.
> > 
> > To provide backwards compatibility on reads, the now, legacy variable
> > named "Visible" still returns a boolean to API clients.  A new
> > variable named ExtendedVisible returns the real, 4-value enum as a
> > string to the client.
> > 
> > To provide backwards compatibility on writes, the backend first looks
> > for the new variable named ExtendedVisible and uses it if present.  If
> > ExtendedVisible isn't present, the backend looks for the legacy
> > variable named Visible and tries to use it.
> > 
> > There are some cases dealing with backwards compatibility where the
> > best behavior isn't obvious.  That is the reason for this email.  I'd
> > like some feedback on how these cases should be handled before I
> > commit the patch.
> > 
> > Here are the cases.
> > 
> > Existing Value      Legacy Set      Proposed Result
> > -------------------------------------------------------
> > Always Visible      True            Leave as Always Visible (*)
> > Always Visible      False           Return Error (**)
> > Never Visible       True            Return Error (**)
> > Never Visible       False           Leave as Never Visible (*)
> > 
> > (*)The assumption is the user or utility invoking this API call does
> > not intend to change the value so silently ignore this change.
> > 
> > I originally returned an error for these cases but was persuaded to
> > change them to the proposed behavior.  The rationale is that the most
> > common use pattern by a client is to read the current values, make the
> > desired changes and then send all values back.  Since the legacy value
> > sent back from the client is the same as the one originally sent to
> > the client, it's reasonable to assume that no change is intended.
> > 
> > (**)The intent of the new, always/never values is to prevent automated
> > utilities from changing the value.  User initiated changes should
> > still be allowed, though.  The backend can't tell who initiated the
> > API call, however, so return a descriptive error saying why the change
> > isn't allowed.
> > 
> > A user who got a preview of this change has requested that the
> > behavior for these cases be changed from returning an error to
> > silently ignoring the change to channel.visible but honoring all other
> > changes in the API call.  I don't feel that behavior is correct.  The
> > user has explicitly enabled the new functionality to prevent such
> > inadvertent changes.  If such a change is attempted, an error should
> > be reported so the user can take an appropriate action.
> > 
> > David
> 
> Looks reasonable.
> 
> If the new extended visible is supplied, will the legacy visible field be
> ignored? Or perhaps they should be mutually exclusive, so that you cannot
> supply both?

If ExtendedVisible is present, then legace Visible is ignored.  Your
suggestion does bring up an interesting case I hadn't fully
considered.  IMO, there are two approaches a client can use to update
an entity.

The first, update approach is to fetch all current values and then
include all changed and unchanged values in the update call.  An old
client using this approach to change visible would always have that
change silently ignored even if it is a valid change.  That doesn't
strike me as good.

The second, update approach is to only include the changed values in
the update call.  That would result in a more reasonable result in all
cases -- the change applied, the change silently and reasonably
applied or an error.

Hmm, requiring the client to provide some other, explicit value
indicating which API version they want to use could solve both cases.
Is the extra complexity worth it, though?  I don't know.

David
-- 
David Engel
david at istwok.net