From 227365af6c4c38b2c7db039dcb13fef5ccb9c423 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Wed, 4 Dec 2024 16:20:14 -0500 Subject: [PATCH 01/25] Issue 3153 first commit --- http/handler_store.go | 17 +++++++- http/handler_store_test.go | 77 ++++++++++++++++++++++++++++++++++++ playground/package-lock.json | 75 ++--------------------------------- 3 files changed, 97 insertions(+), 72 deletions(-) diff --git a/http/handler_store.go b/http/handler_store.go index 35436f3762..9d1bd037c8 100644 --- a/http/handler_store.go +++ b/http/handler_store.go @@ -285,6 +285,22 @@ func (s *storeHandler) ExecRequest(rw http.ResponseWriter, req *http.Request) { switch { case req.URL.Query().Get("query") != "": request.Query = req.URL.Query().Get("query") + + operationNameFromQuery := req.URL.Query().Get("operationName") + if operationNameFromQuery != "" { + request.OperationName = operationNameFromQuery + } + + variablesFromQuery := req.URL.Query().Get("variables") + if variablesFromQuery != "" { + var variables map[string]any + if err := json.Unmarshal([]byte(variablesFromQuery), &variables); err != nil { + responseJSON(rw, http.StatusBadRequest, errorResponse{err}) + return + } + request.Variables = variables + } + case req.Body != nil: if err := requestJSON(req, &request); err != nil { responseJSON(rw, http.StatusBadRequest, errorResponse{err}) @@ -294,7 +310,6 @@ func (s *storeHandler) ExecRequest(rw http.ResponseWriter, req *http.Request) { responseJSON(rw, http.StatusBadRequest, errorResponse{ErrMissingRequest}) return } - var options []client.RequestOption if request.OperationName != "" { options = append(options, client.WithOperationName(request.OperationName)) diff --git a/http/handler_store_test.go b/http/handler_store_test.go index dabf9648bd..d0f7e1e323 100644 --- a/http/handler_store_test.go +++ b/http/handler_store_test.go @@ -16,6 +16,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "testing" "github.com/stretchr/testify/assert" @@ -93,3 +94,79 @@ func TestExecRequest_WithInvalidQuery_HasSpecCompliantErrors(t *testing.T) { "message": "Cannot query field \"invalid\" on type \"User\".", }}) } + +func TestExecRequest_WithValidQuery_HttpGet_WithOperationName_OmitsErrors(t *testing.T) { + cdb := setupDatabase(t) + + query := `query { + User { + name + } + }` + operationName := "UserQuery" + + encodedQuery := url.QueryEscape(query) + encodedOperationName := url.QueryEscape(operationName) + + endpointURL := "http://localhost:9181/api/v0/graphql?query=" + encodedQuery + "&operationName=" + encodedOperationName + + req := httptest.NewRequest(http.MethodGet, endpointURL, nil) + rec := httptest.NewRecorder() + + handler, err := NewHandler(cdb) + require.NoError(t, err) + handler.ServeHTTP(rec, req) + + res := rec.Result() + require.NotNil(t, res.Body) + + resData, err := io.ReadAll(res.Body) + require.NoError(t, err) + + var gqlResponse map[string]any + err = json.Unmarshal(resData, &gqlResponse) + require.NoError(t, err) + + // errors should be omitted + _, ok := gqlResponse["errors"] + assert.False(t, ok) +} + +func TestExecRequest_HttpGet_WithVariables_OmitsErrors(t *testing.T) { + cdb := setupDatabase(t) + + query := `query getUser($filter: UserFilterArg) { + User(filter: $filter) { + name + } + }` + operationName := "getUser" + variables := `{"filter":{"name":{"_eq":"bob"}}}` + + encodedQuery := url.QueryEscape(query) + encodedOperationName := url.QueryEscape(operationName) + encodedVariables := url.QueryEscape(variables) + + endpointURL := "http://localhost:9181/api/v0/graphql?query=" + encodedQuery + "&operationName=" + encodedOperationName + "&variables=" + encodedVariables + + req := httptest.NewRequest(http.MethodGet, endpointURL, nil) + rec := httptest.NewRecorder() + + handler, err := NewHandler(cdb) + require.NoError(t, err) + handler.ServeHTTP(rec, req) + + res := rec.Result() + require.NotNil(t, res.Body) + + resData, err := io.ReadAll(res.Body) + require.NoError(t, err) + + var gqlResponse map[string]any + err = json.Unmarshal(resData, &gqlResponse) + require.NoError(t, err) + + // errors should be omitted + _, ok := gqlResponse["errors"] + assert.False(t, ok) +} diff --git a/playground/package-lock.json b/playground/package-lock.json index 42c3faa7aa..cc23b9c4e7 100644 --- a/playground/package-lock.json +++ b/playground/package-lock.json @@ -59,37 +59,6 @@ "integrity": "sha512-hPYRrKFoI+nuckPgDJfyYAkybFvheo4usS0Vw0HNAe+fmGBQA5Az37b/yStO284atBoqqdOUhKJ3d9Zw3PQkcQ==", "license": "MIT" }, - "node_modules/@codemirror/language": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/@codemirror/language/-/language-6.0.0.tgz", - "integrity": "sha512-rtjk5ifyMzOna1c7PBu7J1VCt0PvA5wy3o8eMVnxMKb7z8KA7JFecvD04dSn14vj/bBaAbqRsGed5OjtofEnLA==", - "peer": true, - "dependencies": { - "@codemirror/state": "^6.0.0", - "@codemirror/view": "^6.0.0", - "@lezer/common": "^1.0.0", - "@lezer/highlight": "^1.0.0", - "@lezer/lr": "^1.0.0", - "style-mod": "^4.0.0" - } - }, - "node_modules/@codemirror/state": { - "version": "6.4.1", - "resolved": "https://registry.npmjs.org/@codemirror/state/-/state-6.4.1.tgz", - "integrity": "sha512-QkEyUiLhsJoZkbumGZlswmAhA7CBU02Wrz7zvH4SrcifbsqwlXShVXg65f3v/ts57W3dqyamEriMhij1Z3Zz4A==", - "peer": true - }, - "node_modules/@codemirror/view": { - "version": "6.35.0", - "resolved": "https://registry.npmjs.org/@codemirror/view/-/view-6.35.0.tgz", - "integrity": "sha512-I0tYy63q5XkaWsJ8QRv5h6ves7kvtrBWjBcnf/bzohFJQc5c14a1AQRdE8QpPF9eMp5Mq2FMm59TCj1gDfE7kw==", - "peer": true, - "dependencies": { - "@codemirror/state": "^6.4.0", - "style-mod": "^4.1.0", - "w3c-keyname": "^2.2.4" - } - }, "node_modules/@emotion/is-prop-valid": { "version": "0.8.8", "resolved": "https://registry.npmjs.org/@emotion/is-prop-valid/-/is-prop-valid-0.8.8.tgz", @@ -818,30 +787,6 @@ "url": "https://github.com/sponsors/nzakas" } }, - "node_modules/@lezer/common": { - "version": "1.2.3", - "resolved": "https://registry.npmjs.org/@lezer/common/-/common-1.2.3.tgz", - "integrity": "sha512-w7ojc8ejBqr2REPsWxJjrMFsA/ysDCFICn8zEOR9mrqzOu2amhITYuLD8ag6XZf0CFXDrhKqw7+tW8cX66NaDA==", - "peer": true - }, - "node_modules/@lezer/highlight": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/@lezer/highlight/-/highlight-1.2.1.tgz", - "integrity": "sha512-Z5duk4RN/3zuVO7Jq0pGLJ3qynpxUVsh7IbUbGj88+uV2ApSAn6kWg2au3iJb+0Zi7kKtqffIESgNcRXWZWmSA==", - "peer": true, - "dependencies": { - "@lezer/common": "^1.0.0" - } - }, - "node_modules/@lezer/lr": { - "version": "1.4.2", - "resolved": "https://registry.npmjs.org/@lezer/lr/-/lr-1.4.2.tgz", - "integrity": "sha512-pu0K1jCIdnQ12aWNaAVU5bzi7Bd1w54J3ECgANPmYLtQKP0HBj2cE/5coBD66MT10xbtIuUr7tg0Shbsvk0mDA==", - "peer": true, - "dependencies": { - "@lezer/common": "^1.0.0" - } - }, "node_modules/@motionone/animation": { "version": "10.18.0", "resolved": "https://registry.npmjs.org/@motionone/animation/-/animation-10.18.0.tgz", @@ -2658,7 +2603,7 @@ "version": "15.7.13", "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.13.tgz", "integrity": "sha512-hCZTSvwbzWGvhqxp/RqVqwU999pBf2vp7hzIjiYOsl8wqOmUxkQ6ddw1cV3l8811+kdUFus/q4d1Y3E3SyEifA==", - "devOptional": true, + "dev": true, "license": "MIT" }, "node_modules/@types/ramda": { @@ -2674,7 +2619,7 @@ "version": "18.3.12", "resolved": "https://registry.npmjs.org/@types/react/-/react-18.3.12.tgz", "integrity": "sha512-D2wOSq/d6Agt28q7rSI3jhU7G6aiuzljDGZ2hTZHIkrTLUI+AF3WMeKkEZ9nN2fkBAlcktT6vcZjDFiIhMYEQw==", - "devOptional": true, + "dev": true, "license": "MIT", "dependencies": { "@types/prop-types": "*", @@ -2685,7 +2630,7 @@ "version": "18.3.1", "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-18.3.1.tgz", "integrity": "sha512-qW1Mfv8taImTthu4KoXgDfLuk4bydU6Q/TkADnDWWHwi4NX4BR+LWfTp2sVmTqRrsHvyDDTelgelxJ+SsejKKQ==", - "devOptional": true, + "dev": true, "license": "MIT", "dependencies": { "@types/react": "*" @@ -3310,7 +3255,7 @@ "version": "3.1.3", "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.1.3.tgz", "integrity": "sha512-M1uQkMl8rQK/szD0LNhtqxIPLpimGm8sOBwU7lLnCpSbTyY3yeU1Vc7l4KT5zT4s/yOxHH5O7tIuuLOCnLADRw==", - "devOptional": true, + "dev": true, "license": "MIT" }, "node_modules/debounce-promise": { @@ -5512,12 +5457,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/style-mod": { - "version": "4.1.2", - "resolved": "https://registry.npmjs.org/style-mod/-/style-mod-4.1.2.tgz", - "integrity": "sha512-wnD1HyVqpJUI2+eKZ+eo1UwghftP6yuFheBqqe+bWCotBjC2K1YnteJILRMs3SM4V/0dLEW1SC27MWP5y+mwmw==", - "peer": true - }, "node_modules/style-value-types": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/style-value-types/-/style-value-types-5.0.0.tgz", @@ -5904,12 +5843,6 @@ "integrity": "sha512-Ld1VelNuX9pdF39h2Hgaeb5hEZM2Z3jUrrMgWQAu82jMtZp7p3vJT3BzToKtZI7NgQssZje5o0zryOrhQvzQAg==", "license": "MIT" }, - "node_modules/w3c-keyname": { - "version": "2.2.8", - "resolved": "https://registry.npmjs.org/w3c-keyname/-/w3c-keyname-2.2.8.tgz", - "integrity": "sha512-dpojBhNsCNN7T82Tm7k26A6G9ML3NkhDsnw9n/eoxSRlVBB4CEtIQ/KTCLI2Fwf3ataSXRhYFkQi3SlnFwPvPQ==", - "peer": true - }, "node_modules/web-streams-polyfill": { "version": "3.3.3", "resolved": "https://registry.npmjs.org/web-streams-polyfill/-/web-streams-polyfill-3.3.3.tgz", From 80f06618183b7faf08fd00630c922b12cdcacd7e Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Wed, 4 Dec 2024 17:05:57 -0500 Subject: [PATCH 02/25] Fixed one unit test --- http/handler_store_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/http/handler_store_test.go b/http/handler_store_test.go index d0f7e1e323..697e156111 100644 --- a/http/handler_store_test.go +++ b/http/handler_store_test.go @@ -98,7 +98,7 @@ func TestExecRequest_WithInvalidQuery_HasSpecCompliantErrors(t *testing.T) { func TestExecRequest_WithValidQuery_HttpGet_WithOperationName_OmitsErrors(t *testing.T) { cdb := setupDatabase(t) - query := `query { + query := `query UserQuery { User { name } @@ -123,6 +123,8 @@ func TestExecRequest_WithValidQuery_HttpGet_WithOperationName_OmitsErrors(t *tes resData, err := io.ReadAll(res.Body) require.NoError(t, err) + t.Log(string(resData)) + var gqlResponse map[string]any err = json.Unmarshal(resData, &gqlResponse) require.NoError(t, err) From 76273aee74aff24a28c199286b61568bbf608278 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Thu, 5 Dec 2024 10:24:26 -0500 Subject: [PATCH 03/25] Feedback changes --- http/handler_store.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/http/handler_store.go b/http/handler_store.go index 9d1bd037c8..a5820b7a21 100644 --- a/http/handler_store.go +++ b/http/handler_store.go @@ -286,10 +286,7 @@ func (s *storeHandler) ExecRequest(rw http.ResponseWriter, req *http.Request) { case req.URL.Query().Get("query") != "": request.Query = req.URL.Query().Get("query") - operationNameFromQuery := req.URL.Query().Get("operationName") - if operationNameFromQuery != "" { - request.OperationName = operationNameFromQuery - } + req.URL.Query().Get("operationName") variablesFromQuery := req.URL.Query().Get("variables") if variablesFromQuery != "" { From c8f70a168fd116132b30f5e160a7443655d04541 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Thu, 5 Dec 2024 10:24:51 -0500 Subject: [PATCH 04/25] Feedback changes --- http/handler_store_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/http/handler_store_test.go b/http/handler_store_test.go index 697e156111..e548cbc5ab 100644 --- a/http/handler_store_test.go +++ b/http/handler_store_test.go @@ -95,7 +95,7 @@ func TestExecRequest_WithInvalidQuery_HasSpecCompliantErrors(t *testing.T) { }}) } -func TestExecRequest_WithValidQuery_HttpGet_WithOperationName_OmitsErrors(t *testing.T) { +func TestExecRequest_HttpGet_WithOperationName(t *testing.T) { cdb := setupDatabase(t) query := `query UserQuery { @@ -134,7 +134,7 @@ func TestExecRequest_WithValidQuery_HttpGet_WithOperationName_OmitsErrors(t *tes assert.False(t, ok) } -func TestExecRequest_HttpGet_WithVariables_OmitsErrors(t *testing.T) { +func TestExecRequest_HttpGet_WithVariables(t *testing.T) { cdb := setupDatabase(t) query := `query getUser($filter: UserFilterArg) { From 462ee5f73b73b807d66dbd712408361dbb0cf7f1 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Thu, 5 Dec 2024 10:29:25 -0500 Subject: [PATCH 05/25] Roll back package-lock.json in playground --- playground/package-lock.json | 77 +++++++++++++++++++++++++++++++++--- 1 file changed, 72 insertions(+), 5 deletions(-) diff --git a/playground/package-lock.json b/playground/package-lock.json index cc23b9c4e7..4d1354f9fe 100644 --- a/playground/package-lock.json +++ b/playground/package-lock.json @@ -59,6 +59,37 @@ "integrity": "sha512-hPYRrKFoI+nuckPgDJfyYAkybFvheo4usS0Vw0HNAe+fmGBQA5Az37b/yStO284atBoqqdOUhKJ3d9Zw3PQkcQ==", "license": "MIT" }, + "node_modules/@codemirror/language": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@codemirror/language/-/language-6.0.0.tgz", + "integrity": "sha512-rtjk5ifyMzOna1c7PBu7J1VCt0PvA5wy3o8eMVnxMKb7z8KA7JFecvD04dSn14vj/bBaAbqRsGed5OjtofEnLA==", + "peer": true, + "dependencies": { + "@codemirror/state": "^6.0.0", + "@codemirror/view": "^6.0.0", + "@lezer/common": "^1.0.0", + "@lezer/highlight": "^1.0.0", + "@lezer/lr": "^1.0.0", + "style-mod": "^4.0.0" + } + }, + "node_modules/@codemirror/state": { + "version": "6.4.1", + "resolved": "https://registry.npmjs.org/@codemirror/state/-/state-6.4.1.tgz", + "integrity": "sha512-QkEyUiLhsJoZkbumGZlswmAhA7CBU02Wrz7zvH4SrcifbsqwlXShVXg65f3v/ts57W3dqyamEriMhij1Z3Zz4A==", + "peer": true + }, + "node_modules/@codemirror/view": { + "version": "6.35.0", + "resolved": "https://registry.npmjs.org/@codemirror/view/-/view-6.35.0.tgz", + "integrity": "sha512-I0tYy63q5XkaWsJ8QRv5h6ves7kvtrBWjBcnf/bzohFJQc5c14a1AQRdE8QpPF9eMp5Mq2FMm59TCj1gDfE7kw==", + "peer": true, + "dependencies": { + "@codemirror/state": "^6.4.0", + "style-mod": "^4.1.0", + "w3c-keyname": "^2.2.4" + } + }, "node_modules/@emotion/is-prop-valid": { "version": "0.8.8", "resolved": "https://registry.npmjs.org/@emotion/is-prop-valid/-/is-prop-valid-0.8.8.tgz", @@ -787,6 +818,30 @@ "url": "https://github.com/sponsors/nzakas" } }, + "node_modules/@lezer/common": { + "version": "1.2.3", + "resolved": "https://registry.npmjs.org/@lezer/common/-/common-1.2.3.tgz", + "integrity": "sha512-w7ojc8ejBqr2REPsWxJjrMFsA/ysDCFICn8zEOR9mrqzOu2amhITYuLD8ag6XZf0CFXDrhKqw7+tW8cX66NaDA==", + "peer": true + }, + "node_modules/@lezer/highlight": { + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/@lezer/highlight/-/highlight-1.2.1.tgz", + "integrity": "sha512-Z5duk4RN/3zuVO7Jq0pGLJ3qynpxUVsh7IbUbGj88+uV2ApSAn6kWg2au3iJb+0Zi7kKtqffIESgNcRXWZWmSA==", + "peer": true, + "dependencies": { + "@lezer/common": "^1.0.0" + } + }, + "node_modules/@lezer/lr": { + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/@lezer/lr/-/lr-1.4.2.tgz", + "integrity": "sha512-pu0K1jCIdnQ12aWNaAVU5bzi7Bd1w54J3ECgANPmYLtQKP0HBj2cE/5coBD66MT10xbtIuUr7tg0Shbsvk0mDA==", + "peer": true, + "dependencies": { + "@lezer/common": "^1.0.0" + } + }, "node_modules/@motionone/animation": { "version": "10.18.0", "resolved": "https://registry.npmjs.org/@motionone/animation/-/animation-10.18.0.tgz", @@ -2603,7 +2658,7 @@ "version": "15.7.13", "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.13.tgz", "integrity": "sha512-hCZTSvwbzWGvhqxp/RqVqwU999pBf2vp7hzIjiYOsl8wqOmUxkQ6ddw1cV3l8811+kdUFus/q4d1Y3E3SyEifA==", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/@types/ramda": { @@ -2619,7 +2674,7 @@ "version": "18.3.12", "resolved": "https://registry.npmjs.org/@types/react/-/react-18.3.12.tgz", "integrity": "sha512-D2wOSq/d6Agt28q7rSI3jhU7G6aiuzljDGZ2hTZHIkrTLUI+AF3WMeKkEZ9nN2fkBAlcktT6vcZjDFiIhMYEQw==", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "@types/prop-types": "*", @@ -2630,7 +2685,7 @@ "version": "18.3.1", "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-18.3.1.tgz", "integrity": "sha512-qW1Mfv8taImTthu4KoXgDfLuk4bydU6Q/TkADnDWWHwi4NX4BR+LWfTp2sVmTqRrsHvyDDTelgelxJ+SsejKKQ==", - "dev": true, + "devOptional": true, "license": "MIT", "dependencies": { "@types/react": "*" @@ -3255,7 +3310,7 @@ "version": "3.1.3", "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.1.3.tgz", "integrity": "sha512-M1uQkMl8rQK/szD0LNhtqxIPLpimGm8sOBwU7lLnCpSbTyY3yeU1Vc7l4KT5zT4s/yOxHH5O7tIuuLOCnLADRw==", - "dev": true, + "devOptional": true, "license": "MIT" }, "node_modules/debounce-promise": { @@ -5457,6 +5512,12 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/style-mod": { + "version": "4.1.2", + "resolved": "https://registry.npmjs.org/style-mod/-/style-mod-4.1.2.tgz", + "integrity": "sha512-wnD1HyVqpJUI2+eKZ+eo1UwghftP6yuFheBqqe+bWCotBjC2K1YnteJILRMs3SM4V/0dLEW1SC27MWP5y+mwmw==", + "peer": true + }, "node_modules/style-value-types": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/style-value-types/-/style-value-types-5.0.0.tgz", @@ -5843,6 +5904,12 @@ "integrity": "sha512-Ld1VelNuX9pdF39h2Hgaeb5hEZM2Z3jUrrMgWQAu82jMtZp7p3vJT3BzToKtZI7NgQssZje5o0zryOrhQvzQAg==", "license": "MIT" }, + "node_modules/w3c-keyname": { + "version": "2.2.8", + "resolved": "https://registry.npmjs.org/w3c-keyname/-/w3c-keyname-2.2.8.tgz", + "integrity": "sha512-dpojBhNsCNN7T82Tm7k26A6G9ML3NkhDsnw9n/eoxSRlVBB4CEtIQ/KTCLI2Fwf3ataSXRhYFkQi3SlnFwPvPQ==", + "peer": true + }, "node_modules/web-streams-polyfill": { "version": "3.3.3", "resolved": "https://registry.npmjs.org/web-streams-polyfill/-/web-streams-polyfill-3.3.3.tgz", @@ -5929,4 +5996,4 @@ "license": "Unlicense" } } -} +} \ No newline at end of file From 942ae7fb917e811ba2165b1703a470e0108a312f Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Thu, 5 Dec 2024 10:40:37 -0500 Subject: [PATCH 06/25] Adjust white space --- playground/package-lock.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playground/package-lock.json b/playground/package-lock.json index 4d1354f9fe..42c3faa7aa 100644 --- a/playground/package-lock.json +++ b/playground/package-lock.json @@ -5996,4 +5996,4 @@ "license": "Unlicense" } } -} \ No newline at end of file +} From 169fbd1503fcedc0bb0337f2bdec68fe082a1aea Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Thu, 5 Dec 2024 11:23:25 -0500 Subject: [PATCH 07/25] Removed extraneous line of code --- http/handler_store.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/http/handler_store.go b/http/handler_store.go index a5820b7a21..990a623a62 100644 --- a/http/handler_store.go +++ b/http/handler_store.go @@ -286,8 +286,6 @@ func (s *storeHandler) ExecRequest(rw http.ResponseWriter, req *http.Request) { case req.URL.Query().Get("query") != "": request.Query = req.URL.Query().Get("query") - req.URL.Query().Get("operationName") - variablesFromQuery := req.URL.Query().Get("variables") if variablesFromQuery != "" { var variables map[string]any From f6a0eb804a05a48c78eb863b05320a240063af5a Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Thu, 5 Dec 2024 14:33:32 -0500 Subject: [PATCH 08/25] Major fixes; Changed tests --- http/handler_ccip_test.go | 10 ++++++++-- http/handler_store.go | 3 +++ http/handler_store_test.go | 36 ++++++++++++++++++++++++++++++++---- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/http/handler_ccip_test.go b/http/handler_ccip_test.go index 43797b622d..1888effe37 100644 --- a/http/handler_ccip_test.go +++ b/http/handler_ccip_test.go @@ -67,7 +67,7 @@ func TestCCIPGet_WithValidData(t *testing.T) { resHex, err := hex.DecodeString(strings.TrimPrefix(ccipRes.Data, "0x")) require.NoError(t, err) - assert.JSONEq(t, `{"data": {"User": [{"name": "bob"}]}}`, string(resHex)) + assert.JSONEq(t, `{"data": {"User": [{"name": "bob"}, {"name": "adam"}]}}`, string(resHex)) } func TestCCIPGet_WithSubscription(t *testing.T) { @@ -153,7 +153,7 @@ func TestCCIPPost_WithValidData(t *testing.T) { resHex, err := hex.DecodeString(strings.TrimPrefix(ccipRes.Data, "0x")) require.NoError(t, err) - assert.JSONEq(t, `{"data": {"User": [{"name": "bob"}]}}`, string(resHex)) + assert.JSONEq(t, `{"data": {"User": [{"name": "bob"}, {"name": "adam"}]}}`, string(resHex)) } func TestCCIPPost_WithInvalidGraphQLRequest(t *testing.T) { @@ -210,5 +210,11 @@ func setupDatabase(t *testing.T) client.DB { err = col.Create(ctx, doc) require.NoError(t, err) + doc2, err := client.NewDocFromJSON([]byte(`{"name": "adam"}`), col.Definition()) + require.NoError(t, err) + + err = col.Create(ctx, doc2) + require.NoError(t, err) + return cdb } diff --git a/http/handler_store.go b/http/handler_store.go index 990a623a62..3d2cef63de 100644 --- a/http/handler_store.go +++ b/http/handler_store.go @@ -284,8 +284,11 @@ func (s *storeHandler) ExecRequest(rw http.ResponseWriter, req *http.Request) { var request GraphQLRequest switch { case req.URL.Query().Get("query") != "": + request.Query = req.URL.Query().Get("query") + request.OperationName = req.URL.Query().Get("operationName") + variablesFromQuery := req.URL.Query().Get("variables") if variablesFromQuery != "" { var variables map[string]any diff --git a/http/handler_store_test.go b/http/handler_store_test.go index e548cbc5ab..71a1d8b4e3 100644 --- a/http/handler_store_test.go +++ b/http/handler_store_test.go @@ -98,11 +98,19 @@ func TestExecRequest_WithInvalidQuery_HasSpecCompliantErrors(t *testing.T) { func TestExecRequest_HttpGet_WithOperationName(t *testing.T) { cdb := setupDatabase(t) - query := `query UserQuery { + query := ` + query UserQuery { User { name } - }` + } + query UserQueryWithDocID { + User { + _docID + name + } + } + ` operationName := "UserQuery" encodedQuery := url.QueryEscape(query) @@ -123,8 +131,6 @@ func TestExecRequest_HttpGet_WithOperationName(t *testing.T) { resData, err := io.ReadAll(res.Body) require.NoError(t, err) - t.Log(string(resData)) - var gqlResponse map[string]any err = json.Unmarshal(resData, &gqlResponse) require.NoError(t, err) @@ -132,6 +138,18 @@ func TestExecRequest_HttpGet_WithOperationName(t *testing.T) { // errors should be omitted _, ok := gqlResponse["errors"] assert.False(t, ok) + + // Compare the response data to the expected output + expectedJSON := `{ + "data": { + "User": [ + {"name": "bob"}, + {"name": "adam"} + ] + } + }` + assert.JSONEq(t, expectedJSON, string(resData)) + } func TestExecRequest_HttpGet_WithVariables(t *testing.T) { @@ -171,4 +189,14 @@ func TestExecRequest_HttpGet_WithVariables(t *testing.T) { // errors should be omitted _, ok := gqlResponse["errors"] assert.False(t, ok) + + // Compare the response data to the expected output + expectedJSON := `{ + "data": { + "User": [ + {"name": "bob"} + ] + } + }` + assert.JSONEq(t, expectedJSON, string(resData)) } From 5afefe6b4db5094a7aa568e3cdc57c88a8e61549 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Thu, 5 Dec 2024 15:14:04 -0500 Subject: [PATCH 09/25] Improve comments, remove error checks --- http/handler_store_test.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/http/handler_store_test.go b/http/handler_store_test.go index 71a1d8b4e3..f2d1993745 100644 --- a/http/handler_store_test.go +++ b/http/handler_store_test.go @@ -135,11 +135,7 @@ func TestExecRequest_HttpGet_WithOperationName(t *testing.T) { err = json.Unmarshal(resData, &gqlResponse) require.NoError(t, err) - // errors should be omitted - _, ok := gqlResponse["errors"] - assert.False(t, ok) - - // Compare the response data to the expected output + // Ensure the response data contains names, but not the _docID field expectedJSON := `{ "data": { "User": [ @@ -186,11 +182,7 @@ func TestExecRequest_HttpGet_WithVariables(t *testing.T) { err = json.Unmarshal(resData, &gqlResponse) require.NoError(t, err) - // errors should be omitted - _, ok := gqlResponse["errors"] - assert.False(t, ok) - - // Compare the response data to the expected output + // Ensure only bob is returned, because of the filter variable expectedJSON := `{ "data": { "User": [ From 5a6078b55d47f0950fcdc0da2fd1b1feb440b48a Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Thu, 5 Dec 2024 15:22:03 -0500 Subject: [PATCH 10/25] Delint --- http/handler_store_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/http/handler_store_test.go b/http/handler_store_test.go index f2d1993745..7d39c4be07 100644 --- a/http/handler_store_test.go +++ b/http/handler_store_test.go @@ -145,7 +145,6 @@ func TestExecRequest_HttpGet_WithOperationName(t *testing.T) { } }` assert.JSONEq(t, expectedJSON, string(resData)) - } func TestExecRequest_HttpGet_WithVariables(t *testing.T) { From 0872e94a10c5a2eaa91af5f1dee63a03456fa9e4 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Fri, 6 Dec 2024 12:23:22 -0500 Subject: [PATCH 11/25] First commit --- cli/start.go | 1 + http/client.go | 23 ++++++++++++++++++++++- http/handler.go | 17 +++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/cli/start.go b/cli/start.go index 0bd1510008..ad2f79c928 100644 --- a/cli/start.go +++ b/cli/start.go @@ -135,6 +135,7 @@ func MakeStartCommand() *cobra.Command { } isDevMode := cfg.GetBool("development") + http.IsDevMode = isDevMode if isDevMode { cmd.Printf(devModeBanner) if cfg.GetBool("keyring.disabled") { diff --git a/http/client.go b/http/client.go index 2e57c017da..63989141d9 100644 --- a/http/client.go +++ b/http/client.go @@ -14,6 +14,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "io" "net/http" "net/url" @@ -470,7 +471,27 @@ func (c *Client) PrintDump(ctx context.Context) error { func (c *Client) Purge(ctx context.Context) error { methodURL := c.http.baseURL.JoinPath("purge") - req, err := http.NewRequestWithContext(ctx, http.MethodPost, methodURL.String(), nil) + // The devmode flag can be obtained by querying the /devmode/ endpoint + suffixURL := "/api/" + Version + devModeURL := strings.TrimSuffix(c.http.baseURL.String(), suffixURL) + "/devmode/" + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, devModeURL, nil) + if err != nil { + return err + } + var isDevMode bool + if err := c.http.requestJson(req, &isDevMode); err != nil { + return err + } + + if !isDevMode { + // The following is not an error and does not return, because the command still + // needs to run, such that the node receives the request and catches the error on that + // side of things. + fmt.Println("Error:this command can only be run in development mode") + } + + req, err = http.NewRequestWithContext(ctx, http.MethodPost, methodURL.String(), nil) if err != nil { return err } diff --git a/http/handler.go b/http/handler.go index 336dfc54d3..32e586a1e8 100644 --- a/http/handler.go +++ b/http/handler.go @@ -12,6 +12,7 @@ package http import ( "context" + "fmt" "net/http" "sync" @@ -21,6 +22,9 @@ import ( "github.com/go-chi/chi/v5" ) +// Global variable for the development mode flag +var IsDevMode bool = false + // Version is the identifier for the current API version. var Version string = "v0" @@ -70,6 +74,13 @@ type Handler struct { txs *sync.Map } +// Helper function for exposing the development mode flag +func responseHTML(w http.ResponseWriter, status int, html string) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(status) + w.Write([]byte(html)) +} + func NewHandler(db client.DB) (*Handler, error) { router, err := NewApiRouter() if err != nil { @@ -88,6 +99,12 @@ func NewHandler(db client.DB) (*Handler, error) { mux.Get("/openapi.json", func(rw http.ResponseWriter, req *http.Request) { responseJSON(rw, http.StatusOK, router.OpenAPI()) }) + + // Expose whether or not the server is running in development mode + mux.Get("/devmode/", func(rw http.ResponseWriter, req *http.Request) { + responseHTML(rw, http.StatusOK, fmt.Sprintf("%v", IsDevMode)) + }) + mux.Handle("/*", playgroundHandler) return &Handler{ db: db, From 96dd91fa9200d84430b3ed0966cfc09b97b689d3 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Fri, 6 Dec 2024 15:15:33 -0500 Subject: [PATCH 12/25] helpful comment --- http/handler_extras.go | 1 + 1 file changed, 1 insertion(+) diff --git a/http/handler_extras.go b/http/handler_extras.go index 50b4cfb99e..29622f6bb5 100644 --- a/http/handler_extras.go +++ b/http/handler_extras.go @@ -25,6 +25,7 @@ func (s *extrasHandler) Purge(rw http.ResponseWriter, req *http.Request) { db := mustGetContextClientDB(req) + // Send either 200 or 400 response based on whether the server is in dev mode if IsDevMode { rw.WriteHeader(http.StatusOK) } else { From 94ca9d18800e6fdaa742e8f1876fd6e68c8e8e4f Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Fri, 6 Dec 2024 15:18:14 -0500 Subject: [PATCH 13/25] Delint --- http/handler_extras.go | 1 - 1 file changed, 1 deletion(-) diff --git a/http/handler_extras.go b/http/handler_extras.go index 29622f6bb5..67cba8d0ef 100644 --- a/http/handler_extras.go +++ b/http/handler_extras.go @@ -22,7 +22,6 @@ import ( type extrasHandler struct{} func (s *extrasHandler) Purge(rw http.ResponseWriter, req *http.Request) { - db := mustGetContextClientDB(req) // Send either 200 or 400 response based on whether the server is in dev mode From 50c9fa96a76530639f9d666a2b54630822c54e28 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Fri, 6 Dec 2024 15:30:13 -0500 Subject: [PATCH 14/25] Clean up white space and comments --- http/client.go | 1 - http/handler.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/http/client.go b/http/client.go index ae54f027ed..2e57c017da 100644 --- a/http/client.go +++ b/http/client.go @@ -474,7 +474,6 @@ func (c *Client) Purge(ctx context.Context) error { if err != nil { return err } - _, err = c.http.request(req) return err } diff --git a/http/handler.go b/http/handler.go index 184860983c..d057ffb178 100644 --- a/http/handler.go +++ b/http/handler.go @@ -22,7 +22,8 @@ import ( ) // Global variable for the development mode flag -// This is used to expose the development mode flag to the frontend at url:port/devmode/ +// This is checked by the http/handler_extras.go/Purge function to determine which response to send +// to the client. var IsDevMode bool = false // Version is the identifier for the current API version. @@ -92,7 +93,6 @@ func NewHandler(db client.DB) (*Handler, error) { mux.Get("/openapi.json", func(rw http.ResponseWriter, req *http.Request) { responseJSON(rw, http.StatusOK, router.OpenAPI()) }) - mux.Handle("/*", playgroundHandler) return &Handler{ db: db, From 12aee9627caffece72c4b7f03c39af4f78edbfc1 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Fri, 6 Dec 2024 15:31:24 -0500 Subject: [PATCH 15/25] Changed comment --- http/handler.go | 1 - 1 file changed, 1 deletion(-) diff --git a/http/handler.go b/http/handler.go index d057ffb178..7d24245ea4 100644 --- a/http/handler.go +++ b/http/handler.go @@ -23,7 +23,6 @@ import ( // Global variable for the development mode flag // This is checked by the http/handler_extras.go/Purge function to determine which response to send -// to the client. var IsDevMode bool = false // Version is the identifier for the current API version. From 4d8fea4da40cacc4705f13e6a8755881953eb0e1 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Fri, 6 Dec 2024 16:53:54 -0500 Subject: [PATCH 16/25] Adjusted tests --- http/handler_extras_test.go | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/http/handler_extras_test.go b/http/handler_extras_test.go index d7d1398e90..870cad02d3 100644 --- a/http/handler_extras_test.go +++ b/http/handler_extras_test.go @@ -20,8 +20,11 @@ import ( "github.com/stretchr/testify/require" ) -func TestPurge(t *testing.T) { +func TestPurgeDevModeTrue(t *testing.T) { cdb := setupDatabase(t) + + IsDevMode = true + url := "http://localhost:9181/api/v0/purge" req := httptest.NewRequest(http.MethodPost, url, nil) @@ -40,3 +43,27 @@ func TestPurge(t *testing.T) { // test will timeout if purge never received <-purgeSub.Message() } + +func TestPurgeDevModeFalse(t *testing.T) { + cdb := setupDatabase(t) + + IsDevMode = false + + url := "http://localhost:9181/api/v0/purge" + + req := httptest.NewRequest(http.MethodPost, url, nil) + rec := httptest.NewRecorder() + + purgeSub, err := cdb.Events().Subscribe(event.PurgeName) + require.NoError(t, err) + + handler, err := NewHandler(cdb) + require.NoError(t, err) + handler.ServeHTTP(rec, req) + + res := rec.Result() + require.Equal(t, 400, res.StatusCode) + + // test will timeout if purge never received + <-purgeSub.Message() +} From 6d103ced0513954369de80a08cbfd5d50cf15445 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Mon, 16 Dec 2024 12:54:58 -0500 Subject: [PATCH 17/25] First commit --- cli/cli.go | 1 + cli/keyring_list.go | 44 +++++++++++++++++++++++++++++++++++++++ internal/planner/group.go | 8 ++++--- keyring/file.go | 24 +++++++++++++++++++++ keyring/keyring.go | 2 ++ keyring/system.go | 6 ++++++ 6 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 cli/keyring_list.go diff --git a/cli/cli.go b/cli/cli.go index 8f9d3fcbd1..faeaa26d1f 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -137,6 +137,7 @@ func NewDefraCommand() *cobra.Command { MakeKeyringGenerateCommand(), MakeKeyringImportCommand(), MakeKeyringExportCommand(), + MakeKeyringListCommand(), ) identity := MakeIdentityCommand() diff --git a/cli/keyring_list.go b/cli/keyring_list.go new file mode 100644 index 0000000000..c17ccd23a6 --- /dev/null +++ b/cli/keyring_list.go @@ -0,0 +1,44 @@ +package cli + +import ( + "github.com/spf13/cobra" +) + +// MakeKeyringListCommand creates a new command to list all keys in the keyring. +func MakeKeyringListCommand() *cobra.Command { + var cmd = &cobra.Command{ + Use: "list", + Short: "List all keys in the keyring", + Long: `List all keys in the keyring. +The DEFRA_KEYRING_SECRET environment variable must be set to unlock the keyring. +This can also be done with a .env file in the working directory or at a path +defined with the --secret-file flag. + +Example: + defradb keyring list`, + Args: cobra.NoArgs, // No arguments expected + RunE: func(cmd *cobra.Command, args []string) error { + keyring, err := openKeyring(cmd) + if err != nil { + return err + } + + keyNames, err := keyring.List() + if err != nil { + return err + } + + if len(keyNames) == 0 { + cmd.Println("No keys found in the keyring.") + return nil + } + + cmd.Println("Keys in the keyring:") + for _, keyName := range keyNames { + cmd.Println("- " + keyName) + } + return nil + }, + } + return cmd +} diff --git a/internal/planner/group.go b/internal/planner/group.go index 900f0ff412..f4afe4a88f 100644 --- a/internal/planner/group.go +++ b/internal/planner/group.go @@ -11,6 +11,8 @@ package planner import ( + "fmt" + "github.com/sourcenetwork/defradb/client" "github.com/sourcenetwork/defradb/client/request" "github.com/sourcenetwork/defradb/internal/core" @@ -59,9 +61,6 @@ type groupExecInfo struct { } // Creates a new group node. - -// The function is recursive and will construct the node-chain for any child (`_group`) collections. -// `groupSelect` is optional and will typically be nil if the child `_group` is not requested. func (p *Planner) GroupBy(n *mapper.GroupBy, parsed *mapper.Select, childSelects []*mapper.Select) (*groupNode, error) { if n == nil { return nil, nil @@ -83,6 +82,9 @@ func (p *Planner) GroupBy(n *mapper.GroupBy, parsed *mapper.Select, childSelects // group by fields have to be propagated downwards to ensure correct sub-grouping, otherwise child // groups will only group on the fields they explicitly reference childSelect.GroupBy.Fields = append(childSelect.GroupBy.Fields, n.Fields...) + for _, field := range childSelect.GroupBy.Fields { + fmt.Println("field", field) + } } dataSources = append(dataSources, newDataSource(childSelect.Index)) } diff --git a/keyring/file.go b/keyring/file.go index a8f7532274..00c968ba41 100644 --- a/keyring/file.go +++ b/keyring/file.go @@ -64,3 +64,27 @@ func (f *fileKeyring) Delete(user string) error { } return err } + +// List is a method that lists all keys in the file-based keyring directory. +func (f *fileKeyring) List() ([]string, error) { + return listKeys(f.dir) +} + +// listKeys is a helper function to list all keys in the given keyring directory. +func listKeys(keyringDir string) ([]string, error) { + // Read all the files in the keyring directory + files, err := os.ReadDir(keyringDir) + if err != nil { + return nil, err // Return an error if the directory cannot be read + } + + // Extract file names + var keyNames []string + for _, file := range files { + if !file.IsDir() { // Only include files, not subdirectories + keyNames = append(keyNames, file.Name()) + } + } + + return keyNames, nil +} diff --git a/keyring/keyring.go b/keyring/keyring.go index 603c25bb78..f7fa5961a1 100644 --- a/keyring/keyring.go +++ b/keyring/keyring.go @@ -24,4 +24,6 @@ type Keyring interface { // // If a key with that name does not exist `ErrNotFound` is returned. Delete(name string) error + // List returns a list of all keys in the keyring, used by the CLI 'keyring list' command + List() ([]string, error) } diff --git a/keyring/system.go b/keyring/system.go index 81575b5501..46ee03e1ca 100644 --- a/keyring/system.go +++ b/keyring/system.go @@ -53,3 +53,9 @@ func (s *systemKeyring) Get(name string) ([]byte, error) { func (s *systemKeyring) Delete(user string) error { return keyring.Delete(s.service, user) } + +func (s *systemKeyring) List() ([]string, error) { + // List does not need to be returned here + // This function is a stub + return nil, nil +} From 662db5a7199828119eeb45828219be3c334a0960 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Mon, 16 Dec 2024 13:01:14 -0500 Subject: [PATCH 18/25] Removed changes to unrelated file --- internal/planner/group.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/planner/group.go b/internal/planner/group.go index f4afe4a88f..900f0ff412 100644 --- a/internal/planner/group.go +++ b/internal/planner/group.go @@ -11,8 +11,6 @@ package planner import ( - "fmt" - "github.com/sourcenetwork/defradb/client" "github.com/sourcenetwork/defradb/client/request" "github.com/sourcenetwork/defradb/internal/core" @@ -61,6 +59,9 @@ type groupExecInfo struct { } // Creates a new group node. + +// The function is recursive and will construct the node-chain for any child (`_group`) collections. +// `groupSelect` is optional and will typically be nil if the child `_group` is not requested. func (p *Planner) GroupBy(n *mapper.GroupBy, parsed *mapper.Select, childSelects []*mapper.Select) (*groupNode, error) { if n == nil { return nil, nil @@ -82,9 +83,6 @@ func (p *Planner) GroupBy(n *mapper.GroupBy, parsed *mapper.Select, childSelects // group by fields have to be propagated downwards to ensure correct sub-grouping, otherwise child // groups will only group on the fields they explicitly reference childSelect.GroupBy.Fields = append(childSelect.GroupBy.Fields, n.Fields...) - for _, field := range childSelect.GroupBy.Fields { - fmt.Println("field", field) - } } dataSources = append(dataSources, newDataSource(childSelect.Index)) } From 0b3a9b51bc60f8b85eeca7eceb972615436a7a8e Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Mon, 16 Dec 2024 13:05:40 -0500 Subject: [PATCH 19/25] Refactored List() function, updated comments --- cli/keyring_list.go | 2 +- keyring/file.go | 14 ++++---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/cli/keyring_list.go b/cli/keyring_list.go index c17ccd23a6..b9c1d1c8bc 100644 --- a/cli/keyring_list.go +++ b/cli/keyring_list.go @@ -16,7 +16,7 @@ defined with the --secret-file flag. Example: defradb keyring list`, - Args: cobra.NoArgs, // No arguments expected + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { keyring, err := openKeyring(cmd) if err != nil { diff --git a/keyring/file.go b/keyring/file.go index 00c968ba41..f1299d2f4d 100644 --- a/keyring/file.go +++ b/keyring/file.go @@ -65,23 +65,17 @@ func (f *fileKeyring) Delete(user string) error { return err } -// List is a method that lists all keys in the file-based keyring directory. func (f *fileKeyring) List() ([]string, error) { - return listKeys(f.dir) -} -// listKeys is a helper function to list all keys in the given keyring directory. -func listKeys(keyringDir string) ([]string, error) { - // Read all the files in the keyring directory - files, err := os.ReadDir(keyringDir) + files, err := os.ReadDir(f.dir) if err != nil { - return nil, err // Return an error if the directory cannot be read + return nil, err } - // Extract file names + // File names are key names var keyNames []string for _, file := range files { - if !file.IsDir() { // Only include files, not subdirectories + if !file.IsDir() { keyNames = append(keyNames, file.Name()) } } From 64a999c67981b70f3763121010151bcf1c20553a Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Mon, 16 Dec 2024 16:33:38 -0500 Subject: [PATCH 20/25] Unit test and delint/gofmt --- cli/keyring_list.go | 10 +++++++ cli/keyring_list_test.go | 65 ++++++++++++++++++++++++++++++++++++++++ keyring/file.go | 1 - 3 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 cli/keyring_list_test.go diff --git a/cli/keyring_list.go b/cli/keyring_list.go index b9c1d1c8bc..35e2a2482e 100644 --- a/cli/keyring_list.go +++ b/cli/keyring_list.go @@ -1,3 +1,13 @@ +// Copyright 2024 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + package cli import ( diff --git a/cli/keyring_list_test.go b/cli/keyring_list_test.go new file mode 100644 index 0000000000..9edbc65bc8 --- /dev/null +++ b/cli/keyring_list_test.go @@ -0,0 +1,65 @@ +// Copyright 2024 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package cli + +import ( + "bytes" + "encoding/hex" + "os" + "regexp" + "testing" + + "github.com/sourcenetwork/defradb/crypto" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestKeyringList(t *testing.T) { + rootdir := t.TempDir() + + err := os.Setenv("DEFRA_KEYRING_SECRET", "password") + require.NoError(t, err) + + keyNames := []string{"keyname1", "keyname2", "keyname3"} + + // Insert the keys into the keyring + for _, keyName := range keyNames { + keyBytes, err := crypto.GenerateAES256() + require.NoError(t, err) + keyHex := hex.EncodeToString(keyBytes) + cmd := NewDefraCommand() + cmd.SetArgs([]string{"keyring", "import", "--rootdir", rootdir, keyName, keyHex}) + err = cmd.Execute() + require.NoError(t, err) + } + + // Run the 'keyring list' command, and require no error on the output + var output bytes.Buffer + cmd := NewDefraCommand() + cmd.SetOut(&output) + cmd.SetArgs([]string{"keyring", "list", "--rootdir", rootdir}) + err = cmd.Execute() + require.NoError(t, err) + + outputString := output.String() + + // Use regex to extract the keys, and compare with the expected values + // We know the format the output should be, which is: "Keys in the keyring:\n- keyname1\n- keyname2\n- keyname3\n" + re := regexp.MustCompile(`-\s([^\n]+)`) + matches := re.FindAllStringSubmatch(outputString, -1) + var extractedKeys []string + for _, match := range matches { + extractedKeys = append(extractedKeys, match[1]) + } + + assert.ElementsMatch(t, keyNames, extractedKeys, "The extracted keys do not match the expected keys.") +} diff --git a/keyring/file.go b/keyring/file.go index f1299d2f4d..d78fe1322c 100644 --- a/keyring/file.go +++ b/keyring/file.go @@ -66,7 +66,6 @@ func (f *fileKeyring) Delete(user string) error { } func (f *fileKeyring) List() ([]string, error) { - files, err := os.ReadDir(f.dir) if err != nil { return nil, err From 1313d54372dda4264f4c815cd5782c18ab58d628 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Tue, 17 Dec 2024 09:31:11 -0500 Subject: [PATCH 21/25] Updated comments, print statements --- cli/keyring_list_test.go | 2 +- keyring/system.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cli/keyring_list_test.go b/cli/keyring_list_test.go index 9edbc65bc8..1c7a5fb9bf 100644 --- a/cli/keyring_list_test.go +++ b/cli/keyring_list_test.go @@ -61,5 +61,5 @@ func TestKeyringList(t *testing.T) { extractedKeys = append(extractedKeys, match[1]) } - assert.ElementsMatch(t, keyNames, extractedKeys, "The extracted keys do not match the expected keys.") + assert.ElementsMatch(t, keyNames, extractedKeys, "The listed keys do not match the expected keys.") } diff --git a/keyring/system.go b/keyring/system.go index 46ee03e1ca..0cafcf1002 100644 --- a/keyring/system.go +++ b/keyring/system.go @@ -55,7 +55,8 @@ func (s *systemKeyring) Delete(user string) error { } func (s *systemKeyring) List() ([]string, error) { - // List does not need to be returned here - // This function is a stub + // The OS keyring does not support listing keys + // This function is a stub for now because the Keyring interface requires it + // Currently, the 'defradb keyring list' command uses only fileKeyring return nil, nil } From 93716022a667cfbf1d697bf53532b039bd4668d7 Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Tue, 17 Dec 2024 09:42:13 -0500 Subject: [PATCH 22/25] Make docs --- .../website/references/cli/defradb_keyring.md | 1 + .../references/cli/defradb_keyring_list.md | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 docs/website/references/cli/defradb_keyring_list.md diff --git a/docs/website/references/cli/defradb_keyring.md b/docs/website/references/cli/defradb_keyring.md index 8dad08b542..f2d3048e71 100644 --- a/docs/website/references/cli/defradb_keyring.md +++ b/docs/website/references/cli/defradb_keyring.md @@ -53,4 +53,5 @@ To learn more about the available options: * [defradb keyring export](defradb_keyring_export.md) - Export a private key * [defradb keyring generate](defradb_keyring_generate.md) - Generate private keys * [defradb keyring import](defradb_keyring_import.md) - Import a private key +* [defradb keyring list](defradb_keyring_list.md) - List all keys in the keyring diff --git a/docs/website/references/cli/defradb_keyring_list.md b/docs/website/references/cli/defradb_keyring_list.md new file mode 100644 index 0000000000..fb43aef05c --- /dev/null +++ b/docs/website/references/cli/defradb_keyring_list.md @@ -0,0 +1,48 @@ +## defradb keyring list + +List all keys in the keyring + +### Synopsis + +List all keys in the keyring. +The DEFRA_KEYRING_SECRET environment variable must be set to unlock the keyring. +This can also be done with a .env file in the working directory or at a path +defined with the --secret-file flag. + +Example: + defradb keyring list + +``` +defradb keyring list [flags] +``` + +### Options + +``` + -h, --help help for list +``` + +### Options inherited from parent commands + +``` + --keyring-backend string Keyring backend to use. Options are file or system (default "file") + --keyring-namespace string Service name to use when using the system backend (default "defradb") + --keyring-path string Path to store encrypted keys when using the file backend (default "keys") + --log-format string Log format to use. Options are text or json (default "text") + --log-level string Log level to use. Options are debug, info, error, fatal (default "info") + --log-output string Log output path. Options are stderr or stdout. (default "stderr") + --log-overrides string Logger config overrides. Format ,=,...;,... + --log-source Include source location in logs + --log-stacktrace Include stacktrace in error and fatal logs + --no-keyring Disable the keyring and generate ephemeral keys + --no-log-color Disable colored log output + --rootdir string Directory for persistent data (default: $HOME/.defradb) + --secret-file string Path to the file containing secrets (default ".env") + --source-hub-address string The SourceHub address authorized by the client to make SourceHub transactions on behalf of the actor + --url string URL of HTTP endpoint to listen on or connect to (default "127.0.0.1:9181") +``` + +### SEE ALSO + +* [defradb keyring](defradb_keyring.md) - Manage DefraDB private keys + From 4131a0a18e4ed37cf528beb1f0ee228b15d39cbd Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Wed, 18 Dec 2024 09:54:49 -0500 Subject: [PATCH 23/25] Typo fix --- cli/keyring_list_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/keyring_list_test.go b/cli/keyring_list_test.go index 1c7a5fb9bf..b7e88ecf93 100644 --- a/cli/keyring_list_test.go +++ b/cli/keyring_list_test.go @@ -53,7 +53,8 @@ func TestKeyringList(t *testing.T) { outputString := output.String() // Use regex to extract the keys, and compare with the expected values - // We know the format the output should be, which is: "Keys in the keyring:\n- keyname1\n- keyname2\n- keyname3\n" + // We know what the format the output should be, which is: + // "Keys in the keyring:\n- keyname1\n- keyname2\n- keyname3\n" re := regexp.MustCompile(`-\s([^\n]+)`) matches := re.FindAllStringSubmatch(outputString, -1) var extractedKeys []string From c16d843db87ed2bb9f464a91b227afd3e708c28c Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Wed, 18 Dec 2024 13:28:20 -0500 Subject: [PATCH 24/25] Error output to systemKeyring.List and unit test --- keyring/errors.go | 12 +++++++++--- keyring/system.go | 2 +- keyring/system_keyring_list_test.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 keyring/system_keyring_list_test.go diff --git a/keyring/errors.go b/keyring/errors.go index 724d36612c..dd957b8d83 100644 --- a/keyring/errors.go +++ b/keyring/errors.go @@ -10,7 +10,13 @@ package keyring -import "github.com/zalando/go-keyring" +import ( + "github.com/zalando/go-keyring" -// ErrNotFound is returned when a keyring item is not found. -var ErrNotFound = keyring.ErrNotFound + "github.com/sourcenetwork/defradb/errors" +) + +var ( + ErrNotFound = keyring.ErrNotFound // ErrNotFound is returned when a keyring item is not found. + ErrSystemKeyringListInvoked = errors.New("listing keys is not supported by OS keyring") +) diff --git a/keyring/system.go b/keyring/system.go index 0cafcf1002..a2140ec7bb 100644 --- a/keyring/system.go +++ b/keyring/system.go @@ -58,5 +58,5 @@ func (s *systemKeyring) List() ([]string, error) { // The OS keyring does not support listing keys // This function is a stub for now because the Keyring interface requires it // Currently, the 'defradb keyring list' command uses only fileKeyring - return nil, nil + return nil, ErrSystemKeyringListInvoked } diff --git a/keyring/system_keyring_list_test.go b/keyring/system_keyring_list_test.go new file mode 100644 index 0000000000..680f646327 --- /dev/null +++ b/keyring/system_keyring_list_test.go @@ -0,0 +1,28 @@ +// Copyright 2024 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package keyring + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSystemKeyringListThrowsError(t *testing.T) { + service := "test-service" + systemKeyring := OpenSystemKeyring(service) + + keys, err := systemKeyring.List() + + assert.Nil(t, keys, "keys should be nil when List is called") + assert.Error(t, err, "an error should be returned when List is called") + assert.Equal(t, ErrSystemKeyringListInvoked, err, "the error should be ErrSystemKeyringListInvoked") +} From bcb5df89816d650d615c25863c41e36556aecafe Mon Sep 17 00:00:00 2001 From: Chris Quigley Date: Fri, 20 Dec 2024 14:37:03 -0500 Subject: [PATCH 25/25] Comment formatting// assert -> require --- keyring/errors.go | 3 ++- keyring/system_keyring_list_test.go | 7 +++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/keyring/errors.go b/keyring/errors.go index dd957b8d83..45280daa1c 100644 --- a/keyring/errors.go +++ b/keyring/errors.go @@ -17,6 +17,7 @@ import ( ) var ( - ErrNotFound = keyring.ErrNotFound // ErrNotFound is returned when a keyring item is not found. + // ErrNotFound is returned when a keyring item is not found. + ErrNotFound = keyring.ErrNotFound ErrSystemKeyringListInvoked = errors.New("listing keys is not supported by OS keyring") ) diff --git a/keyring/system_keyring_list_test.go b/keyring/system_keyring_list_test.go index 680f646327..443b099c01 100644 --- a/keyring/system_keyring_list_test.go +++ b/keyring/system_keyring_list_test.go @@ -13,7 +13,7 @@ package keyring import ( "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSystemKeyringListThrowsError(t *testing.T) { @@ -22,7 +22,6 @@ func TestSystemKeyringListThrowsError(t *testing.T) { keys, err := systemKeyring.List() - assert.Nil(t, keys, "keys should be nil when List is called") - assert.Error(t, err, "an error should be returned when List is called") - assert.Equal(t, ErrSystemKeyringListInvoked, err, "the error should be ErrSystemKeyringListInvoked") + require.Nil(t, keys, "keys should be nil when List is called") + require.ErrorIs(t, err, ErrSystemKeyringListInvoked, "function should throw ErrSystemKeyringListInvoked error") }