node-plex-api icon indicating copy to clipboard operation
node-plex-api copied to clipboard

Handle data format changes in PMS v1.3.x

Open phillipj opened this issue 8 years ago • 17 comments

@benheller from Plex has been kind enough to reach out about breaking changes in the JSON responses from PMS. In PMS v1.3.x there has was introduced some breaking changes in the PMS responses, which I've been told was an effort to prettify, and make the JSON api a supported feature of PMS.

Quoting @benheller:

A pre-1.3.x response might have looked like this:

{
  "_elementType": "MediaContainer",
  "_children": [
    {
      "_elementType": "Hub",
      "size": 0,
      "_children": []
    }
  ]
}

The same response would now be:

{
  "MediaContainer": {
    "size": 12,
    "Hub": []
  }
}

In MOST cases, you can take the _elementType, move it to a top level key (e.g. MediaContainer), and replace _children with its own _elementType (e.g. Hub). That said, the 1.3.0 api changes were inconsistent in places, and on the current release you'll see some keys that do not match the previously reported _elementType properties.
In order to maintain compatibility with both pre- and post-1.3.x versions, the library will need to handle both cases. It may be wise to prioritize XML rather than JSON, which gives you full control over the parsing format via xml2js, so that ALL responses will mimic the "old" style JSON. If a consumer of the library wishes to only support PMS 1.3.x and greater, perhaps you can introduce an option to enable prettified responses?

Initially I agree with Ben on handling both pre / post v1.3.x in this package, so that developers and users of plex-api wouldn't have think about which PMS version they're targeting.

Any thoughts are more than welcome here!

For full disclosure, I cannot work on this for at least or week or two, so any help would be more than welcome.

phillipj avatar Dec 03 '16 13:12 phillipj

Any updates on this issue?? Understanding your previous disclaimer and that I don't have the skills to lend a hand.

ajdexter avatar Jan 18 '17 15:01 ajdexter

@ajdexter afraid there's no updates from me at least...

Any thoughts on how you'd expect this to function? As mentioned in the previous comment, it looks like there's two options:

  1. Add an option to this package, letting users enabling prettified responses
  2. Make this package auto-detect format from the PMS server and possibly transform, ensuring users get one format in the response

The latter might end up putting a lot of complexity into this package, which I've been trying to avoid as much as possible, but that may be worth it if the end-users really expect it.

phillipj avatar Jan 19 '17 21:01 phillipj

Auto-detect and transform the result would be ideal because that means users won't have to change their code at all. 🥇

hyperlink avatar Jan 19 '17 21:01 hyperlink

Sure sounds wonderful for the package users 😄

Which format do we want to transform to?

1. old not so pretty format

{
  "_elementType": "MediaContainer",
  "_children": [
    {
      "_elementType": "Hub",
      "size": 0,
      "_children": []
    }
  ]
}

2. new prettified format

{
  "MediaContainer": {
    "size": 12,
    "Hub": []
  }
}

My five cents would go to the prettified format, as it's more JSON-like and to the point vs the old format which looks like XML represented as JSON.

phillipj avatar Jan 20 '17 07:01 phillipj

One vote for the prettified format as it is indeed more JSON and thus easier to work with in JavaScript

LanderN avatar Jan 20 '17 07:01 LanderN

Not seeing more data with the prettier format. Perhaps a bit more normalized but it looks like the same data structured differently. Either way it will be a breaking change if this module choose to adopt it as dependents will need to update their code. To sum up issues with adopting the new format:

  • Module code will need to be rewritten using the new prettier format
  • Need to create a converter (convert old format to new format)
  • Major version update
  • Dependents now forced to update their code to new format to get working state

Keeping the old format:

  • Minimal changes to module just add converter to response handler
  • Need to create a converter
  • Minor or patch update
  • Dependents don't need to update their code
  • Format still not considered "pretty"

So I lean towards keeping the old format. But ultimately it's up to @phillipj.

BTW I wrote some transform functions last night and I'm hoping to get a branch pushed soon.

hyperlink avatar Jan 20 '17 15:01 hyperlink

Hi all, Plex guy here. Moving forward, you're likely to see more support for the prettified JSON responses. The "old" JSON implementation is largely considered to be unofficial/broken (and I believe actually returns invalid JSON in places). The most robust solution would be to provide an option in the library for responseFormat. It could default to xml, which would make the request using Content-Type: text/xml, run it through an XML > JSON parser, and output something mostly identical to the "old" JSON. The other option could be json, which would make the request using Content-Type: application/json and return the new "prettified" JSON directly. Apps that want to support all PMS versions can use the default XML, and apps that want to support 1.3.x and up can use the json option.

For our internal apps that use node-plex-api we have a :sad_panda: wrapper called getChildren() that looks for child nodes using _children or an array of possible keys (e.g. ['Metadata', 'Directory']). This lets the app be somewhat agnostic to the response format, but it's not ideal. I'd much prefer to get the XML response from the PMS and parse it consistently for all PMS versions (e.g. the xml responseFormat option mentioned above).

benheller avatar Jan 20 '17 15:01 benheller

I obviously haven't been able to prioritise this, which I btw feel really bad about since I acknowledge the fact that folks out there are really bother by this bug.

Until I actually do find the time to work on this, I have just published v5.1.0 of plex-api with a backdoor allowing anyone to decide how responses should be parsed. I don't see this as an ideally fix, but it at least makes it a lot simpler for anyone to give it a shot implementing a parser that handles the new PMS v1.3 format.

EDIT:

Have a look at this commit to see how a rather simple response parser can be provided: https://github.com/phillipj/node-plex-api/commit/76539c8bca455607d3f2aced0b20d91a13d1b936.

I haven't upgrade my Plex Server yet, but looking at response.headers that is provided to a response parser, one might have a chance of identifying newer formats by looking at the x-plex-protocol header which I see as 1.0 on a pre v1.3 Plex Server. Would be nice if someone with an updated server can confirm that header has changed with the new format.

phillipj avatar Apr 16 '17 22:04 phillipj

The header hasn't changed still 1.0.

hyperlink avatar Apr 17 '17 13:04 hyperlink

😤 -- thanks for reporting though!

On Mon, 17 Apr 2017 at 15:55, Xiaoxin Lu [email protected] wrote:

The header hasn't changed still 1.0.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/phillipj/node-plex-api/issues/75#issuecomment-294489962, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLLE9RXJwkPB-m1JfzRLVPmA6DRyMqVks5rw29ugaJpZM4LDTBr .

phillipj avatar Apr 17 '17 13:04 phillipj

Probably have to sniff the response. Most of the code I've seen look for the existence of _children. My convert script also does this.

hyperlink avatar Apr 17 '17 14:04 hyperlink

@hyperlink are you willing to share how you convert the response?

I've started on handling this locally, but hit a challenge when seeing examples where the old format has several _children of the same type. That breaks the recommended approach I've gotten to transform new prettified format -> legacy format (see https://github.com/phillipj/node-plex-api/issues/75#issuecomment-273995254) because that doesn't allow an array of children, like the legacy format did.

Looking at current test fixture of /library/sections (shortened for brevity):

{
  "_elementType": "MediaContainer",
  "_children": [
    {
      "_elementType": "Directory",
      "_children": [
        {
          "_elementType": "Location",
          "id": 1,
          "path": "/Users/foobar/Movies"
        }
      ]
    },
    {
      "_elementType": "Directory",
      "_children": [
        {
          "_elementType": "Location",
          "id": 2,
          "path": "/Users/foobar/Downloads/Photos"
        }
      ]
    }
  ]
}

phillipj avatar May 06 '17 21:05 phillipj

I'm not sure if I understand you well, but it looks like you want to convert the new format to the legacy format which sounds and feels like a step back to me. Please also provide a way to get the new format - otherwise we're limiting ourselves for legacy reasons to a format with awful nesting that is difficult to use.

hansmbakker avatar May 07 '17 07:05 hansmbakker

@wind-rider appreciate you reaching out with your concerns! You are absolutely correct about what I'm thinking about doing.

I absolutely agree that we would be best off defaulting to the new pretty v1.3 format in the long run. That said, to me it's also very important to support existing users of this plex-api package. An important aspect of that is to not force users to upgrade to a new major version of plex-api with a completely new format, or else being stuck with a borked project.

Therefore I'm considering getting a patch version out there that handles both formats by converting the new pretty format to the legacy format. That would allow existing users of plex-api to upgrade their PMS to v1.3 and above without a sweat, inline with @hyperlink previous https://github.com/phillipj/node-plex-api/issues/75#issuecomment-274092594.

When that's done, I'm all for publishing a new major version of plex-api that only supports the v1.3 format.

Also worth mentioning this is early stage testing for me, nothing is written in stone :) And with the release of plex-api v5.1.0 we have configurable response parser when creating a PlexAPI instance, which possibly can make this transition a little easier for both camps; those who wants the legacy format or the ones that wants the pretty v1.3 format.

Any thoughts on those ramblings are more than appreciated.

phillipj avatar May 07 '17 10:05 phillipj

Ok, that's clear. And I just read this:

The default implementation either parses the JSON in the response from the Plex Server, converts XML to a JavaScript object or returns the response as is, depending on the response Content-Type header.

Probably this won't work for legacy applications expecting legacy object structures coming out of this, but I think it should work if you would start with a new application now, wouldn't it?

One thing that would still be nice is to have common API calls in this library, because as I understand it programmers have to format their own urls / queries to put in this library's functions.

hansmbakker avatar May 07 '17 12:05 hansmbakker

@phillipj here's my convert script.

hyperlink avatar May 08 '17 16:05 hyperlink

Certainly not a permanent fix that will cover all scenarios, but with #92 merged and pushed as v5.2.3 there's some support for PMS v1.3 atleast. Still very much open for further improvements and tweaks to make sure working with PMS v1.3 gets even better.

phillipj avatar Aug 20 '19 07:08 phillipj