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

Search for routes that contain swagger_doc #877

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

igor-drozdov
Copy link

If a client-defined route contains doc word, then the call to that URL will be unexpectedly performed

Let's filter the routes using swagger_doc instead

If a client-defined route contains `doc` word, then the call to that URL will be unexpectedly performed

Let's filter the routes using `swagger_doc` instead
@igor-drozdov
Copy link
Author

@LeFnord could you please have a look at this PR?

@anothermh
Copy link

@igor-drozdov Thank you for working on this. I am having a problem with this method at the moment as well and it's an edge case you may want to consider in your fix here.

The default value for mount_path is /swagger_doc but this can be overridden in the call to add_swagger_documentation:

:mount_path => '/swagger_doc',

I override this with a custom path and as a result I can't use the rake task to export documentation because /swagger_doc doesn't exist in my routes.

I'm overriding this method for now:

require 'rake/tasklib'

module GrapeSwagger
  module Rake
    class OapiTasks < ::Rake::TaskLib
      private

      def urls_for(api_class)
        api_class.routes
          .map(&:path)
          .select { |e| e.include?('/open_api') }
          .reject { |e| e.include?(':name') }
          .map { |e| format_path(e) }
          .map { |e| [e, ENV.fetch('resource', nil)].join('/').chomp('/') }
      end
    end
  end
end

If you're able to capture the mount_path and use that instead of swagger_doc that would resolve the bug I'm experiencing. I don't know if the value can be read, this has come up only in the last few hours.

@igor-drozdov
Copy link
Author

If you're able to capture the mount_path and use that instead of swagger_doc that would resolve the bug I'm experiencing. I don't know if the value can be read, this has come up only in the last few hours.

@anothermh thanks for your feedback!

Since the variable is configurable, then my fix is a breaking change because someone can have openapi_doc and it will stop working for them 🤔 We definitely need to capture the mount_path value but I'm also not sure that it can be done. We can path the mount path as a param to the rake task though but have this value doc by default for backward compatibility.

Let's see what @LeFnord thinks about it 👍

@LeFnord
Copy link
Member

LeFnord commented Feb 19, 2023

sorry @igor-drozdov

Since the variable is configurable, then my fix is a breaking change because someone can have openapi_doc and it will stop working for them 🤔 We definitely need to capture the mount_path value but I'm also not sure that it can be done. We can path the mount path as a param to the rake task though but have this value doc by default for backward compatibility.

yes that is exactly the case, mount_path is configurable, so your change can/will break
so try instead to fetch the mount path from the grape-swagger configuration

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