Grapify the projects snippet API

lib/api/project_snippets.rb

@@ -3,6 +3,9 @@ module API
   class ProjectSnippets < Grape::API
     before { authenticate! }
 
+    params do
+      requires :id, type: String, desc: 'The ID of a project'
+    end
     resource :projects do
       helpers do
         def handle_project_member_errors(errors)
@@ -18,111 +21,108 @@ module API
         end
       end
 
-      # Get a project snippets
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      # Example Request:
-      #   GET /projects/:id/snippets
+      desc 'Get all project snippets' do
+        success Entities::ProjectSnippet
+      end
       get ":id/snippets" do
         present paginate(snippets_for_current_user), with: Entities::ProjectSnippet
       end
 
-      # Get a project snippet
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   snippet_id (required) - The ID of a project snippet
-      # Example Request:
-      #   GET /projects/:id/snippets/:snippet_id
+      desc 'Get a single project snippet' do
+        success Entities::ProjectSnippet
+      end
+      params do
+        requires :snippet_id, type: Integer, desc: 'The ID of a project snippet'
+      end
       get ":id/snippets/:snippet_id" do
-        @snippet = snippets_for_current_user.find(params[:snippet_id])
-        present @snippet, with: Entities::ProjectSnippet
-      end
-
-      # Create a new project snippet
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   title (required) - The title of a snippet
-      #   file_name (required) - The name of a snippet file
-      #   code (required) - The content of a snippet
-      #   visibility_level (required) - The snippet's visibility
-      # Example Request:
-      #   POST /projects/:id/snippets
+        snippet = snippets_for_current_user.find(params[:snippet_id])
+        present snippet, with: Entities::ProjectSnippet
+      end
+
+      desc 'Create a new project snippet' do
+        success Entities::ProjectSnippet
+      end
+      params do
+        requires :title, type: String, desc: 'The title of the snippet'
+        requires :file_name, type: String, desc: 'The file name of the snippet'
+        requires :code, type: String, desc: 'The content of the snippet'
+        requires :visibility_level, type: Integer,
+                                    values: [Gitlab::VisibilityLevel::PRIVATE,
+                                             Gitlab::VisibilityLevel::INTERNAL,
+                                             Gitlab::VisibilityLevel::PUBLIC],
+                                    desc: 'The visibility level of the snippet'
+      end
       post ":id/snippets" do
         authorize! :create_project_snippet, user_project
-        required_attributes! [:title, :file_name, :code, :visibility_level]
+        snippet_params = declared_params
+        snippet_params[:content] = snippet_params.delete(:code)
 
-        attrs = attributes_for_keys [:title, :file_name, :visibility_level]
-        attrs[:content] = params[:code] if params[:code].present?
-        @snippet = CreateSnippetService.new(user_project, current_user,
-                                            attrs).execute
+        snippet = CreateSnippetService.new(user_project, current_user, snippet_params).execute
 
-        if @snippet.errors.any?
-          render_validation_error!(@snippet)
+        if snippet.persisted?
+          present snippet, with: Entities::ProjectSnippet
         else
-          present @snippet, with: Entities::ProjectSnippet
+          render_validation_error!(snippet)
         end
       end
 
-      # Update an existing project snippet
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   snippet_id (required) - The ID of a project snippet
-      #   title (optional) - The title of a snippet
-      #   file_name (optional) - The name of a snippet file
-      #   code (optional) - The content of a snippet
-      #   visibility_level (optional) - The snippet's visibility
-      # Example Request:
-      #   PUT /projects/:id/snippets/:snippet_id
+      desc 'Update an existing project snippet' do
+        success Entities::ProjectSnippet
+      end
+      params do
+        requires :snippet_id, type: Integer, desc: 'The ID of a project snippet'
+        optional :title, type: String, desc: 'The title of the snippet'
+        optional :file_name, type: String, desc: 'The file name of the snippet'
+        optional :code, type: String, desc: 'The content of the snippet'
+        optional :visibility_level, type: Integer,
+                                    values: [Gitlab::VisibilityLevel::PRIVATE,
+                                             Gitlab::VisibilityLevel::INTERNAL,
+                                             Gitlab::VisibilityLevel::PUBLIC],
+                                    desc: 'The visibility level of the snippet'
+        at_least_one_of :title, :file_name, :code, :visibility_level
+      end
       put ":id/snippets/:snippet_id" do
-        @snippet = snippets_for_current_user.find(params[:snippet_id])
-        authorize! :update_project_snippet, @snippet
+        snippet = snippets_for_current_user.find_by(id: params.delete(:snippet_id))
+        not_found!('Snippet') unless snippet
+
+        authorize! :update_project_snippet, snippet
+
+        snippet_params = declared_params(include_missing: false)
+        snippet_params[:content] = snippet_params.delete(:code) if snippet_params[:code].present?
 
-        attrs = attributes_for_keys [:title, :file_name, :visibility_level]
-        attrs[:content] = params[:code] if params[:code].present?
+        UpdateSnippetService.new(user_project, current_user, snippet,
+                                 snippet_params).execute
 
-        UpdateSnippetService.new(user_project, current_user, @snippet,
-                                 attrs).execute
-        if @snippet.errors.any?
-          render_validation_error!(@snippet)
+        if snippet.persisted?
+          present snippet, with: Entities::ProjectSnippet
         else
-          present @snippet, with: Entities::ProjectSnippet
+          render_validation_error!(snippet)
         end
       end
 
-      # Delete a project snippet
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   snippet_id (required) - The ID of a project snippet
-      # Example Request:
-      #   DELETE /projects/:id/snippets/:snippet_id
+      desc 'Delete a project snippet'
+      params do
+        requires :snippet_id, type: Integer, desc: 'The ID of a project snippet'
+      end
       delete ":id/snippets/:snippet_id" do
-        begin
-          @snippet = snippets_for_current_user.find(params[:snippet_id])
-          authorize! :update_project_snippet, @snippet
-          @snippet.destroy
-        rescue
-          not_found!('Snippet')
-        end
+        snippet = snippets_for_current_user.find_by(id: params[:snippet_id])
+        not_found!('Snippet') unless snippet
+
+        authorize! :admin_project_snippet, snippet
+        snippet.destroy
       end
 
-      # Get a raw project snippet
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   snippet_id (required) - The ID of a project snippet
-      # Example Request:
-      #   GET /projects/:id/snippets/:snippet_id/raw
+      desc 'Get a raw project snippet'
+      params do
+        requires :snippet_id, type: Integer, desc: 'The ID of a project snippet'
+      end
       get ":id/snippets/:snippet_id/raw" do
-        @snippet = snippets_for_current_user.find(params[:snippet_id])
+        snippet = snippets_for_current_user.find_by(id: params[:snippet_id])
+        not_found!('Snippet') unless snippet
 
         env['api.format'] = :txt
         content_type 'text/plain'
-        present @snippet.content
+        present snippet.content
       end
     end
   end

spec/requests/api/project_snippets_spec.rb

@@ -3,10 +3,12 @@ require 'rails_helper'
 describe API::API, api: true do
   include ApiHelpers
 
+  let(:project) { create(:empty_project, :public) }
+  let(:admin) { create(:admin) }
+
   describe 'GET /projects/:project_id/snippets/:id' do
     # TODO (rspeicher): Deprecated; remove in 9.0
     it 'always exposes expires_at as nil' do
-      admin = create(:admin)
       snippet = create(:project_snippet, author: admin)
 
       get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}", admin)
@@ -17,9 +19,9 @@ describe API::API, api: true do
   end
 
   describe 'GET /projects/:project_id/snippets/' do
+    let(:user) { create(:user) }
+
     it 'returns all snippets available to team member' do
-      project = create(:project, :public)
-      user = create(:user)
       project.team << [user, :developer]
       public_snippet = create(:project_snippet, :public, project: project)
       internal_snippet = create(:project_snippet, :internal, project: project)
@@ -34,8 +36,6 @@ describe API::API, api: true do
     end
 
     it 'hides private snippets from regular user' do
-      project = create(:project, :public)
-      user = create(:user)
       create(:project_snippet, :private, project: project)
 
       get api("/projects/#{project.id}/snippets/", user)
@@ -45,16 +45,16 @@ describe API::API, api: true do
   end
 
   describe 'POST /projects/:project_id/snippets/' do
-    it 'creates a new snippet' do
-      admin = create(:admin)
-      project = create(:project)
-      params = {
+    let(:params) do
+      {
         title: 'Test Title',
         file_name: 'test.rb',
         code: 'puts "hello world"',
         visibility_level: Gitlab::VisibilityLevel::PUBLIC
       }
+    end
 
+    it 'creates a new snippet' do
       post api("/projects/#{project.id}/snippets/", admin), params
 
       expect(response).to have_http_status(201)
@@ -64,12 +64,20 @@ describe API::API, api: true do
       expect(snippet.file_name).to eq(params[:file_name])
       expect(snippet.visibility_level).to eq(params[:visibility_level])
     end
+
+    it 'returns 400 for missing parameters' do
+      params.delete(:title)
+
+      post api("/projects/#{project.id}/snippets/", admin), params
+
+      expect(response).to have_http_status(400)
+    end
   end
 
   describe 'PUT /projects/:project_id/snippets/:id/' do
+    let(:snippet) { create(:project_snippet, author: admin) }
+
     it 'updates snippet' do
-      admin = create(:admin)
-      snippet = create(:project_snippet, author: admin)
       new_content = 'New content'
 
       put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin), code: new_content
@@ -78,9 +86,24 @@ describe API::API, api: true do
       snippet.reload
       expect(snippet.content).to eq(new_content)
     end
+
+    it 'returns 404 for invalid snippet id' do
+      put api("/projects/#{snippet.project.id}/snippets/1234", admin), title: 'foo'
+
+      expect(response).to have_http_status(404)
+      expect(json_response['message']).to eq('404 Snippet Not Found')
+    end
+
+    it 'returns 400 for missing parameters' do
+      put api("/projects/#{project.id}/snippets/1234", admin)
+
+      expect(response).to have_http_status(400)
+    end
   end
 
   describe 'DELETE /projects/:project_id/snippets/:id/' do
+    let(:snippet) { create(:project_snippet, author: admin) }
+
     it 'deletes snippet' do
       admin = create(:admin)
       snippet = create(:project_snippet, author: admin)
@@ -89,18 +112,31 @@ describe API::API, api: true do
 
       expect(response).to have_http_status(200)
     end
+
+    it 'returns 404 for invalid snippet id' do
+      delete api("/projects/#{snippet.project.id}/snippets/1234", admin)
+
+      expect(response).to have_http_status(404)
+      expect(json_response['message']).to eq('404 Snippet Not Found')
+    end
   end
 
   describe 'GET /projects/:project_id/snippets/:id/raw' do
-    it 'returns raw text' do
-      admin = create(:admin)
-      snippet = create(:project_snippet, author: admin)
+    let(:snippet) { create(:project_snippet, author: admin) }
 
+    it 'returns raw text' do
       get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/raw", admin)
 
       expect(response).to have_http_status(200)
       expect(response.content_type).to eq 'text/plain'
       expect(response.body).to eq(snippet.content)
     end
+
+    it 'returns 404 for invalid snippet id' do
+      delete api("/projects/#{snippet.project.id}/snippets/1234", admin)
+
+      expect(response).to have_http_status(404)
+      expect(json_response['message']).to eq('404 Snippet Not Found')
+    end
   end
 end