YARD visibility property support

[daareiza]

Sometimes you want to have your API endpoints properly documented but some endpoints should remain private for internal usage, this visibility property support adds that feature to swagger_yard

[nicksieger]

@daareiza Thanks for the PR! I like the idea overall and think it's useful. Is @visibility a standard YARD tag? I didn't see it mentioned in the documentation. What do you think about using the @api tag instead?

[daareiza]

probably you are right, I think I chose @visibility thinking about RDOC @!visibility tag, not YARD, probably it is an easy change, I will try

[daareiza]

it was not that easy after all :/, for some reason the Yard parser doesn't adds the tags the object :|, but it works with visibility because it is an attribute of the object

[nicksieger]

That's ok, thanks for trying! We can go with your PR as-is. Going to add a note about visibility in the readme, and I'll release a new version. Sorry for taking so long to get back to you!

[nicksieger]

Hmm, thinking about this some more, I'm wondering about the default value for include_private. Seems like a more sensible default would be include_private = false, no? When swagger_yard-rails is used, the route inspector already excludes private methods by default. I wonder if include private should even be configurable. We could change the guard to just return self if yard_object.visibility != :public. What do you think about that?

[nicksieger]

Basically these changes:

diff --git a/lib/swagger_yard/api_group.rb b/lib/swagger_yard/api_group.rb
index 66eb8f5..6d62bb9 100644
--- a/lib/swagger_yard/api_group.rb
+++ b/lib/swagger_yard/api_group.rb
@@ -49,7 +49,7 @@ module SwaggerYard
     end

     def add_yard_object(yard_object)
-      return self if yard_object.visibility == :private && !SwaggerYard.config.include_private
+      return self if yard_object.visibility != :public

       case yard_object.type
       when :class # controller
diff --git a/lib/swagger_yard/configuration.rb b/lib/swagger_yard/configuration.rb
index 0d653b3..255ac07 100644
--- a/lib/swagger_yard/configuration.rb
+++ b/lib/swagger_yard/configuration.rb
@@ -6,7 +6,6 @@ module SwaggerYard
     attr_accessor :controller_path, :model_path
     attr_accessor :path_discovery_function
     attr_accessor :security_definitions
-    attr_accessor :include_private

     # openapi-compatible names
     alias openapi_version swagger_version
@@ -21,7 +20,6 @@ module SwaggerYard
       @description = "Configure description with SwaggerYard.config.description"
       @security_definitions = {}
       @external_schema = {}
-      @include_private = true
     end

     def external_schema(mappings = nil)
diff --git a/spec/lib/swagger_yard/api_group_spec.rb b/spec/lib/swagger_yard/api_group_spec.rb
index d0fab30..fd24c4b 100644
--- a/spec/lib/swagger_yard/api_group_spec.rb
+++ b/spec/lib/swagger_yard/api_group_spec.rb
@@ -52,30 +52,14 @@ describe SwaggerYard::ApiGroup do
       fixture_files.map { |f| SwaggerYard.yard_objects_from_file(f, :class) }.flatten
     end

-    context 'include private' do
-      before { SwaggerYard.config.include_private = true }
-      it 'should add private resources if include_private is true' do
-        groups = []
-        expect(yard_objects.count).to eq 4
-        yard_objects.each { |o| groups << SwaggerYard::ApiGroup.new.add_yard_object(o) }
-        resources = groups.map { |g| g.resource }.keep_if { |r| !r.nil? }
-        paths = groups.map { |g| g.path_items.keys }.flatten.keep_if { |p| !p.nil? }
-        expect(resources).to contain_exactly(*%w[Bonjour Farewell FullyPrivate SemiPrivate])
-        expect(paths).to contain_exactly(*%w[/bonjour /goodbye /fully_private /semi_private_public /semi_private])
-      end
-    end
-
-    context 'exclude private' do
-      before { SwaggerYard.config.include_private = false }
-      it 'should ignore private resources if include_private is false' do
-        groups = []
-        expect(yard_objects.count).to eq 4
-        yard_objects.each { |o| groups << SwaggerYard::ApiGroup.new.add_yard_object(o) }
-        resources = groups.map { |g| g.resource }.keep_if { |r| !r.nil? }
-        paths = groups.map { |g| g.path_items.keys }.flatten.keep_if { |p| !p.nil? }
-        expect(resources).to contain_exactly(*%w[Bonjour Farewell SemiPrivate])
-        expect(paths).to contain_exactly(*%w[/bonjour /goodbye /semi_private_public])
-      end
+    it 'should ignore non-public resources' do
+      groups = []
+      expect(yard_objects.count).to eq 4
+      yard_objects.each { |o| groups << SwaggerYard::ApiGroup.new.add_yard_object(o) }
+      resources = groups.map { |g| g.resource }.keep_if { |r| !r.nil? }
+      paths = groups.map { |g| g.path_items.keys }.flatten.keep_if { |p| !p.nil? }
+      expect(resources).to contain_exactly(*%w[Bonjour Farewell SemiPrivate])
+      expect(paths).to contain_exactly(*%w[/bonjour /goodbye /semi_private_public])
     end
   end
 end

[daareiza]

Hmm, thinking about this some more, I'm wondering about the default value for include_private. Seems like a more sensible default would be include_private = false, no? When swagger_yard-rails is used, the route inspector already excludes private methods by default. I wonder if include private should even be configurable. We could change the guard to just return self if yard_object.visibility != :public. What do you think about that?

I thought about that too, but basically I did it that way because of backward compatibility, right now it includes everything, so if people is using that tag for some other purpose, it will break things, although for sure false is a better default.

About it being configurable or not, it is just a matter of use cases, I would suggest leaving it configurable, since users can generate a swagger file for public access, and another one for internal usage on a private server that includes private endpoints (that's my use case actually)