Suggestions (#1)

* suggestions

* makefile, lint

* webrpc v0.21.0

* github action go version from go.mod file

* non exporting Config.get method

* lint in CI

* invalid tests cases

* test case for session admin without account

* lint imports

* pr comments

.github/workflows/go.yml

@@ -10,19 +10,31 @@ on:
     branches: [ "master" ]
 
 jobs:
-
-  build:
+  test:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
 
     - name: Set up Go
       uses: actions/setup-go@v4
       with:
-        go-version: '1.21'
+        go-version-file: 'go.mod'
 
     - name: Build
       run: go build -v ./...
 
     - name: Test
       run: go test -v ./...
+
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: stable
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v6
+        with:
+          version: v1.60

.gitignore

@@ -0,0 +1 @@
+coverage.out

.golangci.yml

@@ -0,0 +1,88 @@
+linters:
+  # Disable all linters.
+  disable-all: true
+  # Enable specific linter
+  enable:
+    - errcheck
+    - gci
+    - wrapcheck
+    - ineffassign
+    - unused
+
+run:
+  # Number of operating system threads (`GOMAXPROCS`) that can execute golangci-lint simultaneously.
+  # If it is explicitly set to 0 (i.e. not the default) then golangci-lint will automatically set the value to match Linux container CPU quota.
+  # Default: the number of logical CPUs in the machine
+  concurrency: 8
+  # Timeout for analysis, e.g. 30s, 5m.
+  # Default: 1m
+  timeout: 5m
+  go: '1.22.0'
+
+output:
+  # Show statistics per linter.
+  show-stats: true
+  # Sort results by the order defined in `sort-order`.
+  sort-results: true
+  # Order to use when sorting results.
+  # Require `sort-results` to `true`.
+  # Possible values: `file`, `linter`, and `severity`.
+  #
+  # If the severity values are inside the following list, they are ordered in this order:
+  #   1. error
+  #   2. warning
+  #   3. high
+  #   4. medium
+  #   5. low
+  # Either they are sorted alphabetically.
+  sort-order:
+    - linter
+    - file
+    - severity
+
+issues:
+  # Maximum issues count per one linter.
+  # Set to 0 to disable.
+  # Default: 50
+  max-issues-per-linter: 0
+  # Maximum count of issues with the same text.
+  # Set to 0 to disable.
+  # Default: 3
+  max-same-issues: 0
+  exclude-rules:
+    - linters:
+        - lll
+      source: "^//go:generate "
+  exclude-dirs:
+    - "tools"
+  exclude-files:
+    - ".*\\.gen\\.go$"
+    - ".*\\.ridl$"
+
+linters-settings:
+  errcheck:
+    # List of functions to exclude from checking, where each entry is a single function to exclude.
+    # See https://github.com/kisielk/errcheck#excluding-functions for details.
+    exclude-functions:
+      - (net/http.ResponseWriter).Write
+
+  gci:
+    # Section configuration to compare against.
+    # Section names are case-insensitive and may contain parameters in ().
+    # The default order of sections is `standard > default > custom > blank > dot > alias > localmodule`,
+    # If `custom-order` is `true`, it follows the order of `sections` option.
+    # Default: ["standard", "default"]
+    sections:
+      - standard # Standard section: captures all standard packages.
+      - default # Default section: contains all imports that could not be matched to another section type.
+      - prefix(github.com/0xsequence/authcontrol) # Custom section: groups all imports with the specified Prefix.
+    # Skip generated files.
+    # Default: true
+    skip-generated: true
+    # Enable custom order of sections.
+    # If `true`, make the section order the same as the order of `sections`.
+    # Default: false
+    custom-order: true
+    # Drops lexical ordering for custom sections.
+    # Default: false
+    no-lex-order: false

Makefile

@@ -0,0 +1,29 @@
+TEST_FLAGS ?= -p 8 -failfast -race -shuffle on
+
+all:
+	@echo "make <cmd>:"
+	@echo ""
+	@echo "commands:"
+	@awk -F'[ :]' '/^#+/ {comment=$$0; gsub(/^#+[ ]*/, "", comment)} !/^(_|all:)/ && /^([A-Za-z_-]+):/ && !seen[$$1]++ {printf "  %-24s %s\n", $$1, (comment ? "- " comment : ""); comment=""} !/^#+/ {comment=""}' Makefile
+
+test-clean:
+	go clean -testcache
+
+test: test-clean
+	go test -run=$(TEST) $(TEST_FLAGS) -json ./... | tparse --all --follow
+
+test-rerun: test-clean
+	go run github.com/goware/rerun/cmd/rerun -watch ./ -run 'make test'
+
+test-coverage:
+	go test -run=$(TEST) $(TEST_FLAGS) -cover -coverprofile=coverage.out -json ./... | tparse --all --follow
+
+test-coverage-inspect: test-coverage
+	go tool cover -html=coverage.out
+
+generate:
+	go generate -x ./...
+
+lint:
+	golangci-lint run ./... --fix -c .golangci.yml
+

common.go

@@ -29,70 +29,69 @@ type UserStore interface {
 }
 
 // Config is a generic map of services/methods to a config value.
+// map[service]map[method]T
 type Config[T any] map[string]map[string]T
 
-// Get returns the config value for the given request.
+// returns the config value for the given request.
 func (c Config[T]) Get(r *rcpRequest) (v T, ok bool) {
-	if c == nil || r.Package != "rpc" {
+	if c == nil {
 		return v, false
 	}
-	serviceCfg, ok := c[r.Service]
-	if !ok {
-		return v, false
-	}
-	methodCfg, ok := serviceCfg[r.Method]
+
+	methodCfg, ok := c[r.serviceName][r.methodName]
 	if !ok {
 		return v, false
 	}
+
 	return methodCfg, true
 }
 
 // rcpRequest is a parsed RPC request.
 type rcpRequest struct {
-	Package string
-	Service string
-	Method  string
+	packageName string
+	serviceName string
+	methodName  string
 }
 
 // newRequest parses a path into an rcpRequest.
 func newRequest(path string) *rcpRequest {
-	parts := strings.Split(path, "/")
-	if len(parts) != 4 {
-		return nil
-	}
-	if parts[0] != "" {
+	p := strings.Split(path, "/")
+	if len(p) < 4 {
 		return nil
 	}
-	t := rcpRequest{
-		Package: parts[1],
-		Service: parts[2],
-		Method:  parts[3],
+
+	t := &rcpRequest{
+		packageName: p[len(p)-3],
+		serviceName: p[len(p)-2],
+		methodName:  p[len(p)-1],
 	}
-	if t.Package == "" || t.Service == "" || t.Method == "" {
+
+	if t.packageName != "rpc" {
 		return nil
 	}
-	return &t
+
+	return t
 }
 
 // ACL is a list of session types, encoded as a bitfield.
 // SessionType(n) is represented by n=-the bit.
 type ACL uint64
 
 // NewACL returns a new ACL with the given session types.
-func NewACL(t ...proto.SessionType) ACL {
-	var types ACL
-	for _, v := range t {
-		types = types.And(v)
+func NewACL(sessions ...proto.SessionType) ACL {
+	var acl ACL
+	for _, v := range sessions {
+		acl = acl.And(v)
 	}
-	return types
+	return acl
 }
 
 // And returns a new ACL with the given session types added.
-func (t ACL) And(types ...proto.SessionType) ACL {
-	for _, v := range types {
-		t |= 1 << v
+func (a ACL) And(session ...proto.SessionType) ACL {
+	for _, v := range session {
+		a |= 1 << v
 	}
-	return t
+	return a
 }
 
 // Includes returns true if the ACL includes the given session type.

common_test.go

@@ -7,19 +7,21 @@ import (
 	"net/http/httptest"
 	"testing"
 
-	"github.com/0xsequence/authcontrol/proto"
 	"github.com/go-chi/jwtauth/v5"
 	"github.com/stretchr/testify/require"
+
+	"github.com/0xsequence/authcontrol/proto"
 )
 
-func mustJWT(t *testing.T, auth *jwtauth.JWTAuth, claims map[string]any) string {
+func mustJWT(t *testing.T, auth *jwtauth.JWTAuth, claims map[string]any) *string {
 	t.Helper()
 	if claims == nil {
-		return ""
+		return nil
 	}
+
 	_, token, err := auth.Encode(claims)
 	require.NoError(t, err)
-	return token
+	return &token
 }
 
 const HeaderKey = "Test-Key"
@@ -28,27 +30,28 @@ func keyFunc(r *http.Request) string {
 	return r.Header.Get(HeaderKey)
 }
 
-func executeRequest(ctx context.Context, handler http.Handler, path, accessKey, jwt string) (bool, http.Header, error) {
+func executeRequest(t *testing.T, ctx context.Context, handler http.Handler, path, accessKey string, jwt *string) (bool, error) {
 	req, err := http.NewRequest("POST", path, nil)
-	if err != nil {
-		return false, nil, err
-	}
+	require.NoError(t, err)
+
 	req.Header.Set("X-Real-IP", "127.0.0.1")
 	if accessKey != "" {
 		req.Header.Set(HeaderKey, accessKey)
 	}
-	if jwt != "" {
-		req.Header.Set("Authorization", "Bearer "+jwt)
+
+	if jwt != nil {
+		req.Header.Set("Authorization", "Bearer "+*jwt)
 	}
 
 	rr := httptest.NewRecorder()
 	handler.ServeHTTP(rr, req.WithContext(ctx))
 
 	if status := rr.Result().StatusCode; status < http.StatusOK || status >= http.StatusBadRequest {
 		w := proto.WebRPCError{}
-		json.Unmarshal(rr.Body.Bytes(), &w)
-		return false, rr.Header(), w
+		err = json.Unmarshal(rr.Body.Bytes(), &w)
+		require.NoError(t, err)
+		return false, w
 	}
 
-	return true, rr.Header(), nil
+	return true, nil
 }

context.go

@@ -23,6 +23,10 @@ var (
 	ctxKeyProjectID   = &contextKey{"ProjectID"}
 )
 
+//
+// Session Type
+//
+
 // WithSessionType adds the access key to the context.
 func WithSessionType(ctx context.Context, accessType proto.SessionType) context.Context {
 	return context.WithValue(ctx, ctxKeySessionType, accessType)
@@ -37,8 +41,12 @@ func GetSessionType(ctx context.Context) (proto.SessionType, bool) {
 	return v, true
 }
 
-// WithAccount adds the account to the context.
-func WithAccount(ctx context.Context, account string) context.Context {
+//
+// Account
+//
+
+// withAccount adds the account to the context.
+func withAccount(ctx context.Context, account string) context.Context {
 	return context.WithValue(ctx, ctxKeyAccount, account)
 }
 
@@ -48,8 +56,12 @@ func GetAccount(ctx context.Context) (string, bool) {
 	return v, ok
 }
 
-// WithUser adds the user to the context.
-func WithUser(ctx context.Context, user any) context.Context {
+//
+// User
+//
+
+// withUser adds the user to the context.
+func withUser(ctx context.Context, user any) context.Context {
 	return context.WithValue(ctx, ctxKeyUser, user)
 }
 
@@ -59,6 +71,10 @@ func GetUser[T any](ctx context.Context) (T, bool) {
 	return v, ok
 }
 
+//
+// Service
+//
+
 // WithService adds the service to the context.
 func WithService(ctx context.Context, service string) context.Context {
 	return context.WithValue(ctx, ctxKeyService, service)
@@ -70,8 +86,12 @@ func GetService(ctx context.Context) (string, bool) {
 	return v, ok
 }
 
-// WithAccessKey adds the access key to the context.
-func WithAccessKey(ctx context.Context, accessKey string) context.Context {
+//
+// AccessKey
+//
+
+// withAccessKey adds the access key to the context.
+func withAccessKey(ctx context.Context, accessKey string) context.Context {
 	return context.WithValue(ctx, ctxKeyAccessKey, accessKey)
 }
 
@@ -81,6 +101,10 @@ func GetAccessKey(ctx context.Context) (string, bool) {
 	return v, ok
 }
 
+//
+// Project ID
+//
+
 // withProjectID adds the projectID to the context.
 func withProjectID(ctx context.Context, projectID uint64) context.Context {
 	return context.WithValue(ctx, ctxKeyProjectID, projectID)