Restructured the metadata, need to redo the formatter specs

lib/rspec/swagger/formatter.rb

@@ -19,19 +19,15 @@ module RSpec
       def example_finished(notification)
         return unless notification.example.metadata[:swagger_object] == :response
 
-        data = notification.example.metadata[:swagger_data]
-        document = document_for(data[:document])
-        path_item = path_item_for(document, data[:path])
-        # TODO output path_item's parameters
-        operation = operation_for(path_item, data[:operation])
-        # TODO output operation's parameters
-        response = response_for(operation, data[:status_code])
-        response[:description] = data[:response_description] if data[:response_description]
-        response[:examples] = prepare_example(data[:example]) if data[:example]
+        notification.example.metadata.each do |k, v|
+          puts "#{k}\t#{v}" if k.to_s.starts_with?("swagger")
+        end
 
-        # notification.example.metadata.each do |k, v|
-        #   puts "#{k}\t#{v}" if k.to_s.starts_with?("swagger")
-        # end
+        metadata  = notification.example.metadata
+        document  = document_for(metadata[:swagger_document])
+        path_item = path_item_for(document, metadata[:swagger_path_item])
+        operation = operation_for(path_item, metadata[:swagger_operation])
+        response  = response_for(operation, metadata[:swagger_response])
       end
 
       def close(_notification)
@@ -50,30 +46,56 @@ module RSpec
         end
       end
 
-      def path_item_for(document, path_name)
+      def path_item_for(document, swagger_path_item)
+        name = swagger_path_item[:path]
+
         document[:paths] ||= {}
-        document[:paths][path_name] ||= {}
+        document[:paths][name] ||= {}
+        if swagger_path_item[:parameters]
+          document[:paths][name][:parameters] = prepare_parameters(swagger_path_item[:parameters])
+        end
+        document[:paths][name]
       end
 
-      def operation_for(path, operation_name)
-        path[operation_name] ||= {responses: {}}
+      def operation_for(path, swagger_operation)
+        method = swagger_operation[:method]
+
+        path[method] ||= {responses: {}}
+        path[method].tap do |operation|
+          if swagger_operation[:parameters]
+            operation[:parameters] = prepare_parameters(swagger_operation[:parameters])
+          end
+          operation.merge!(swagger_operation.slice(
+            :summary, :description, :externalDocs, :operationId,
+            :consumes, :produces, :schemes, :deprecated, :security
+          ))
         end
+      end
+
+      def response_for(operation, swagger_response)
+        status = swagger_response[:status_code]
 
-      def response_for(operation, status_code)
-        operation[:responses][status_code] ||= {}
+        operation[:responses][status] ||= {}
+        operation[:responses][status].tap do |response|
+          if swagger_response[:examples]
+            response[:examples] = prepare_examples(swagger_response[:examples])
+          end
+          response.merge!(swagger_response.slice(:description, :schema, :headers))
+        end
       end
 
+      def prepare_parameters(params)
+        params.values
+      end
 
-      def prepare_example(example)
-        mime_type = example[:content_type]
-        body = example[:body]
-        if mime_type == 'application/json'
+      def prepare_examples(examples)
+        if examples["application/json"].present?
           begin
-            body = JSON.parse(body)
+            examples["application/json"] = JSON.parse(examples["application/json"])
           rescue JSON::ParserError => e
           end
         end
-        { mime_type => body }
+        examples
       end
     end
   end

lib/rspec/swagger/helpers.rb

@@ -46,23 +46,23 @@ module RSpec
           #TODO template might be a $ref
           meta = {
             swagger_object: :path_item,
-            swagger_data: {
-              document: attributes[:swagger_document] || RSpec.configuration.swagger_docs.keys.first,
-              path: template
-            }
+            swagger_document: attributes[:swagger_document] || RSpec.configuration.swagger_docs.keys.first,
+            swagger_path_item: {path: template}
           }
           describe(template, meta, &block)
         end
       end
 
       module PathItem
-        def operation verb, desc, &block
+        def operation verb, attributes = {}, &block
+          attributes.symbolize_keys!
+
           # TODO, check verbs against a whitelist
 
           verb = verb.to_s.downcase
           meta = {
             swagger_object: :operation,
-            swagger_data: metadata[:swagger_data].merge(operation: verb.to_sym, operation_description: desc)
+            swagger_operation: attributes.merge(method: verb.to_sym).reject{ |v| v.nil? }
           }
           describe(verb.to_s, meta, &block)
         end
@@ -72,10 +72,26 @@ module RSpec
         def parameter name, attributes = {}
           attributes.symbolize_keys!
 
-          raise ArgumentError, "Missing 'in' parameter" unless attributes[:in]
+          raise ArgumentError, "Parameter is missing required 'in' value." unless attributes[:in]
           locations = [:query, :header, :path, :formData, :body]
+
           unless locations.include? attributes[:in]
-            raise ArgumentError, "Invalid 'in' parameter, must be one of #{locations}"
+            raise ArgumentError, "Parameter has an invalid 'in' value. Try: #{locations}."
+          end
+
+          if attributes[:in] == :body
+            unless attributes[:schema].present?
+              raise ArgumentError, "Parameter is missing required 'schema' value."
+            end
+          else
+            unless attributes[:type].present?
+              raise ArgumentError, "Parameter is missing required 'type' value."
+            end
+
+            types = %i(string number integer boolean array file)
+            unless types.include?(attributes[:type])
+              raise ArgumentError, "Parameter has an invalid 'type' value. Try: #{types}."
+            end
           end
 
           # Path attributes are always required
@@ -85,9 +101,12 @@ module RSpec
           #   param = { '$ref' => name.delete(:ref) || name.delete('ref') }
           # end
 
-          params = metadata[:swagger_data][:params] ||= {}
+          object_key = "swagger_#{metadata[:swagger_object]}".to_sym
+          object_data = metadata[object_key] ||= {}
 
+          params = object_data[:parameters] ||= {}
           param = { name: name.to_s }.merge(attributes)
+
           # Params should be unique based on the 'name' and 'in' values.
           param_key = "#{param[:in]}&#{param[:name]}"
           params[param_key] = param
@@ -96,29 +115,28 @@ module RSpec
 
       module Operation
         def consumes *mime_types
-          metadata[:swagger_data][:consumes] = mime_types
+          metadata[:swagger_operation][:consumes] = mime_types
         end
 
         def produces *mime_types
-          metadata[:swagger_data][:produces] = mime_types
+          metadata[:swagger_operation][:produces] = mime_types
         end
 
-        def response status_code, desc, params = {}, headers = {}, &block
+        def response status_code, desc, params = {}, &block
           unless status_code == :default || (100..599).cover?(status_code)
             raise ArgumentError, "status_code must be an integer 100 to 599, or :default"
           end
           meta = {
             swagger_object: :status_code,
-            swagger_data: metadata[:swagger_data].merge(status_code: status_code, response_description: desc)
+            swagger_response: {status_code: status_code, description: desc}
           }
           describe(status_code, meta) do
             self.module_exec(&block) if block_given?
 
             before do |example|
-              swagger_data = example.metadata[:swagger_data]
-
-              path = resolve_path(swagger_data[:path], self)
-              headers = resolve_headers(swagger_data)
+              method = example.metadata[:swagger_operation][:method]
+              path = resolve_path(example.metadata[:swagger_path_item][:path], self)
+              headers = resolve_headers(example.metadata)
 
               # Run the request
               args = if ::Rails::VERSION::MAJOR >= 5
@@ -126,13 +144,11 @@ module RSpec
               else
                 [path, params, headers]
               end
-              self.send(swagger_data[:operation], *args)
+              self.send(method, *args)
 
               if example.metadata[:capture_example]
-                swagger_data[:example] = {
-                  body: response.body,
-                  content_type: response.content_type.to_s
-                }
+                examples = example.metadata[:swagger_response][:examples] ||= {}
+                examples[response.content_type.to_s] = response.body
               end
             end
 
@@ -150,38 +166,35 @@ module RSpec
       end
 
       module Resolver
-        # TODO Really hate that we have to keep passing swagger_data around
-        # like this
-        def load_document swagger_data
-          ::RSpec.configuration.swagger_docs[swagger_data[:document]]
-        end
-
-        def resolve_prodces swagger_data
-          swagger_data[:produces] #|| load_document(swagger_data)[:produces]
+        def resolve_prodces metadata
+          metadata[:swagger_operation][:produces]
         end
 
-        def resolve_consumes swagger_data
-          swagger_data[:consumes] #|| load_document(swagger_data)[:consumes]
+        def resolve_consumes metadata
+          metadata[:swagger_operation][:consumes]
         end
 
-        def resolve_headers swagger_data
+        def resolve_headers metadata
           headers = {}
           # Match the names that Rails uses internally
-          if produces = resolve_prodces(swagger_data)
+          if produces = resolve_prodces(metadata)
             headers['HTTP_ACCEPT'] = produces.join(';')
           end
-          if consumes = resolve_consumes(swagger_data)
+          if consumes = resolve_consumes(metadata)
             headers['CONTENT_TYPE'] = consumes.first
           end
           headers
         end
 
-        def resolve_params swagger_data, group_instance
-          params = swagger_data[:params].values
+        def resolve_params metadata, group_instance
+          path_item = metadata[:swagger_path_item] || {}
+          operation = metadata[:swagger_operation] || {}
+          params = path_item.fetch(:parameters, {}).merge(operation.fetch(:parameters, {}))
+
           # TODO resolve $refs
           # TODO there should only be one body param
           # TODO there should not be both body and formData params
-          params.map do |p|
+          params.values.map do |p|
             p.slice(:name, :in).merge(value: group_instance.send(p[:name]))
           end
         end

spec/requests/request_spec.rb

@@ -2,18 +2,18 @@ require 'swagger_helper'
 
 RSpec.describe "Requestsing", type: :request do
   path '/posts' do
-    operation "GET", "fetch list" do
+    operation "GET", summary:"fetch list" do
       produces 'application/json'
 
       # params
       response(200, "successful", {})
     end
 
-    operation "POST", "create" do
+    operation "POST",summary: "create" do
       produces 'application/json'
       consumes 'application/json'
 
-      parameter "body", in: :body
+      parameter "body", in: :body, schema: { foo: :bar}
       let(:body) { { post: { title: 'asdf', body: "blah" } } }
 
 # TODO: it should pull the body from the params
@@ -29,14 +29,14 @@ RSpec.describe "Requestsing", type: :request do
   end
 
   path '/posts/{post_id}' do
-    parameter "post_id", {in: :path}
+    parameter "post_id", {in: :path, type: :integer}
     let(:post_id) { 1 }
 
-    operation "GET", "fetch item" do
+    operation "GET", summary: "fetch item" do
       produces 'application/json'
 
       before { Post.new.save }
-      parameter "op-param", {in: :query}
+      parameter "op-param", {in: :query, type: :string}
       response(200, "success", {}) do
         capture_example
       end

spec/rspec/swagger/formatter_spec.rb

-require "spec_helper"
+require 'swagger_helper'
 
 RSpec.describe RSpec::Swagger::Formatter do
   let(:output) { StringIO.new }

spec/rspec/swagger/helpers_spec.rb

-require "spec_helper"
+require 'swagger_helper'
 
 RSpec.describe RSpec::Swagger::Helpers::Paths do
   let(:klass) do
@@ -18,10 +18,8 @@ RSpec.describe RSpec::Swagger::Helpers::Paths do
   it "defaults to the first swagger document if not specified" do
     expect(subject).to receive(:describe).with("/ping", {
       swagger_object: :path_item,
-      swagger_data: {
-        document: RSpec.configuration.swagger_docs.keys.first,
-        path: '/ping'
-      }
+      swagger_document: RSpec.configuration.swagger_docs.keys.first,
+      swagger_path_item: {path: '/ping'}
     })
 
     subject.path('/ping')
@@ -30,15 +28,54 @@ RSpec.describe RSpec::Swagger::Helpers::Paths do
   it "accepts specified swagger document name" do
     expect(subject).to receive(:describe).with("/ping", {
       swagger_object: :path_item,
-      swagger_data: {
-        document: 'hello_swagger.json',
-        path: '/ping'
-      }
+      swagger_document: 'hello_swagger.json',
+      swagger_path_item: {path: '/ping'}
     })
 
     subject.path('/ping', swagger_document: 'hello_swagger.json')
   end
+end
+
+RSpec.describe RSpec::Swagger::Helpers::PathItem do
+  let(:klass) do
+    Class.new do
+      include RSpec::Swagger::Helpers::PathItem
+      attr_accessor :metadata
+      def describe *args ; end
+    end
+  end
+  subject { klass.new }
+
+  describe "#operation" do
+
+    it "requires only an HTTP verb" do
+      expect(subject).to receive(:describe).with('get', {
+        swagger_object: :operation,
+        swagger_operation: {method: :get}
+      })
+
+      subject.operation('GET')
+    end
+
+    it "accepts other options" do
+      expect(subject).to receive(:describe).with('head', {
+        swagger_object: :operation,
+        swagger_operation: {
+          method: :head, tags: ['pet'], summary: 'Updates',
+          description: 'Updates a pet in the store with form data',
+          operationId: 'updatePetWithForm'
+        }
+      })
+
+      subject.operation('head',
+        tags: ['pet'],
+        summary: 'Updates',
+        description: 'Updates a pet in the store with form data',
+        operationId: 'updatePetWithForm'
+      )
+    end
 
+  end
 end
 
 RSpec.describe RSpec::Swagger::Helpers::Parameters do
@@ -51,31 +88,50 @@ RSpec.describe RSpec::Swagger::Helpers::Parameters do
   subject { klass.new }
 
   describe "#parameter" do
-    before { subject.metadata = {swagger_object: :path_item, swagger_data: {}} }
+    before { subject.metadata = {swagger_object: :path_item} }
 
     it "requires 'in' parameter" do
       expect{ subject.parameter("name", foo: :bar) }.to raise_exception(ArgumentError)
     end
 
     it "validates 'in' parameter" do
-      expect{ subject.parameter("name", in: :form_data) }.to raise_exception(ArgumentError)
-      expect{ subject.parameter("name", in: "formData") }.to raise_exception(ArgumentError)
-      expect{ subject.parameter("name", in: :formData) }.not_to raise_exception
+      expect{ subject.parameter("name", in: :form_data, type: :string) }.to raise_exception(ArgumentError)
+      expect{ subject.parameter("name", in: "formData", type: :string) }.to raise_exception(ArgumentError)
+      expect{ subject.parameter("name", in: :formData, type: :string) }.not_to raise_exception
+    end
+
+    it "requies a schema for body params" do
+      expect{ subject.parameter(:name, in: :body) }.to raise_exception(ArgumentError)
+      expect{ subject.parameter(:name, in: :body, schema: {ref: '#/definitions/foo'}) }.not_to raise_exception
+    end
+
+    it "requires a type for non-body params" do
+      expect{ subject.parameter(:name, in: :path) }.to raise_exception(ArgumentError)
+      expect{ subject.parameter(:name, in: :path, type: :number) }.not_to raise_exception
+    end
+
+    it "validates types" do
+      %i(string number integer boolean array file).each do |type|
+        expect{ subject.parameter(:name, in: :path, type: type) }.not_to raise_exception
+      end
+      [100, :pickles, "stuff"].each do |type|
+        expect{ subject.parameter(:name, in: :path, type: type) }.to raise_exception(ArgumentError)
+      end
     end
 
     it "marks path parameters as required" do
-      subject.parameter("name", in: :path)
+      subject.parameter("name", in: :path, type: :boolean)
 
-      expect(subject.metadata[:swagger_data][:params].values.first).to include(required: true)
+      expect(subject.metadata[:swagger_path_item][:parameters].values.first).to include(required: true)
     end
 
     it "keeps parameters unique by name and location" do
-      subject.parameter('foo', in: :path)
-      subject.parameter('foo', in: :path)
-      subject.parameter('bar', in: :query)
-      subject.parameter('baz', in: :query)
+      subject.parameter('foo', in: :path, type: :integer)
+      subject.parameter('foo', in: :path, type: :integer)
+      subject.parameter('bar', in: :query, type: :integer)
+      subject.parameter('baz', in: :query, type: :integer)
 
-      expect(subject.metadata[:swagger_data][:params].length).to eq 3
+      expect(subject.metadata[:swagger_path_item][:parameters].length).to eq 3
     end
   end
 end
@@ -91,7 +147,7 @@ RSpec.describe RSpec::Swagger::Helpers::Operation do
   subject { klass.new }
 
   describe "#response" do
-    before { subject.metadata = {swagger_object: :operation, swagger_data: {}} }
+    before { subject.metadata = {swagger_object: :operation} }
 
     it "requires code be an integer 100...600 or :default" do
       expect{ subject.response 1, "description" }.to raise_exception(ArgumentError)
@@ -112,7 +168,7 @@ RSpec.describe RSpec::Swagger::Helpers::Resolver do
   # Tthis helper is an include rather than an extend we can get it pulled into
   # the test just by matching the filter metadata.
   describe("#resolve_params", :swagger_object) do
-    let(:swagger_data) { { params: params } }
+    let(:metadata) { {swagger_operation: {parameters: params}} }
 
     describe "with a missing value" do
       let(:params) { {"path&post_id" => {name: "post_id", in: :path}} }
@@ -120,7 +176,7 @@ RSpec.describe RSpec::Swagger::Helpers::Resolver do
       # TODO best thing would be to lazily evaulate the params so we'd only
       # hit this if something was trying to use it.
       it "raises an error" do
-        expect{resolve_params(swagger_data, self)}.to raise_exception(NoMethodError)
+        expect{resolve_params(metadata, self)}.to raise_exception(NoMethodError)
       end
     end
 
@@ -129,7 +185,7 @@ RSpec.describe RSpec::Swagger::Helpers::Resolver do
       let(:post_id) { 123 }
 
       it "returns it" do
-        expect(resolve_params(swagger_data, self)).to eq([{name: "post_id", in: :path, value: 123}])
+        expect(resolve_params(metadata, self)).to eq([{name: "post_id", in: :path, value: 123}])
       end
     end
   end
@@ -159,18 +215,18 @@ RSpec.describe RSpec::Swagger::Helpers::Resolver do
 
   describe "#resolve_headers", :swagger_object do
     context "with consumes set" do
-      let(:swagger_data) { {consumes: ['application/json']} }
+      let(:metadata) { {swagger_operation: {consumes: ['application/json']}} }
 
       it "sets the Content-Type header" do
-        expect(resolve_headers(swagger_data)).to include('CONTENT_TYPE' => 'application/json')
+        expect(resolve_headers(metadata)).to include('CONTENT_TYPE' => 'application/json')
       end
     end
 
     context "with produces set" do
-      let(:swagger_data) { {produces: ['application/xml']} }
+      let(:metadata) { {swagger_operation: {produces: ['application/xml']}} }
 
       it "sets the Accepts header" do
-        expect(resolve_headers(swagger_data)).to include('HTTP_ACCEPT' => 'application/xml')
+        expect(resolve_headers(metadata)).to include('HTTP_ACCEPT' => 'application/xml')
       end
     end