Skip to content
This repository was archived by the owner on Jan 21, 2020. It is now read-only.

Support nested namespace in API name #19

Closed
wants to merge 1 commit into from
Closed

Support nested namespace in API name #19

wants to merge 1 commit into from

Conversation

kevinpapst
Copy link
Contributor

Hi,
I had some troubles accessing the auto generated documentation for REST APIs, which use nested namespaces. By default Apigility creates new APIs without any preceding namespace, like this:
"ApiName\V1\MyApi\MyApiResource"
But it supports APIs with deeper namespaces (like "Company\Rest\ApiName\V1\MyApi\MyApiResource") as well.
Even in the admin/settings UI where namespace separator are converted to a dot like this:
https://myApigility.dev/apigility/ui#/api/Company.Rest.ApiName/v1/rest-services?view=settings

You can see that conversion for example in \ZF\Apigility\Admin\Model\VersioningModelFactory::normalizeModuleName()

But: nested namespaces are not supported in the documentation module, generated URLs look like this:
https://myApigility.dev/apigility/documentation/Company\Rest\ApiName-v1
=> breaks routing and leads to a 404.

My questions are: Is that on purpose? Is there any reason for this behavior? And is there another way to achieve my goal (support APIs with deeper namespace nesting) without the attached code changes?

My pull request includes a quick way to fix that, but as I don't know the reason for the original behavior, I have no idea whether this might break anything else (including backward compatibility).

Any feedback is highly appreciated. And thanks for that great product!

@kevinpapst
Copy link
Contributor Author

Seems to be a duplicate of #16 but with a slightly different approach. This PR replaces backslashes with dots and vice versa, not relying on ApiFactory to replace dots with backslashes automagically (which I didn't think was possible at the time of writing).

@weierophinney weierophinney added this to the 1.2.0 milestone Jul 13, 2016
@weierophinney weierophinney self-assigned this Jul 13, 2016
weierophinney added a commit that referenced this pull request Jul 13, 2016
weierophinney added a commit that referenced this pull request Jul 13, 2016
weierophinney added a commit that referenced this pull request Jul 13, 2016
@weierophinney
Copy link
Member

Merged to develop for release with 1.2.0. Thanks, @kevinpapst !

@kevinpapst
Copy link
Contributor Author

Wow, so I can get rid of our internal fork. What a great message. Thanks 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants