Repository: nuveo/prest
Pull request #176 (2017-06-29 21:57:32)

Add Group by clause support
refs #153

This PR also add support to Group Functions like SUM, AVG.

codecov[bot]: # [Codecov](https://codecov.io/gh/nuveo/prest/pull/176?src=pr&el=h1) Report
> Merging [#176](https://codecov.io/gh/nuveo/prest/pull/176?src=pr&el=desc) into [master](https://codecov.io/gh/nuveo/prest/commit/2a6db097af06d2b597419236b2bccb7c45b73322?src=pr&el=desc) will **decrease** coverage by `0.07%`.
> The diff coverage is `80%`.

[![Impacted file tree graph](https://codecov.io/gh/nuveo/prest/pull/176/graphs/tree.svg?token=eVD9urwIEv&width=650&height=150&src=pr)](https://codecov.io/gh/nuveo/prest/pull/176?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #176      +/-   ##
==========================================
- Coverage   86.33%   86.25%   -0.08%     
==========================================
  Files          11       11              
  Lines         856      866      +10     
==========================================
+ Hits          739      747       +8     
- Misses         80       81       +1     
- Partials       37       38       +1
```


| [Impacted Files](https://codecov.io/gh/nuveo/prest/pull/176?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [adapters/postgres/postgres.go](https://codecov.io/gh/nuveo/prest/pull/176?src=pr&el=tree#diff-YWRhcHRlcnMvcG9zdGdyZXMvcG9zdGdyZXMuZ28=) | `84.4% <100%> (+0.27%)` | :arrow_up: |
| [controllers/tables.go](https://codecov.io/gh/nuveo/prest/pull/176?src=pr&el=tree#diff-Y29udHJvbGxlcnMvdGFibGVzLmdv) | `95.83% <33.33%> (-1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/nuveo/prest/pull/176?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/176?src=pr&el=footer). Last update [2a6db09...81ed999](https://codecov.io/gh/nuveo/prest/pull/176?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

marioidival: WIP yet

marioidival: Working in README

Patch:
diff --git a/README.md b/README.md
index 6e25c6c..7d87c89 100644
--- a/README.md
+++ b/README.md
@@ -282,6 +282,28 @@ Examples:
     GET /DATABASE/SCHEMA/TABLE/?_order=fieldname01,-fieldname02,fieldname03
 
 
+## GROUP BY
+
+We support this Group Functions:
+
+| name | Use in request |
+| ------- | ------------- |
+| SUM | sum:field |
+| AVG | avg:field |
+| MAX | max:field |
+| MIN | min:field |
+| MEDIAN | median:field |
+| STDDEV | stddev:field |
+| VARIANCE | variance:field |
+
+### Examples:
+	GET /DATABASE/SCHEMA/TABLE/?_select=fieldname00,fieldname01&_groupby=fieldname01
+
+#### Using Group Functions
+	GET /DATABASE/SCHEMA/TABLE/?_select=fieldname00,sum:fieldname01&_groupby=fieldname01
+
+
+
 ## Executing SQL scripts
 
 If need perform an advanced SQL, you can write some scripts SQL and access them by REST. These scripts are templates where you can pass by URL, values to them.
diff --git a/adapters/postgres/postgres.go b/adapters/postgres/postgres.go
index 66182ab..4e1acb9 100644
--- a/adapters/postgres/postgres.go
+++ b/adapters/postgres/postgres.go
@@ -44,6 +44,8 @@ func chkInvalidIdentifier(identifer ...string) bool {
 		for _, v := range ival {
 			if !unicode.IsLetter(v) &&
 				!unicode.IsDigit(v) &&
+				v != '(' &&
+				v != ')' &&
 				v != '_' &&
 				v != '.' &&
 				v != '-' &&
@@ -614,26 +616,40 @@ func TablePermissions(table string, op string) bool {
 }
 
 // FieldsPermissions get fields permissions based in prest configuration
-func FieldsPermissions(table string, cols []string, op string) []string {
+func FieldsPermissions(r *http.Request, table string, op string) []string {
 	restrict := config.PrestConf.AccessConf.Restrict
+	cols := ColumnsByRequest(r)
 	if !restrict {
 		return cols
 	}
 
+	queries := r.URL.Query()
+
 	var permittedCols []string
 	tables := config.PrestConf.AccessConf.Tables
 	for _, t := range tables {
 		if t.Name == table {
-			for _, f := range t.Fields {
-				for _, col := range cols {
-					// return all permitted fields if have "*" in SELECT
-					if op == "read" && col == "*" {
-						return t.Fields
-					}
+			for _, col := range cols {
+				// return all permitted fields if have "*" in SELECT
+				if op == "read" && col == "*" {
+					return t.Fields
+				}
 
-					if col == f {
+				if queries.Get("_groupby") != "" {
+					if strings.Contains(col, ":") {
+						groupFunc, err := NormalizeGroupFunction(col)
+						if err == nil {
+							permittedCols = append(permittedCols, groupFunc)
+						}
+					} else {
 						permittedCols = append(permittedCols, col)
 					}
+				} else {
+					for _, f := range t.Fields {
+						if col == f {
+							permittedCols = append(permittedCols, col)
+						}
+					}
 				}
 			}
 			return permittedCols
@@ -661,3 +677,31 @@ func ColumnsByRequest(r *http.Request) []string {
 	}
 	return columns
 }
+
+// GroupByClause get params in request to add group by clause
+func GroupByClause(r *http.Request) (groupBySQL string) {
+	queries := r.URL.Query()
+	groupQuery := queries.Get("_groupby")
+	if groupQuery == "" {
+		return
+	}
+
+	groupBySQL = fmt.Sprintf(statements.GroupBy, groupQuery)
+	return
+}
+
+// NormalizeGroupFunction normalize url params values to sql group functions
+func NormalizeGroupFunction(paramValue string) (groupFuncSQL string, err error) {
+	values := strings.Split(paramValue, ":")
+	groupFunc := strings.ToUpper(values[0])
+
+	switch groupFunc {
+	case "SUM", "AVG", "MAX", "MIN", "MEDIAN", "STDDEV", "VARIANCE":
+		// values[1] it's a field in table
+		groupFuncSQL = fmt.Sprintf("%s(%s)", groupFunc, values[1])
+		return
+	default:
+		err = fmt.Errorf("this function %s is not a valid group function", groupFunc)
+		return
+	}
+}
diff --git a/controllers/tables.go b/controllers/tables.go
index 619bf0e..afd0add 100644
--- a/controllers/tables.go
+++ b/controllers/tables.go
@@ -112,8 +112,7 @@ func SelectFromTables(w http.ResponseWriter, r *http.Request) {
 	table := vars["table"]
 
 	// get selected columns, "*" if empty "_columns"
-	cols := postgres.ColumnsByRequest(r)
-	cols = postgres.FieldsPermissions(table, cols, "read")
+	cols := postgres.FieldsPermissions(r, table, "read")
 
 	if len(cols) == 0 {
 		err := fmt.Errorf("you don't have permission for this action, please check the permitted fields for this table")
@@ -165,6 +164,12 @@ func SelectFromTables(w http.ResponseWriter, r *http.Request) {
 			requestWhere)
 	}
 
+	groupBySQL := postgres.GroupByClause(r)
+
+	if groupBySQL != "" {
+		sqlSelect = fmt.Sprintf("%s %s", sqlSelect, groupBySQL)
+	}
+
 	order, err := postgres.OrderByRequest(r)
 	if err != nil {
 		err = fmt.Errorf("could not perform OrderByRequest: %v", err)
diff --git a/statements/queries.go b/statements/queries.go
index b03d752..a531695 100644
--- a/statements/queries.go
+++ b/statements/queries.go
@@ -120,6 +120,9 @@ DELETE FROM %s.%s.%s`
 	// UpdateQuery query
 	UpdateQuery = `
 UPDATE %s.%s.%s SET %s`
+
+	// GroupBy query
+	GroupBy = `GROUP BY %s`
 )
 
 var (


Test patch:
diff --git a/adapters/postgres/postgres_test.go b/adapters/postgres/postgres_test.go
index 7ac1337..942b98b 100644
--- a/adapters/postgres/postgres_test.go
+++ b/adapters/postgres/postgres_test.go
@@ -186,6 +186,41 @@ func TestInvalidWhereByRequest(t *testing.T) {
 	}
 }
 
+func TestGroupByClause(t *testing.T) {
+	var testCases = []struct {
+		description string
+		url         string
+		expectedSQL string
+		emptyCase   bool
+	}{
+		{"Group by clause with one field", "/prest/public/test5?_groupby=celphone", "GROUP BY celphone", false},
+		{"Group by clause with two fields", "/prest/public/test5?_groupby=celphone,name", "GROUP BY celphone,name", false},
+		{"Group by clause without fields", "/prest/public/test5?_groupby=", "", true},
+	}
+
+	for _, tc := range testCases {
+		t.Log(tc.description)
+		req, err := http.NewRequest("GET", tc.url, nil)
+		if err != nil {
+			t.Errorf("expected no errors in http request, got %v", err)
+		}
+
+		groupBySQL := GroupByClause(req)
+
+		if !tc.emptyCase && groupBySQL == "" {
+			t.Error("expected groupBySQL, got empty string")
+		}
+
+		if tc.emptyCase && groupBySQL != "" {
+			t.Errorf("expected empty, got %v", groupBySQL)
+		}
+
+		if groupBySQL != tc.expectedSQL {
+			t.Errorf("expected %s, got %s", tc.expectedSQL, groupBySQL)
+		}
+	}
+}
+
 func TestEmptyTable(t *testing.T) {
 	response, err := Query("SELECT * FROM test_empty_table")
 	if err != nil {
@@ -435,6 +470,7 @@ func TestChkInvaidIdentifier(t *testing.T) {
 		{"fild'Name", true},
 		{"fild\"Name", true},
 		{"fild;Name", true},
+		{"SUM(test)", false},
 		{"_123456789_123456789_123456789_123456789_123456789_123456789_12345", true},
 	}
 
@@ -736,20 +772,26 @@ func TestTablePermissions(t *testing.T) {
 func TestFieldsPermissions(t *testing.T) {
 	var testCases = []struct {
 		description string
+		url         string
 		table       string
-		fields      []string
 		permission  string
 		resultLen   int
 	}{
-		{"Read valid field", "test_list_only_id", []string{"id"}, "read", 1},
-		{"Read invalid field", "test_list_only_id", []string{"name"}, "read", 0},
-		{"Read non existing field", "test_list_only_id", []string{"non_existing_field"}, "read", 0},
-		{"Select with *", "test_list_only_id", []string{"*"}, "read", 1},
+		{"Read valid field", "/prest/public/test_list_only_id?_select=id", "test_list_only_id", "read", 1},
+		{"Read invalid field", "/prest/public/test_list_only_id?_select=name", "test_list_only_id", "read", 0},
+		{"Read non existing field", "/prest/public/test_list_only_id?_select=non_existing_field", "test_list_only_id", "read", 0},
+		{"Select with *", "/prest/public/test_list_only_id?_select=*", "test_list_only_id", "read", 1},
 	}
 
 	for _, tc := range testCases {
 		t.Log(tc.description)
-		fields := FieldsPermissions(tc.table, tc.fields, tc.permission)
+
+		r, err := http.NewRequest("GET", tc.url, nil)
+		if err != nil {
+			t.Errorf("expected no errors on NewRequest, but got: %v", err)
+		}
+
+		fields := FieldsPermissions(r, tc.table, tc.permission)
 		if len(fields) != tc.resultLen {
 			t.Errorf("expected %d, got: %d - %v", tc.resultLen, len(fields), fields)
 		}
@@ -759,8 +801,14 @@ func TestFieldsPermissions(t *testing.T) {
 func TestRestrictFalse(t *testing.T) {
 	config.PrestConf.AccessConf.Restrict = false
 
-	t.Log("Read unrestrict")
-	fields := FieldsPermissions("test_list_only_id", []string{"*"}, "read")
+	t.Log("Read unrestrict", config.PrestConf.AccessConf.Restrict)
+
+	r, err := http.NewRequest("GET", "/prest/public/test_list_only_id?_select=*", nil)
+	if err != nil {
+		t.Errorf("expected no errors on NewRequest, but got: %v", err)
+	}
+
+	fields := FieldsPermissions(r, "test_list_only_id", "read")
 	if fields[0] != "*" {
 		t.Errorf("expected '*', got: %s", fields[0])
 	}
@@ -841,6 +889,7 @@ func TestColumnsByRequest(t *testing.T) {
 		}
 	}
 }
+
 func TestParseArray(t *testing.T) {
 	in := []interface{}{"value 1", "value 2", "value 3"}
 	ret := parseArray(in)
@@ -863,3 +912,30 @@ func TestParseArray(t *testing.T) {
 		t.Errorf("Error expected %s, got %s", retString, ret)
 	}
 }
+
+func TestNormalizeGroupFunction(t *testing.T) {
+	var testCases = []struct {
+		description string
+		urlValue    string
+		expectedSQL string
+	}{
+		{"Normalize AVG Function", "avg:age", "AVG(age)"},
+		{"Normalize SUM Function", "sum:age", "SUM(age)"},
+		{"Normalize MAX Function", "max:age", "MAX(age)"},
+		{"Normalize MIN Function", "min:age", "MIN(age)"},
+		{"Normalize MEDIAN Function", "median:age", "MEDIAN(age)"},
+		{"Normalize STDDEV Function", "stddev:age", "STDDEV(age)"},
+		{"Normalize VARIANCE Function", "variance:age", "VARIANCE(age)"},
+	}
+
+	for _, tc := range testCases {
+		partialSQL, err := NormalizeGroupFunction(tc.urlValue)
+		if err != nil {
+			t.Errorf("This function should not return error: %s", tc.description)
+		}
+
+		if tc.expectedSQL != partialSQL {
+			t.Errorf("expected: %s, got: %s", tc.expectedSQL, partialSQL)
+		}
+	}
+}
diff --git a/adapters/postgres/queries_test.go b/adapters/postgres/queries_test.go
index 350a030..fcecc3f 100644
--- a/adapters/postgres/queries_test.go
+++ b/adapters/postgres/queries_test.go
@@ -12,6 +12,7 @@ import (
 )
 
 func TestMain(m *testing.M) {
+	os.Setenv("PREST_CONF", "../../testdata/prest.toml")
 	config.Load()
 	createMockScripts(config.PrestConf.QueriesPath)
 	writeMockScripts(config.PrestConf.QueriesPath)
diff --git a/controllers/tables_test.go b/controllers/tables_test.go
index d524732..7dddaa5 100644
--- a/controllers/tables_test.go
+++ b/controllers/tables_test.go
@@ -84,11 +84,13 @@ func TestSelectFromTables(t *testing.T) {
 		{"execute select in a table with count function", "/prest/public/test?_count=name", "GET", http.StatusOK, ""},
 		{"execute select in a table with custom where clause", "/prest/public/test?name=$eq.nuveo", "GET", http.StatusOK, ""},
 		{"execute select in a table with custom join clause", "/prest/public/test?_join=inner:test8:test8.nameforjoin:$eq:test.name", "GET", http.StatusOK, ""},
-		{"execute select in a table with order clause", "/prest/public/test?_order=name", "GET", http.StatusOK, ""},
 		{"execute select in a table with order clause empty", "/prest/public/test?_order=", "GET", http.StatusOK, ""},
 		{"execute select in a table with custom where clause and pagination", "/prest/public/test?name=$eq.nuveo&_page=1&_page_size=20", "GET", http.StatusOK, ""},
 		{"execute select in a table with select fields", "/prest/public/test5?_select=celphone,name", "GET", http.StatusOK, ""},
 		{"execute select in a table with select *", "/prest/public/test5?_select=*", "GET", http.StatusOK, ""},
+
+		{"execute select in a table with group by clause", "/prest/public/test_group_by_table?_select=age,sum:salary&_groupby=age", "GET", http.StatusOK, "[{\"age\":20,\"sum\":1350}, \n {\"age\":19,\"sum\":7997}]"},
+
 		{"execute select in a view without custom where clause", "/prest/public/view_test", "GET", http.StatusOK, ""},
 		{"execute select in a view with count all fields *", "/prest/public/view_test?_count=*", "GET", http.StatusOK, ""},
 		{"execute select in a view with count function", "/prest/public/view_test?_count=player", "GET", http.StatusOK, ""},
@@ -106,6 +108,8 @@ func TestSelectFromTables(t *testing.T) {
 		{"execute select in a table with invalid where clause", "/prest/public/test?0name=$eq.nuveo", "GET", http.StatusBadRequest, ""},
 		{"execute select in a table with invalid count clause", "/prest/public/test?_count=0name", "GET", http.StatusBadRequest, ""},
 		{"execute select in a table with invalid order clause", "/prest/public/test?_order=0name", "GET", http.StatusBadRequest, ""},
+		{"execute select in a table with invalid fields using group by clause", "/prest/public/test_group_by_table?_select=pa,sum:pum&_groupby=pa", "GET", http.StatusBadRequest, ""},
+
 		{"execute select in a view with an other column", "/prest/public/view_test?_select=celphone", "GET", http.StatusBadRequest, ""},
 		{"execute select in a view with where and column invalid", "/prest/public/view_test?0celphone=$eq.888888", "GET", http.StatusBadRequest, ""},
 		{"execute select in a view with custom join clause invalid", "/prest/public/view_test?_join=inner:test2.name:eq:view_test.player", "GET", http.StatusBadRequest, ""},
diff --git a/testdata/prest.toml b/testdata/prest.toml
index 0f3c494..d54a7a0 100644
--- a/testdata/prest.toml
+++ b/testdata/prest.toml
@@ -7,6 +7,7 @@
 port = 6000
 
 [pg]
+user = "postgres"
 database = "prest"
 
 [access]
@@ -66,3 +67,8 @@ restrict = true  # can access only the tables listed below
     permissions = ["read"]
     fields = ["player"]
 
+    [[access.tables]]
+    name = "test_group_by_table"
+    permissions = ["read"]
+    fields = ["id", "name", "age", "salary"]
+
diff --git a/testdata/schema.sh b/testdata/schema.sh
index 3889437..bb05dd8 100644
--- a/testdata/schema.sh
+++ b/testdata/schema.sh
@@ -18,6 +18,7 @@ psql prest -c "create table test_jsonb_bug(id serial NOT NULL, name text, data j
 psql prest -c "create table testjson(name text, data jsonb);" -U postgres
 psql prest -c "create table testarray(id serial, data character varying(250)[]);" -U postgres
 psql prest -c "create table test_empty_table(id serial, data character varying(250)[]);" -U postgres
+psql prest -c "create table test_group_by_table(id serial, name text, age integer, salary int);" -U postgres
 
 # Inserts
 psql prest -c "insert into test (name) values ('prest tester');" -U postgres
@@ -35,6 +36,11 @@ psql prest -c "insert into test_jsonb_bug (name, data) values('goku', '{\"techni
 psql prest -c "insert into test_jsonb_bug (name, data) values('goku', '{\"description\": \"testing\"}');" -U postgres
 psql prest -c "insert into testarray (id, data) values(100, '{\"Gohan\",\"Goten\"}');" -U postgres
 
+psql prest -c "insert into test_group_by_table(name, age, salary) values('joao', 20, 1250);" -U postgres
+psql prest -c "insert into test_group_by_table(name, age, salary) values('maria', 19, 3999);" -U postgres
+psql prest -c "insert into test_group_by_table(name, age, salary) values('gopher', 20, 100);" -U postgres
+psql prest -c "insert into test_group_by_table(name, age, salary) values('guitarra humana', 19, 3998);" -U postgres
+
 # Views
 psql prest -c "create table table_to_view(id serial, name text, celphone text);" -U postgres
 psql prest -c "insert into table_to_view (name, celphone) values ('gopher', '8888888')" -U postgres