Repository: nuveo/prest
Pull request #109 (2017-02-22 14:02:53)

execute script as public function
Splitting `ExecuteFromScripts` in two functions, to be used as public function and possible to call from external module

henriquechehad: @avelino @felipeweb @crgimenes review please
codecov-io: # [Codecov](https://codecov.io/gh/nuveo/prest/pull/109?src=pr&el=h1) Report
> Merging [#109](https://codecov.io/gh/nuveo/prest/pull/109?src=pr&el=desc) into [master](https://codecov.io/gh/nuveo/prest/commit/4eb65dd0e053dc67fe9da2d81eb27bbdd7cef440?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `100%`.


```diff
@@            Coverage Diff             @@
##           master     #109      +/-   ##
==========================================
+ Coverage   89.82%   89.86%   +0.03%     
==========================================
  Files           9        9              
  Lines         806      809       +3     
==========================================
+ Hits          724      727       +3     
  Misses         53       53              
  Partials       29       29
```


| [Impacted Files](https://codecov.io/gh/nuveo/prest/pull/109?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [controllers/sql.go](https://codecov.io/gh/nuveo/prest/compare/4eb65dd0e053dc67fe9da2d81eb27bbdd7cef440...33dd92f0de329e260bac6a6b4a1515152083cc88?src=pr&el=tree#diff-Y29udHJvbGxlcnMvc3FsLmdv) | `100% <100%> (ø)` | :white_check_mark: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/nuveo/prest/pull/109?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/nuveo/prest/pull/109?src=pr&el=footer). Last update [4eb65dd...33dd92f](https://codecov.io/gh/nuveo/prest/compare/4eb65dd0e053dc67fe9da2d81eb27bbdd7cef440...33dd92f0de329e260bac6a6b4a1515152083cc88?el=footer&src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Patch:
diff --git a/adapters/postgres/postgres.go b/adapters/postgres/postgres.go
index ca982d1..2205047 100644
--- a/adapters/postgres/postgres.go
+++ b/adapters/postgres/postgres.go
@@ -125,11 +125,12 @@ func WhereByRequest(r *http.Request, initialPlaceholderID int) (whereSyntax stri
 }
 
 // DatabaseClause return a SELECT `query`
-func DatabaseClause(req *http.Request) (query string) {
+func DatabaseClause(req *http.Request) (query string, hasCount bool) {
 	queries := req.URL.Query()
-	hasCount := queries.Get("_count")
+	countQuery := queries.Get("_count")
 
-	if hasCount != "" {
+	if countQuery != "" {
+		hasCount = true
 		query = fmt.Sprintf(statements.DatabasesSelect, statements.FieldCountDatabaseName)
 	} else {
 		query = fmt.Sprintf(statements.DatabasesSelect, statements.FieldDatabaseName)
@@ -138,11 +139,12 @@ func DatabaseClause(req *http.Request) (query string) {
 }
 
 // SchemaClause return a SELECT `query`
-func SchemaClause(req *http.Request) (query string) {
+func SchemaClause(req *http.Request) (query string, hasCount bool) {
 	queries := req.URL.Query()
-	hasCount := queries.Get("_count")
+	countQuery := queries.Get("_count")
 
-	if hasCount != "" {
+	if countQuery != "" {
+		hasCount = true
 		query = fmt.Sprintf(statements.SchemasSelect, statements.FieldCountSchemaName)
 	} else {
 		query = fmt.Sprintf(statements.SchemasSelect, statements.FieldSchemaName)
diff --git a/controllers/databases.go b/controllers/databases.go
index 146990b..62b8e1c 100644
--- a/controllers/databases.go
+++ b/controllers/databases.go
@@ -18,7 +18,7 @@ func GetDatabases(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	query := postgres.DatabaseClause(r)
+	query, hasCount := postgres.DatabaseClause(r)
 	sqlDatabases := fmt.Sprint(query, statements.DatabasesWhere)
 
 	if requestWhere != "" {
@@ -28,7 +28,7 @@ func GetDatabases(w http.ResponseWriter, r *http.Request) {
 	order, _ := postgres.OrderByRequest(r)
 	if order != "" {
 		sqlDatabases = fmt.Sprint(sqlDatabases, order)
-	} else {
+	} else if !hasCount {
 		sqlDatabases = fmt.Sprint(sqlDatabases, fmt.Sprintf(statements.DatabasesOrderBy, statements.FieldDatabaseName))
 	}
 
@@ -39,7 +39,6 @@ func GetDatabases(w http.ResponseWriter, r *http.Request) {
 	}
 
 	sqlDatabases = fmt.Sprint(sqlDatabases, " ", page)
-
 	object, err := postgres.Query(sqlDatabases, values...)
 	if err != nil {
 		log.Println(err)
diff --git a/controllers/schemas.go b/controllers/schemas.go
index 722117d..66003b0 100644
--- a/controllers/schemas.go
+++ b/controllers/schemas.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"log"
 	"net/http"
-	"strings"
 
 	"github.com/nuveo/prest/adapters/postgres"
 	"github.com/nuveo/prest/statements"
@@ -19,20 +18,16 @@ func GetSchemas(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	sqlSchemas := postgres.SchemaClause(r)
+	sqlSchemas, hasCount := postgres.SchemaClause(r)
 
 	if requestWhere != "" {
 		sqlSchemas = fmt.Sprint(sqlSchemas, " WHERE ", requestWhere)
 	}
 
-	if strings.Contains(sqlSchemas, "COUNT") {
-		sqlSchemas = fmt.Sprint(sqlSchemas, fmt.Sprintf(statements.SchemasGroupBy, statements.FieldSchemaName))
-	}
-
 	order, _ := postgres.OrderByRequest(r)
 	if order != "" {
 		sqlSchemas = fmt.Sprint(sqlSchemas, order)
-	} else {
+	} else if !hasCount {
 		sqlSchemas = fmt.Sprint(sqlSchemas, fmt.Sprintf(statements.SchemasOrderBy, statements.FieldSchemaName))
 	}
 
@@ -43,7 +38,6 @@ func GetSchemas(w http.ResponseWriter, r *http.Request) {
 	}
 
 	sqlSchemas = fmt.Sprint(sqlSchemas, " ", page)
-
 	object, err := postgres.Query(sqlSchemas, values...)
 	if err != nil {
 		log.Println(err)
diff --git a/controllers/sql.go b/controllers/sql.go
index 5711a8a..2e8e8a8 100644
--- a/controllers/sql.go
+++ b/controllers/sql.go
@@ -8,29 +8,37 @@ import (
 	"github.com/nuveo/prest/adapters/postgres"
 )
 
-// ExecuteFromScripts is a controller to peform SQL in scripts created by users
-func ExecuteFromScripts(rw http.ResponseWriter, rq *http.Request) {
-	vars := mux.Vars(rq)
-	queriesPath := vars["queriesLocation"]
-	script := vars["script"]
-
+// ExecuteScriptQuery is a function to execute and return result of script query
+func ExecuteScriptQuery(rq *http.Request, queriesPath string, script string) ([]byte, error) {
 	sqlPath, err := postgres.GetScript(rq.Method, queriesPath, script)
 	if err != nil {
 		log.Printf("could not get script %s/%s, %+v", queriesPath, script, err)
-		http.Error(rw, err.Error(), http.StatusBadRequest)
-		return
+		return nil, err
 	}
 
 	sql, values, err := postgres.ParseScript(sqlPath, rq.URL.Query())
 	if err != nil {
 		log.Printf("could not parse script %s/%s, %+v", queriesPath, script, err)
-		http.Error(rw, err.Error(), http.StatusBadRequest)
-		return
+		return nil, err
 	}
 
 	result, err := postgres.ExecuteScripts(rq.Method, sql, values)
 	if err != nil {
 		log.Printf("could not execute sql %+v, %s", err, sql)
+		return nil, err
+	}
+
+	return result, nil
+}
+
+// ExecuteFromScripts is a controller to peform SQL in scripts created by users
+func ExecuteFromScripts(rw http.ResponseWriter, rq *http.Request) {
+	vars := mux.Vars(rq)
+	queriesPath := vars["queriesLocation"]
+	script := vars["script"]
+
+	result, err := ExecuteScriptQuery(rq, queriesPath, script)
+	if err != nil {
 		http.Error(rw, err.Error(), http.StatusBadRequest)
 		return
 	}


Test patch:
diff --git a/adapters/postgres/postgres_test.go b/adapters/postgres/postgres_test.go
index 871560f..7e1ed1b 100644
--- a/adapters/postgres/postgres_test.go
+++ b/adapters/postgres/postgres_test.go
@@ -536,7 +536,7 @@ func TestDatabaseClause(t *testing.T) {
 			t.Errorf("expected no errors on NewRequest, got: %v", err)
 		}
 
-		query := DatabaseClause(r)
+		query, _ := DatabaseClause(r)
 		if query != tc.queryExpected {
 			t.Errorf("query unexpected, got: %s", query)
 		}
@@ -561,7 +561,7 @@ func TestSchemaClause(t *testing.T) {
 			t.Errorf("expected no errors on NewRequest, got: %v", err)
 		}
 
-		query := SchemaClause(r)
+		query, _ := SchemaClause(r)
 		if query != tc.queryExpected {
 			t.Errorf("query unexpected, got: %s", query)
 		}
diff --git a/controllers/databases_test.go b/controllers/databases_test.go
index 82231bb..3092a7b 100644
--- a/controllers/databases_test.go
+++ b/controllers/databases_test.go
@@ -20,7 +20,7 @@ func TestGetDatabases(t *testing.T) {
 		{"Get databases with custom where clause", "/databases?datname=$eq.prest", "GET", 200},
 		{"Get databases with custom order clause", "/databases?_order=datname", "GET", 200},
 		{"Get databases with custom where clause and pagination", "/databases?datname=$eq.prest&_page=1&_page_size=20", "GET", 200},
-		{"Get databases with COUNT clause", "/databases?_count=", "GET", 200},
+		{"Get databases with COUNT clause", "/databases?_count=*", "GET", 200},
 		{"Get databases with custom where invalid clause", "/databases?0datname=prest", "GET", 400},
 		{"Get databases with custom where and pagination invalid", "/databases?datname=$eq.prest&_page=A", "GET", 400},
 		{"Get databases with noexistent column", "/databases?datatata=$eq.test", "GET", 500},
diff --git a/controllers/schemas_test.go b/controllers/schemas_test.go
index 1218514..886841d 100644
--- a/controllers/schemas_test.go
+++ b/controllers/schemas_test.go
@@ -15,15 +15,16 @@ func TestGetSchemas(t *testing.T) {
 		url         string
 		method      string
 		status      int
+		body        string
 	}{
-		{"Get schemas without custom where clause", "/schemas", "GET", 200},
-		{"Get schemas with custom where clause", "/schemas?schema_name=$eq.public", "GET", 200},
-		{"Get schemas with custom order clause", "/schemas?schema_name=$eq.public&_order=schema_name", "GET", 200},
-		{"Get schemas with custom where clause and pagination", "/schemas?schema_name=$eq.public&_page=1&_page_size=20", "GET", 200},
-		{"Get schemas with COUNT clause", "/schemas?_count=*", "GET", 200},
-		{"Get schemas with custom where invalid clause", "/schemas?0schema_name=$eq.public", "GET", 400},
-		{"Get schemas with custom where and pagination invalid", "/schemas?schema_name=$eq.public&_page=A", "GET", 400},
-		{"Get schemas with noexistent column", "/schemas?schematame=$eq.test", "GET", 500},
+		{"Get schemas without custom where clause", "/schemas", "GET", 200, "[{\"schema_name\":\"information_schema\"},{\"schema_name\":\"pg_catalog\"},{\"schema_name\":\"pg_temp_1\"},{\"schema_name\":\"pg_toast\"},{\"schema_name\":\"pg_toast_temp_1\"},{\"schema_name\":\"public\"}]"},
+		{"Get schemas with custom where clause", "/schemas?schema_name=$eq.public", "GET", 200, "[{\"schema_name\":\"public\"}]"},
+		{"Get schemas with custom order clause", "/schemas?schema_name=$eq.public&_order=schema_name", "GET", 200, "[{\"schema_name\":\"public\"}]"},
+		{"Get schemas with custom where clause and pagination", "/schemas?schema_name=$eq.public&_page=1&_page_size=20", "GET", 200, "[{\"schema_name\":\"public\"}]"},
+		{"Get schemas with COUNT clause", "/schemas?_count=*", "GET", 200, "[{\"count\":6}]"},
+		{"Get schemas with custom where invalid clause", "/schemas?0schema_name=$eq.public", "GET", 400, "invalid identifier: 0schema_name\n"},
+		{"Get schemas with custom where and pagination invalid", "/schemas?schema_name=$eq.public&_page=A", "GET", 400, "Paging error\n"},
+		{"Get schemas with noexistent column", "/schemas?schematame=$eq.test", "GET", 500, "pq: column \"schematame\" does not exist\n"},
 	}
 
 	router := mux.NewRouter()
@@ -34,6 +35,7 @@ func TestGetSchemas(t *testing.T) {
 	r := api.Request{}
 	for _, tc := range testCases {
 		t.Log(tc.description)
-		doRequest(t, server.URL+tc.url, r, tc.method, tc.status, "GetSchemas")
+		doRequest(t, server.URL+tc.url, r, tc.method, tc.status, "GetSchemas", tc.body)
+
 	}
 }
diff --git a/controllers/sql_test.go b/controllers/sql_test.go
index a983613..6dbc253 100644
--- a/controllers/sql_test.go
+++ b/controllers/sql_test.go
@@ -1,6 +1,7 @@
 package controllers
 
 import (
+	"net/http"
 	"net/http/httptest"
 	"os"
 	"testing"
@@ -21,6 +22,47 @@ func TestMain(m *testing.M) {
 	os.Exit(code)
 }
 
+func TestExecuteScriptQuery(t *testing.T) {
+	r := mux.NewRouter()
+	r.HandleFunc("/testing/script-get/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		resp, err := ExecuteScriptQuery(r, "fulltable", "get_all")
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+		}
+		w.Write(resp)
+	}))
+
+	r.HandleFunc("/testing/script-post/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		resp, err := ExecuteScriptQuery(r, "fulltable", "write_all")
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+		}
+		w.Write(resp)
+	}))
+
+	ts := httptest.NewServer(r)
+	defer ts.Close()
+
+	var testCases = []struct {
+		description string
+		url         string
+		method      string
+		status      int
+	}{
+		{"Execute script GET method", "/testing/script-get/?field1=gopher", "GET", 200},
+		{"Execute script POST method", "/testing/script-post/?field1=gopherzin&field2=pereira", "POST", 200},
+		// errors
+		{"Execute script GET method invalid", "/testing/script-get/?nonexistent=gopher", "GET", 400},
+		{"Execute script POST method invalid", "/testing/script-post/?nonexistent=gopher", "POST", 400},
+	}
+
+	apiReq := api.Request{}
+	for _, tc := range testCases {
+		t.Log(tc.description)
+		doRequest(t, ts.URL+tc.url, apiReq, tc.method, tc.status, "ExecuteScriptQuery")
+	}
+}
+
 func TestExecuteFromScripts(t *testing.T) {
 	router := mux.NewRouter()
 	router.HandleFunc("/_QUERIES/{queriesLocation}/{script}", ExecuteFromScripts)
diff --git a/controllers/utils_test.go b/controllers/utils_test.go
index c203904..a25bb41 100644
--- a/controllers/utils_test.go
+++ b/controllers/utils_test.go
@@ -148,7 +148,7 @@ func doValidPatchRequest(t *testing.T, url string, r api.Request, where string)
 	}
 }
 
-func doRequest(t *testing.T, url string, r api.Request, method string, expectedStatus int, where string) {
+func doRequest(t *testing.T, url string, r api.Request, method string, expectedStatus int, where string, expectedBody ...string) {
 	fmt.Println("Test:", where)
 	var byt []byte
 	var err error
@@ -175,10 +175,16 @@ func doRequest(t *testing.T, url string, r api.Request, method string, expectedS
 		t.Errorf("expected %d, got: %d", expectedStatus, resp.StatusCode)
 	}
 
-	_, err = ioutil.ReadAll(resp.Body)
+	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
 		t.Error("error on ioutil ReadAll", err)
 	}
+
+	if len(expectedBody) > 0 {
+		if string(body) != expectedBody[0] {
+			t.Errorf("expected %q, got: %q", expectedBody, string(body))
+		}
+	}
 }
 
 func createMockScripts(base string) {