Support base64 encoding

[dagwieers]

With the recent change from Addon Signals to base64 encoding, we need to support both hex and base64 encoding.

This PR includes:

• support for base64 encoding when the original sender uses base64 encoding
• communicate with trakt using base64 encoding
• add unit tests for encoding/decoding hex and base64 encoding
• add testing using AddonSignals
• add testing for Python 3.5

This fixes #91

[mediaminister]

Replace hexlify with encode('hex')

This doesn't work with Python 3 (Kodi 19). Hexlify was used for a good reason.

2019-11-24 15:46:29.421 T:2600   ERROR: EXCEPTION Thrown (PythonToCppException) : -->Python callback/script returned the following error<--
                                             - NOTE: IGNORING THIS CAN LEAD TO MEMORY LEAKS!
                                            Error Type: <class 'AttributeError'>
                                            Error Contents: 'str' object has no attribute 'decode'
                                            Traceback (most recent call last):
                                              File "C:\Users\öl\AppData\Roaming\Kodi\addons\service.upnext\resources\lib\monitor.py", line 79, in onNotification
                                                data = utils.decode_data(data)
                                              File "C:\Users\öl\AppData\Roaming\Kodi\addons\service.upnext\resources\lib\utils.py", line 81, in decode_data
                                                json_data = payload.decode('hex')
                                            AttributeError: 'str' object has no attribute 'decode'
                                            -->End of Python script error report<--

[dagwieers]

We'll probably move to base64 instead?

[mediaminister]

I think that's a good option now that Addon Signals has moved to base64, service.upnext should also do this to maintain compatibility.
Checking if a string is base64 and using hex as a fallback is even better.

[dagwieers]

That won't work as we have to communicate back. We need to effectively track what encoding was used by which sender and reply accordingly.

[mediaminister]

Just base64 then.

[dagwieers]

I think the only way forward is to do it like this. Store the encoding-type per sender, so we support both base64 and hex encoding.

@im85288 Do you agree with this implementation?

We should work towards releasing this as soon as possible.

[dagwieers]

Next to this, I think we need fix the Wiki and provide sensible instructions for add-on writers on how to integrate with Up Next.

@im85288 Can you give @mediaminister and me access to the Wiki?

[im85288]

@dagwieers - I guess so as it seems to be the only way to have things generic. I have not been keeping up but if the problem comes down to only Addon Signals...maybe we should just go back to having this addon use it as it used to so that all of that sort of logic belongs in that addon rather than here?

[im85288]

Next to this, I think we need fix the Wiki and provide sensible instructions for add-on writers on how to integrate with Up Next.

@im85288 Can you give @mediaminister and me access to the Wiki?

Sure - Not sure I know how to do it though..but will take a look

[im85288]

@dagwieers You should now have access to the wiki, let me know if there are any issues

[dagwieers]

Ok, this PR is not ready until it has been reviewed and confirmed to work.

[dagwieers]

Tested with VRT NU add-on (hex) and Netflix (base64).
Please review and test.

[mediaminister]

Doesn't work yet.

2019-11-25 10:43:05.280 T:140024741861120  NOTICE: [service.upnext] PlaybackManager -> Error: no episode could be found to play next...exiting

[dagwieers]

@mediaminister Kodi version ? Python version ?

I am testing a newer version using the base64 class for Python 3, and have a separate function for encoding the data. And I added some unit tests (#95) for this to ensure it works as expected on all Python versions as we have had some complaints I cannot reproduce.

[mediaminister]

Kodi 18.5 Git:20191117-37f51f6e63 Platform: Linux x86 64-bit, Python 2 with VRT NU add-on master branch

[dagwieers]

@mediaminister Dat heb ik nochtans getest.

[mediaminister]

Watching two episodes works sometimes. But I can't watch three or more episodes in a row. (I don't use the "Watch Now" button.)

[dagwieers]

I am planning to write integration tests as well where we can perform successive calls and fake playing media. But there will always be some gray area when Kodi is involved. (e.g. Unicode and behaviour)

I did not test multiple successive calls myself yet at this stage. I first want to finish the unit tests in this PR to work fine. Hopefully tonight.

[dagwieers]

The unit tests work, despite Travis CI not working in GitHub today (?).
So I think this is ready for a review and more testing.

This time I tested successive continuation, and it works.
Lacking an environment, I haven't tested this on Python 3 and I wouldn't trust the unit tests.
Tested using VRTNU master branch (hex) and Netflix latest release using Addon Signals (base64).

[dagwieers]

but if the problem comes down to only Addon Signals.

That is the problem, since v0.0.9 the implementation was not using Addon Signals and other projects may have implemented support in their add-on (like we did with VRT NU). See also: #29

So I am afraid we have to support both hex and base64-encoded payloads for some time. I don't mind if we add a warning so over time people move to base64 encoded payloads, but we cannot just move to using Addon Signals at this time.

[anxdpanic]

Tested py3.7 hex and base64, and py3.5 hex, seems to be working fine

[im85288]

@dagwieers - Thanks for the explanation and getting this resolved. When your ready for me to merge and build a release candidate just let me know.

[dagwieers]

Let's get the approval of @mediaminister. His Kodi testing skills have been unmatched.

[mediaminister]

I tested with VRT NU add-on on Kodi 18/Python 2 and Kodi 19/Python3 and I can confirm it works.
I also tested with Netflix add-on on Kodi 18/Python 2

I didn't manage yet to get Netflix add-on working on Kodi 19/Python3 (Windows) because of a missing Cryptodome dependency.

So, we should wait for a confirmation of base64 working on Python 3 before merging.

[dagwieers]

Let me also add unit tests for Addon Signals.

[dagwieers]

Personally, I would merge this and release this as soon as possible. Since friday Up Next is broken for all add-ons using Addon Signals.

In the unlikely case we broken Python 3 using base64 we can fix that afterwards. The number of users running a Kodi v19 is limited (according to inputstreamhelper and Up Next download stats it is less than 1%) and typically users that chose to live on the edge.

[im85288]

@dagwieers Cool merging now, we can create new issues etc as needed in the future

[mediaminister]

I tested base64 with VRT NU add-on on Kodi 19 Python 3 and it works.

[dagwieers]

@mediaminister Great to hear! It is not the same as an Addon Signals test, and our encode/decode tests has its limitations too. But at least we know it does not have any obvious Python 3 issues. Thanks!

[dagwieers]

I do have to note that we are still trying hex-encoding first, and then fall back on base64. And the main reason for this is that an hex-encoded string can be decoded as base64 without any issues (it could be a valid base64 string too) whereas the hex-encoding has various type checks that do raise an exception.

We may want to improve our code by validating that the decoded content is proper JSON, and if not try base64. But it is probably better to get rid of hex-encoding in the not too distant future.

It would be good to know our customers better and have a complete list of add-ons using Up Next.