Add publish_apidoc flag to SpecTree constructor #311

AndreasBBS · 2023-05-14T13:09:54Z

The publish_apidoc flag is added to the SpecTree constructor to allow users to disable publishing of the API documentation. If the flag is set to False, the register_route method will not be called, and the backend.app attribute will be set to the app attribute.

As discussed in #310

The `publish_apidoc` flag is added to the `SpecTree` constructor to allow users to disable publishing of the API documentation. If the flag is set to `False`, the `register_route` method will not be called, and the `backend.app` attribute will be set to the `app` attribute.

The app attribute was being set in the backend, but it was not being used. This commit removes the app attribute from the backend.

kemingy

Thanks for your contribution! 🎉

kemingy · 2023-05-15T07:44:35Z

spectree/spec.py

+        if self.publish_apidoc:
+            self.backend.register_route(self.app)


I think register_route is only used when we need to generate the OpenAPI doc.

You don't have to call this method if all you need is the validation. The code changes to the other two files look good to me.

Maybe we can add this to the README HowTo part? I think this flag is not necessary unless we support more features like "only expose part of the doc". WDYT?

It seems to me that other parts rely on the self.app inside spectree so I kept the register part of the code that sets that variable. For example the starlette_plugin.py uses it in find_routes. Tracking down the implications of removing the self.app was too complex for the fix I was trying to do so for safety I decided to keep it. If you are confident that it be the same as omitting the register(app) step then it's fine with me adding it to the README.

Thanks for your time. I have double-checked the code, all the self.app usages are related to generating the OpenAPI config. Without the spec.register(), we should be safe to use the validation feature without binding the OpenAPI endpoints.

Is it also the case for the plugins?

Falcon uses self.spectree.app here:

spectree/spectree/plugins/falcon_plugin.py

Lines 87 to 101 in 7d585b6

def find_routes(self):

routes = []

def find_node(node):

if node.resource and node.resource.__class__.__name__ not in DOC_CLASS:

routes.append(node)

for child in node.children:

find_node(child)

for route in self.spectree.app._router._roots:

find_node(route)

return routes

Starlette uses self.spectree.app here:

spectree/spectree/plugins/starlette_plugin.py

Lines 143 to 179 in 7d585b6

def find_routes(self):

routes = []

def parse_route(app, prefix=""):

# :class:`starlette.staticfiles.StaticFiles` doesn't have routes

if not app.routes:

return

for route in app.routes:

if route.path.startswith(f"/{self.config.path}"):

continue

func = route.app

if isinstance(func, partial):

try:

func = func.__wrapped__

except AttributeError:

pass

if inspect.isclass(func):

for method in METHODS:

if getattr(func, method, None):

routes.append(

Route(

f"{prefix}{route.path}",

{method.upper()},

getattr(func, method),

)

)

elif inspect.isfunction(func):

routes.append(

Route(f"{prefix}{route.path}", route.methods, route.endpoint)

)

else:

parse_route(route, prefix=f"{prefix}{route.path}")

parse_route(self.spectree.app)

return routes

Are this find_routes methods also only for generating the OpenAPI config? I refactored this two plugins because they were using self.app on themselves and the BasePlugin does not have a self.app. I felt it wasn't very consistent across plugins and the app can still be accessed from self.spectree.app. This might be a change to keep.

If you give me the confirmation these find_routes methods on the plugins are also strictly for OpenAPI generation I'll go ahead and remove the changes and add a new entry to README.md documenting that omitting the register(app) will cause the OpenAPI routes to not be generated but the validation will still be enforced.

Also let me know if you want me to keep the self.app -> self.spectree.app refactor.

P.S.: No need to thank me for my time. It's my pleasure to be able to contribute to the tools I use.

find_routes is only used to generate the OpenAPI config.

Yes, they should not access self.app since there is no such attribute.

You're right. The implementation here is a bit ugly. SpecTree and BasePlugin can access each other. Need to refactor this part.

I'm okay with the self.app -> self.spectree.app refactor. To make it consistent, you can also choose to delete the app arg from register_root() and use self.spectree.app directly.

mistake

AndreasBBS added 2 commits May 14, 2023 14:06

🐛 fix(spectree): remove unnecessary app attribute from backend

7d585b6

The app attribute was being set in the backend, but it was not being used. This commit removes the app attribute from the backend.

kemingy previously approved these changes May 15, 2023

View reviewed changes

kemingy linked an issue May 15, 2023 that may be closed by this pull request

feat: Disable publishing of /apidoc route #310

Open

kemingy reviewed May 15, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add publish_apidoc flag to SpecTree constructor #311

Add publish_apidoc flag to SpecTree constructor #311

AndreasBBS commented May 14, 2023

kemingy left a comment

kemingy May 15, 2023

kemingy May 23, 2023

AndreasBBS May 23, 2023

kemingy May 24, 2023

AndreasBBS May 24, 2023

kemingy May 25, 2023

		if self.publish_apidoc:
		self.backend.register_route(self.app)

	def find_routes(self):
	routes = []

	def find_node(node):
	if node.resource and node.resource.__class__.__name__ not in DOC_CLASS:
	routes.append(node)

	for child in node.children:
	find_node(child)

	for route in self.spectree.app._router._roots:
	find_node(route)

	return routes

	def find_routes(self):
	routes = []

	def parse_route(app, prefix=""):
	# :class:`starlette.staticfiles.StaticFiles` doesn't have routes
	if not app.routes:
	return
	for route in app.routes:
	if route.path.startswith(f"/{self.config.path}"):
	continue

	func = route.app
	if isinstance(func, partial):
	try:
	func = func.__wrapped__
	except AttributeError:
	pass

	if inspect.isclass(func):
	for method in METHODS:
	if getattr(func, method, None):
	routes.append(
	Route(
	f"{prefix}{route.path}",
	{method.upper()},
	getattr(func, method),
	)
	)
	elif inspect.isfunction(func):
	routes.append(
	Route(f"{prefix}{route.path}", route.methods, route.endpoint)
	)
	else:
	parse_route(route, prefix=f"{prefix}{route.path}")

	parse_route(self.spectree.app)
	return routes

Add publish_apidoc flag to SpecTree constructor #311

Are you sure you want to change the base?

Add publish_apidoc flag to SpecTree constructor #311

Conversation

AndreasBBS commented May 14, 2023

kemingy left a comment

Choose a reason for hiding this comment

kemingy May 15, 2023

Choose a reason for hiding this comment

kemingy May 23, 2023

Choose a reason for hiding this comment

AndreasBBS May 23, 2023

Choose a reason for hiding this comment

kemingy May 24, 2023

Choose a reason for hiding this comment

AndreasBBS May 24, 2023

Choose a reason for hiding this comment

kemingy May 25, 2023

Choose a reason for hiding this comment