From fd01f3aacda1e7e1966489e7d9a31f89745cd509 Mon Sep 17 00:00:00 2001
From: Sebastian Ziebell <sebastian.ziebell@asquera.de>
Date: Wed, 13 Feb 2013 12:09:16 +0100
Subject: [PATCH] API: fixes a few return codes for project snippets

When using project snippets via API the functions now provide status codes for
different situations other then only returning 404 error. If required parameters are missing,
e.g. `title` when creating a project snippet a 400 (Bad request) error is returned. The snippet
delete function now is idempotent and returns a 200 (Ok) regardless if the snippet with the
given id is available or not. Changing return codes of these functions has the advantage that
the 404 error is used only for resources, which are not available.

Tests added to check these status codes when handling project snippets.
---
 lib/api/projects.rb                | 14 ++++++++++----
 spec/requests/api/projects_spec.rb | 30 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index f8c9701ecaf..02f10b60cb7 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -368,6 +368,10 @@ module Gitlab
       post ":id/snippets" do
         authorize! :write_snippet, user_project
 
+        error!("Title not given", 400) if !params[:title].present?
+        error!("Filename not given", 400) if !params[:file_name].present?
+        error!("Code not given", 400) if !params[:code].present?
+
         attrs = attributes_for_keys [:title, :file_name]
         attrs[:expires_at] = params[:lifetime] if params[:lifetime].present?
         attrs[:content] = params[:code] if params[:code].present?
@@ -415,10 +419,12 @@ module Gitlab
       # Example Request:
       #   DELETE /projects/:id/snippets/:snippet_id
       delete ":id/snippets/:snippet_id" do
-        @snippet = user_project.snippets.find(params[:snippet_id])
-        authorize! :modify_snippet, @snippet
-
-        @snippet.destroy
+        begin
+          @snippet = user_project.snippets.find(params[:snippet_id])
+          authorize! :modify_snippet, user_project
+          @snippet.destroy
+        rescue
+        end
       end
 
       # Get a raw project snippet
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 7a3677d157d..a04c2318a79 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -411,6 +411,18 @@ describe Gitlab::API do
         file_name: 'sample.rb', code: 'test'
       response.status.should == 400
     end
+
+    it "should return a 400 error if file_name not given" do
+      post api("/projects/#{project.id}/snippets", user),
+        title: 'api test', code: 'test'
+      response.status.should == 400
+    end
+
+    it "should return a 400 error if code not given" do
+      post api("/projects/#{project.id}/snippets", user),
+        title: 'api test', file_name: 'sample.rb'
+      response.status.should == 400
+    end
   end
 
   describe "PUT /projects/:id/snippets/:shippet_id" do
@@ -421,6 +433,13 @@ describe Gitlab::API do
       json_response['title'].should == 'example'
       snippet.reload.content.should == 'updated code'
     end
+
+    it "should update an existing project snippet with new title" do
+      put api("/projects/#{project.id}/snippets/#{snippet.id}", user),
+        title: 'other api test'
+      response.status.should == 200
+      json_response['title'].should == 'other api test'
+    end
   end
 
   describe "DELETE /projects/:id/snippets/:snippet_id" do
@@ -428,6 +447,12 @@ describe Gitlab::API do
       expect {
         delete api("/projects/#{project.id}/snippets/#{snippet.id}", user)
       }.to change { Snippet.count }.by(-1)
+      response.status.should == 200
+    end
+
+    it "should return success when deleting unknown snippet id" do
+      delete api("/projects/#{project.id}/snippets/1234", user)
+      response.status.should == 200
     end
   end
 
@@ -436,6 +461,11 @@ describe Gitlab::API do
       get api("/projects/#{project.id}/snippets/#{snippet.id}/raw", user)
       response.status.should == 200
     end
+
+    it "should return a 404 error if raw project snippet not found" do
+      get api("/projects/#{project.id}/snippets/5555/raw", user)
+      response.status.should == 404
+    end
   end
 
   describe "GET /projects/:id/:sha/blob" do
-- 
GitLab