get_resource: does not work for filedata

[mheden]

It seems like get_resource with the argument get_file=True will fail since the Joplin API is sending a binary blob in this case and not json data.

Excerpt:

Traceback (most recent call last):
  File "C:\devel\joplin\.venv\lib\site-packages\requests\models.py", line 910, in json
    return complexjson.loads(self.text, **kwargs)
  File "C:\Python39\lib\json\__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "C:\Python39\lib\json\decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "C:\Python39\lib\json\decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
...
    pprint(api.get_resource(id_, get_file=True))
  File "C:\devel\joplin\.venv\lib\site-packages\joppy\api.py", line 265, in get_resource
    response: JoplinItem = self.get(f"/resources/{id_}/file", query=query).json()
  File "C:\devel\joplin\.venv\lib\site-packages\requests\models.py", line 917, in json
    raise RequestsJSONDecodeError(e.msg, e.doc, e.pos)
requests.exceptions.JSONDecodeError: [Errno Expecting value] �PNG
→
IHDR☻g♠��D� IDATx���l[������N+�
...

version info:
joppy: 0.0.6
Joplin 2.7.15 (prod, win32)
Sync Version: 3
Profile Version: 41
Revision: 8352e23

[mheden]

Maybe add a data attribute to JoplinItem and do something like this:

    def get_resource(
        self, id_: str, get_file: bool = False, **query: JoplinTypes
    ) -> JoplinItem:
        """Get the resource with the given ID."""
        if get_file:
            response: JoplinItem = {'data': self.get(f"/resources/{id_}/file").content}
        else:
            response: JoplinItem = self.get(f"/resources/{id_}", query=query).json()
        return response

[marph91]

Hi, thanks for the bug report!

I wasn't aware of this flag. It seems to be not documented at https://joplinapp.org/api/references/rest_api/ and I can't find any other reference. Do you have maybe any reference of this flag? However, it seems to be available in the API and joppy shouldn't throw an exception.

Edit: I see I introduced the flag myself. It's not part of the joplin API. Your suggestion looks reasonable. I will have time to take a closer look tomorrow.

[mheden]

Thanks!

Maybe it's a bit weird to force something into JSON when there is just a binary blob (and also since query doesn't work with the /file endpoint) so another suggestion is that the API function is split into two instead:

    def get_resource_metadata(
        self, id_: str, **query: JoplinTypes
    ) -> JoplinItem:
        """Get the resource with the given ID."""
        response: JoplinItem = self.get(f"/resources/{id_}", query=query).json()
        return response

    def get_resource_filedata(
        self, id_: str
    ) -> bytes:
        """Get the resource file data with the given ID."""
        return self.get(f"/resources/{id_}/file").content

[marph91]

I agree. Explicitly defining two separate functions is better, since the return type is different and we can omit the query argument.

Do you want to create a PR? If so, a test case similar to https://github.com/marph91/joppy/blob/master/test/test_api.py#L526-L532 would be nice. If not, I can do the changes, since I have the project checked out and ready :)

[mheden]

It's probably better if you do it then :-)