Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FR: custom cdn options #423

Closed
wants to merge 4 commits into from

Conversation

joshua-auchincloss
Copy link
Contributor

Hello,

Hoping to add the following features, rationale to enable support for e.g. private resource CDNs:

  • custom CDN options for redoc & swagger (defaulting to previously used constants)
  • options for default swagger cdn at OpenAPIHandler initialization

@codecov-commenter
Copy link

codecov-commenter commented Nov 12, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (310dc28) 97.70% compared to head (7bf3178) 97.69%.

Files Patch % Lines
blacksheep/server/openapi/ui.py 96.55% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #423      +/-   ##
==========================================
- Coverage   97.70%   97.69%   -0.01%     
==========================================
  Files          67       67              
  Lines        6277     6300      +23     
==========================================
+ Hits         6133     6155      +22     
- Misses        144      145       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Do not assume that URLs are for CDNs
- Remove the connection between OpenAPIHandler and class with files URLs
@RobertoPrevato
Copy link
Member

RobertoPrevato commented Nov 17, 2023

Hi @joshua-auchincloss
I apologize for replying so late. I considered merging your contribution and also made some changes to your code. But I finally changed mind.

With the current code API I implemented, all it takes to achieve what you want is to define a subclass of the provided classes and including the desired HTML, like in this example:

from dataclasses import dataclass
from pathlib import Path

from blacksheep import Application
from blacksheep.server.openapi.v3 import OpenAPIHandler
from blacksheep.server.openapi.ui import SwaggerUIProvider, UIOptions
from openapidocs.v3 import Info

app = Application()


class CustomUIProvider(SwaggerUIProvider):
    def get_openapi_ui_html(self, options: UIOptions) -> str:
        _template = Path("example.html").read_text()
        return _template.replace("{options.spec_url}", options.spec_url)


docs = OpenAPIHandler(info=Info(title="Example API", version="0.0.1"))
# Set the UI provider as desired:
docs.ui_providers = [CustomUIProvider()]
docs.bind_app(app)


@dataclass
class Foo:
    foo: str


@app.route("/foo")
async def get_foo() -> Foo:
    return Foo("Hello!")

example.html:

<!DOCTYPE html>
<html>
<head>
    <title>My desired title</title>
    <link rel="icon" href="/favicon.png"/>
    <link type="text/css" rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/5.10.0/swagger-ui.css">
</head>
<body>
    <div id="swagger-ui"></div>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/5.10.0/swagger-ui-bundle.min.js"></script>
    <script>
    const ui = SwaggerUIBundle({
        url: '{options.spec_url}',
        oauth2RedirectUrl: window.location.origin + '/docs/oauth2-redirect',
        dom_id: '#swagger-ui',
        presets: [
            SwaggerUIBundle.presets.apis,
            SwaggerUIBundle.SwaggerUIStandalonePreset
        ],
        layout: "BaseLayout",
        deepLinking: true,
        showExtensions: true,
        showCommonExtensions: true
    })
    </script>
</body>
</html>

It is just 6 lines of Python, and a simple HTML file:

+from blacksheep.server.openapi.ui import SwaggerUIProvider, UIOptions
from openapidocs.v3 import Info

app = Application()


+class CustomUIProvider(SwaggerUIProvider):
+    def get_openapi_ui_html(self, options: UIOptions) -> str:
+        _template = Path("example.html").read_text()
+        return _template.replace("{options.spec_url}", options.spec_url)


docs = OpenAPIHandler(info=Info(title="Example API", version="0.0.1"))
# Set the UI provider as desired:
+docs.ui_providers = [CustomUIProvider()]
docs.bind_app(app)

With the additional benefit that you can control everything of the HTML (favicon, metatags, etc.), not only the URL sources for the static files.
I don't think adding support for controlling the URL sources of static files is a better alternative.

@joshua-auchincloss
Copy link
Contributor Author

Hi @joshua-auchincloss

I apologize for replying so late. I considered merging your contribution and also made some changes to your code. But I finally changed mind.

With the current code API I implemented, all it takes to achieve what you want is to define a subclass of the provided classes and including the desired HTML, like in this example:

from dataclasses import dataclass

from pathlib import Path



from blacksheep import Application

from blacksheep.server.openapi.v3 import OpenAPIHandler

from blacksheep.server.openapi.ui import SwaggerUIProvider, UIOptions

from openapidocs.v3 import Info



app = Application()





class CustomUIProvider(SwaggerUIProvider):

    def get_openapi_ui_html(self, options: UIOptions) -> str:

        _template = Path("example.html").read_text()

        return _template.replace("{options.spec_url}", options.spec_url)





docs = OpenAPIHandler(info=Info(title="Example API", version="0.0.1"))

# Set the UI provider as desired:

docs.ui_providers = [CustomUIProvider()]

docs.bind_app(app)





@dataclass

class Foo:

    foo: str





@app.route("/foo")

async def get_foo() -> Foo:

    return Foo("Hello!")

example.html:

<!DOCTYPE html>

<html>

<head>

    <title>My desired title</title>

    <link rel="icon" href="/favicon.png"/>

    <link type="text/css" rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/5.10.0/swagger-ui.css">

</head>

<body>

    <div id="swagger-ui"></div>

    <script src="https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/5.10.0/swagger-ui-bundle.min.js"></script>

    <script>

    const ui = SwaggerUIBundle({

        url: '{options.spec_url}',

        oauth2RedirectUrl: window.location.origin + '/docs/oauth2-redirect',

        dom_id: '#swagger-ui',

        presets: [

            SwaggerUIBundle.presets.apis,

            SwaggerUIBundle.SwaggerUIStandalonePreset

        ],

        layout: "BaseLayout",

        deepLinking: true,

        showExtensions: true,

        showCommonExtensions: true

    })

    </script>

</body>

</html>

It is just 6 lines of Python, and a simple HTML file:

+from blacksheep.server.openapi.ui import SwaggerUIProvider, UIOptions

from openapidocs.v3 import Info



app = Application()





+class CustomUIProvider(SwaggerUIProvider):

+    def get_openapi_ui_html(self, options: UIOptions) -> str:

+        _template = Path("example.html").read_text()

+        return _template.replace("{options.spec_url}", options.spec_url)





docs = OpenAPIHandler(info=Info(title="Example API", version="0.0.1"))

# Set the UI provider as desired:

+docs.ui_providers = [CustomUIProvider()]

docs.bind_app(app)

With the additional benefit that you can control everything of the HTML (favicon, metatags, etc.), not only the URL sources for the static files.

I don't think adding support for controlling the URL sources of static files is a better alternative.

Hey thanks for the reply - I appreciate the review, although our opinions differ on this I guess. From an end user perspective, why would I make a custom html handler and custom HTML source files when it's already bundled with the server in package resources? I don't think it's (unreasonable) to have support for these use cases without an extra ~2 files in my source code

RobertoPrevato added a commit that referenced this pull request Dec 15, 2023
@RobertoPrevato
Copy link
Member

@joshua-auchincloss fair enough. I agree it's generally good to offer one more option to users. I recreated a PR from your branch.
Weeks ago I modified the naming, to not assume that a custom URL always goes to a CDN (static files could also be self-hosted in the app itself, or hosted in other services that are not a content delivery network).

docs = OpenAPIHandler(info=Info(title="Cats API", version="0.0.1"))
docs.ui_providers[0].ui_files = UIFilesOptions(
    js_url=get_test_files_url("swag-js"),
    css_url=get_test_files_url("swag-css"),
)
docs.ui_providers.append(
    ReDocUIProvider(
        ui_files=UIFilesOptions(
            js_url=get_test_files_url("redoc-js"),
            fonts_url=get_test_files_url("redoc-fonts"),
        )
    )
)

RobertoPrevato added a commit that referenced this pull request Dec 15, 2023
* Add support for custom Swagger UI files URL sources

* refactor: expose cdn opts

- added tests

* Fix test assertion

* Apply corrections before merge

- Do not assume that URLs are for CDNs
- Remove the connection between OpenAPIHandler and class with files URLs

* Restore contribution for PR #423

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update __init__.py

---------

Co-authored-by: benefactarch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants