LibrePCB / librepcb-api

Server-side implementation of the LibrePCB API
https://api.librepcb.org/api/v1/
GNU General Public License v3.0
0 stars 0 forks source link

Update json library objects #3

Open ubruhin opened 8 years ago

ubruhin commented 8 years ago

Current response for GET /api/v1/libraries/:

{
    "uuid": "d0ff7c29-ef27-468a-a90e-518083b3d8e4",
    "name": "Base",
    "url": "git://github.com/librepcb-libraries/base",
    "publisher": {
        "name": "LibrePCB"
    },
    "dependencies": [
        "5a27a376-f1d9-4fac-a695-ef64d7c7012c"
    ]
}

Issues:

  1. Missing multilingual support
  2. Missing attributes: format_version, version, author, deprecated, download_url, description, keywords, image
  3. Attribute publisher is no longer required

    New json notation (proposal):

{
    "uuid": "d0ff7c29-ef27-468a-a90e-518083b3d8e4",
    "format_version": "0.1",
    "version": "0.0.1",
    "author": "LibrePCB",
    "deprecated": false,
    "url": "https://github.com/LibrePCB-Libraries/LibrePCB_Base.lplib",
    "download_url": "https://github.com/LibrePCB-Libraries/LibrePCB_Base.lplib/archive/master.zip",
    "image": "????",
    "name": {
        "en_US": "LibrePCB Base"
    },
    "description": {
        "en_US": "Official LibrePCB Base Library"
    },
    "keywords": {
        "en_US": ""
    },
    "dependencies": [
        "5a27a376-f1d9-4fac-a695-ef64d7c7012c"
    ]
}

Open questions:

dbrgn commented 8 years ago

With "image" you mean an icon? Base64 encoded gzipped string could be an option. Not sure what the best way would be.

I strongly support a hash (SHA256 probably). Maybe also an optional GPG signature.

ubruhin commented 8 years ago

With "image" you mean an icon?

Yes, my idea is to place an (optional) image.svg in the root directory of libraries. This image should then be available through the LibrePCB API.

We may use a very simple solution for the first release of LibrePCB (resp. for the API version 1.0). And the simplest solution seems to be to embed the image.svg as a gzipped and base64 encoded string, as you already mentioned @dbrgn.

The biggest issue of that solution is that the size of the API response will increase massively, so it takes more time to download the list of available libraries.

Let me make some calculations:

This would require 100*10*0.4*1.33 = 532kB more space in the API response to deliver the images. With a relatively slow Internet connection of 1Mbit/s it would take about 4.3 seconds to download the list of available libraries!

That sounds not very acceptable to me, but maybe I have calculated with a too worse case ;)

In addition to the increased download time, the API response gets wasted with long base64 strings, so the response would no longer be easily human readable. After all, it may be better to only provide an URL in the API response, which points to the image.svg file (e.g. on GitHub). LibrePCB could then download all the images asynchronously after the list of repositories is received. This is harder to implement, but the user experience would be better due to more responsive GUI.

rnestler commented 8 years ago

Assuming that gzip compresses the file to 40% of its original file size

Since svg is basically text it should compress to ~25% of its size. Also HTTP supports compression directly so you don't need to encode it as base64.

dbrgn commented 8 years ago

Yes, but HTTPS should not be compressed: https://en.wikipedia.org/wiki/HTTP_compression#Security_implications

ubruhin commented 8 years ago
ubruhin commented 8 years ago

Maybe it would also be useful to provide the file size of the ZIP file, e.g. "filesize": 123456789 (in bytes). This way the user can see how large a library is, and the download progress can be calculated exactly (or is there another way to get the size of the ZIP file before downloading it?).

dbrgn commented 8 years ago

or is there another way to get the size of the ZIP file before downloading it?

Yes, a proper HTTP response will include the file size in the header.

I like the idea of including a "non-binding" file size though.

ubruhin commented 8 years ago

Yes, a proper HTTP response will include the file size in the header.

I assumed this too. But because this did not work in the new download manager, I did some research. Now I realize that GitHub does not send the Content-Length every time when downloading a ZIP file from there. I think GitHub creates the ZIP on demand and thus does not know the filesize instantly. When accessing the file a second time, we get the Content-Length, propably due to caching.

me@pc:~$ curl -I https://codeload.github.com/LibrePCB/LibrePCB/zip/master
HTTP/1.1 200 OK
Transfer-Encoding: chunked
Access-Control-Allow-Origin: https://render.githubusercontent.com
Content-Security-Policy: default-src 'none'; style-src 'unsafe-inline'
Strict-Transport-Security: max-age=31536000
Vary: Authorization,Accept-Encoding
X-Content-Type-Options: nosniff
X-Frame-Options: deny
X-XSS-Protection: 1; mode=block
ETag: "6ea3d06128d7f79df6e109daffa21c33e03305c6"
Content-Type: application/zip
Content-Disposition: attachment; filename=LibrePCB-master.zip
X-Geo-Block-List: 
Date: Sun, 18 Sep 2016 22:49:21 GMT
X-GitHub-Request-Id: B93E527B:092E:5D6534:57DF19F0

me@pc:~$ curl -I https://codeload.github.com/LibrePCB/LibrePCB/zip/master
HTTP/1.1 200 OK
Content-Length: 5628130
Access-Control-Allow-Origin: https://render.githubusercontent.com
Content-Security-Policy: default-src 'none'; style-src 'unsafe-inline'
Strict-Transport-Security: max-age=31536000
Vary: Authorization,Accept-Encoding
X-Content-Type-Options: nosniff
X-Frame-Options: deny
X-XSS-Protection: 1; mode=block
ETag: "6ea3d06128d7f79df6e109daffa21c33e03305c6"
Content-Type: application/zip
Content-Disposition: attachment; filename=LibrePCB-master.zip
X-Geo-Block-List: 
Date: Sun, 18 Sep 2016 22:49:30 GMT
X-GitHub-Request-Id: B93E527B:092E:5D6541:57DF19F9
dbrgn commented 8 years ago

Yes, you're right. Some servers don't provide it immediately. Probably Github is doing zip stream encoding.

But in that case, the correct thing is probably not to show a progress bar at all. Just show an activity indicator. That's also what download managers like chrome / firefox do. Or do you think we should show the untrusted file size?

In any case, don't assume that the file is finished when all bytes are received according to the size field in the database. Instead, always wait for the download to finish.

ubruhin commented 8 years ago

In my opinion we should use the untrusted file size as a fallback if Content-Length is missing. But only for the progress bar, not to decide when to stop downloading.

dbrgn commented 8 years ago

Ok, just be careful that we don't get security issues by assuming a different file size than the actual one :)

dbrgn commented 8 years ago

Do we want url validation for the url and download_url fields?

dbrgn commented 8 years ago

Attribute publisher is no longer required

Why? What's the difference to author?

dbrgn commented 8 years ago

Missing multilingual support

This one is a bit tricky. We'll probably have to discuss it in person, there are different approaches to storage and fallback that have different pros and cons.

ubruhin commented 8 years ago

Do we want url validation for the url and download_url fields?

What do you mean exactly? The field "url" is optional, so it can be an empty string. But "download_url" needs to be a valid url.

Attribute publisher is no longer require

Why? What's the difference to author?

Author is the person who created the library, the publisher is the person who publishes it ;) The author is defined in a library.xml file, the publisher not. I think the author is the only relevant information for end users, so it's enough to provide an author field, but no publisher field.

Missing multilingual support

This one is a bit tricky.

I would just use the same system as LibrePCB uses (LibrePCB already has multilingual support). As you see in my json example from above, multilingual support is implemented like a dictionary:

{
    "name": {
        "en_US": "LibrePCB Base"
    },
    "description": {
        "en_US": "Official LibrePCB Base Library"
    },
    "keywords": {
        "en_US": ""
    }
}

The locale "en_US" is always required (otherwise the library is considered as invalid), all other locales are optional.

Edit: Maybe arrays are better?

{
    "name": [
        {
            "locale": "en_US",
            "value": "LibrePCB Base"
        }
    ],
}
dbrgn commented 8 years ago

What do you mean exactly? The field "url" is optional, so it can be an empty string. But "download_url" needs to be a valid url.

The question was whether we want to reject invalid URLs, e.g. http:/missing-slash/hello.zip or git@github.com:/foo/bar.git.

I think the author is the only relevant information for end users, so it's enough to provide an author field, but no publisher field.

Ok, then I'll replace the publisher field with an author field.

Which of the new fields (format_version, version, author, deprecated, download_url, description, keywords, image) are required, and which ones are optional?

ubruhin commented 8 years ago

The question was whether we want to reject invalid URLs, e.g. http:/missing-slash/hello.zip or git@github.com:/foo/bar.git.

Yes, theoretically we should reject invalid URLs. But for the beginning this is not really required because we are the only guys who can add new URLs, so it's in our hands to insert valid URLs ;)

Ok, then I'll replace the publisher field with an author field.

:+1:

Which of the new fields (format_version, version, author, deprecated, download_url, description, keywords, image) are required, and which ones are optional?

All fields are mandatory, except "image" (which I would name "icon_url" as noticed above). The fields "name", "description", "keywords" require at least a value for the locale "en_US", other locales are optional. Descriptions and keywords can be empty, names must not be empty.

dbrgn commented 8 years ago

The fields "name", "description", "keywords" require at least a value for the locale "en_US", other locales are optional.

I think now I know how to implement translations. We can use PostgreSQL hstore fields: https://www.postgresql.org/docs/9.1/static/hstore.html