xbmc / metadata.themoviedb.org.python

Other
44 stars 41 forks source link

Change default ID to imdb #75

Closed finkleandeinhorn closed 3 years ago

finkleandeinhorn commented 3 years ago

During my migration to Matrix, it came to my attention that this new default scraper sets the ListItem.IMDBNumber InfoLabel inconsistently with the "old" XML-based scraper.

For example, after scanning (a subset of) my library in using the old scraper, I ran the following JSON-RPC command: {"jsonrpc":"2.0","id":17,"method":"VideoLibrary.GetMovies","params":{"properties":["uniqueid","imdbnumber"]}} And got this response back:

{
    "id": 17,
    "jsonrpc": "2.0",
    "result": {
        "limits": {
            "end": 3,
            "start": 0,
            "total": 3
        },
        "movies": [
           {
                "imdbnumber": "tt0088763",
                "label": "Back to the Future",
                "movieid": 1,
                "uniqueid": {
                    "imdb": "tt0088763",
                    "tmdb": "105"
                }
            },
           {
                "imdbnumber": "tt0101452",
                "label": "Bill & Ted's Bogus Journey",
                "movieid": 2,
                "uniqueid": {
                    "imdb": "tt0101452",
                    "tmdb": "1649"
                }
            },
           {
                "imdbnumber": "tt2584384",
                "label": "Jojo Rabbit",
                "movieid": 3,
                "uniqueid": {
                    "imdb": "tt2584384",
                    "tmdb": "515001"
                }
            }
        ]
    }
}

Which shows that the IMDb ID is correctly set, and matches the "uniqueid" key for IMDb as well. A log can be found here: https://pastebin.com/w8NvT4pN

Using the new scraper, I ran the same JSON-RPC command, and got this response instead:

{
    "id": 5,
    "jsonrpc": "2.0",
    "result": {
        "limits": {
            "end": 3,
            "start": 0,
            "total": 3
        },
        "movies": [
           {
                "imdbnumber": "105",
                "label": "Back to the Future",
                "movieid": 1,
                "uniqueid": {
                    "imdb": "tt0088763",
                    "tmdb": "105"
                }
            },
           {
                "imdbnumber": "1649",
                "label": "Bill & Ted's Bogus Journey",
                "movieid": 2,
                "uniqueid": {
                    "imdb": "tt0101452",
                    "tmdb": "1649"
                }
            },
           {
                "imdbnumber": "515001",
                "label": "Jojo Rabbit",
                "movieid": 3,
                "uniqueid": {
                    "imdb": "tt2584384",
                    "tmdb": "515001"
                }
            }
        ]
    }
}

So in the case of the new scraper, the TMDb ID is getting set as the IMDb ID, which isn't correct, and is inconsistent with the old scraper. A log of this can be found here: https://pastebin.com/dhwrbC8j

In this PR, I have fixed this issue by simply changing the "default" ID (as described in the docs to 'imdb' instead of 'tmdb'.

This results in consistent behavior with the old scraper, when making that same JSON-RPC call:

{
    "id": 7,
    "jsonrpc": "2.0",
    "result": {
        "limits": {
            "end": 3,
            "start": 0,
            "total": 3
        },
        "movies": [
           {
                "imdbnumber": "tt0088763",
                "label": "Back to the Future",
                "movieid": 1,
                "uniqueid": {
                    "imdb": "tt0088763",
                    "tmdb": "105"
                }
            },
           {
                "imdbnumber": "tt0101452",
                "label": "Bill & Ted's Bogus Journey",
                "movieid": 2,
                "uniqueid": {
                    "imdb": "tt0101452",
                    "tmdb": "1649"
                }
            },
           {
                "imdbnumber": "tt2584384",
                "label": "Jojo Rabbit",
                "movieid": 3,
                "uniqueid": {
                    "imdb": "tt2584384",
                    "tmdb": "515001"
                }
            }
        ]
    }
}

Finally, a log of that can be found here: https://pastebin.com/TDNy722f

Unless there is a particular reason that this change was made, it seems odd for this inconsistency to be maintained, so I hope that this PR will be considered valid.

rmrector commented 3 years ago

The "imdbnumber" field should no longer be used. Use the uniqueID instead.

I'm sure this can be merged for an upcoming version, but deprecated is deprecated, use the better option.

finkleandeinhorn commented 3 years ago

Is there any way for a skin to read that value? We actually found this inconsistency while working on a skin, which can easily parse InfoLabels, but I'm not aware of a way to get the uniqueIDs.

rmrector commented 3 years ago

Yes, there is an infolabel ListItem.UniqueID(imdb).

finkleandeinhorn commented 3 years ago

Well, I'll close this then. If ListItem.IMDbNumber is indeed, deprecated, we'll start to make the switch to using ListItem.UniqueID.