Skip to content

Commit 3fb8b27

Browse files
committed
Merge branch 'development' of github.com:gofr-dev/gofr into release/v1.35.0
2 parents 8834530 + c73187e commit 3fb8b27

File tree

7 files changed

+80
-44
lines changed

7 files changed

+80
-44
lines changed

Diff for: pkg/gofr/container/mock_container.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package container
22

33
import (
44
"context"
5-
"fmt"
6-
"net/http"
75
"testing"
86

97
"go.uber.org/mock/gomock"
@@ -33,7 +31,7 @@ type Mocks struct {
3331

3432
type options func(c *Container, ctrl *gomock.Controller) any
3533

36-
//nolint:revive //Because user should not access the options, and we might change it to an interface in the future.
34+
//nolint:revive // Because user should not access the options, and we might change it to an interface in the future.
3735
func WithMockHTTPService(httpServiceNames ...string) options {
3836
return func(c *Container, ctrl *gomock.Controller) any {
3937
mockservice := service.NewMockHTTP(ctrl)
@@ -124,8 +122,9 @@ func NewMockContainer(t *testing.T, options ...options) (*Container, *Mocks) {
124122
Metrics: mockMetrics,
125123
}
126124

127-
mockMetrics.EXPECT().RecordHistogram(gomock.Any(), "app_http_service_response", gomock.Any(), "path", gomock.Any(),
128-
"method", gomock.Any(), "status", fmt.Sprintf("%v", http.StatusInternalServerError)).AnyTimes()
125+
// TODO: Remove this expectation from mock container (previous generalisation) to the actual tests where their expectations are being set.
126+
mockMetrics.EXPECT().RecordHistogram(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
127+
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
129128

130129
return container, &mocks
131130
}

Diff for: pkg/gofr/context_test.go

+10-14
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"net/http"
88
"net/http/httptest"
99
"testing"
10+
"time"
1011

1112
"github.com/golang-jwt/jwt/v5"
1213
"github.com/gorilla/websocket"
@@ -78,42 +79,37 @@ func TestContext_AddTrace(t *testing.T) {
7879

7980
func TestContext_WriteMessageToSocket(t *testing.T) {
8081
port := testutil.GetFreePort(t)
81-
8282
t.Setenv("HTTP_PORT", fmt.Sprint(port))
8383

8484
app := New()
8585

86-
server := httptest.NewServer(app.httpServer.router)
87-
defer server.Close()
88-
8986
app.WebSocket("/ws", func(ctx *Context) (any, error) {
9087
socketErr := ctx.WriteMessageToSocket("Hello! GoFr")
9188
if socketErr != nil {
9289
return nil, socketErr
9390
}
9491

95-
// TODO: returning error here to close the connection to the websocket
96-
// as the websocket close error is not caught because we are using no bind function here.
97-
// this must not be necessary. We should put an actual check in handleWebSocketConnection method instead.
98-
return nil, &websocket.CloseError{Code: websocket.CloseNormalClosure, Text: "Error closing"}
92+
conn := ctx.GetConnectionFromContext(ctx)
93+
defer conn.Close()
94+
95+
return "", socketErr
9996
})
10097

10198
go app.Run()
99+
time.Sleep(100 * time.Millisecond) // Wait for the server to boot
102100

103-
wsURL := "ws" + server.URL[len("http"):] + "/ws"
101+
wsURL := fmt.Sprintf("ws://localhost:%d/ws", port)
104102

105-
// Create a WebSocket client
106103
ws, resp, err := websocket.DefaultDialer.Dial(wsURL, nil)
107-
require.NoError(t, err)
104+
require.NoError(t, err, "WebSocket handshake failed")
108105

109106
defer resp.Body.Close()
110107
defer ws.Close()
111108

112109
_, message, err := ws.ReadMessage()
113-
require.NoError(t, err)
110+
require.NoError(t, err, "Failed to read WebSocket message")
114111

115-
expectedResponse := "Hello! GoFr"
116-
assert.Equal(t, expectedResponse, string(message))
112+
assert.Equal(t, "Hello! GoFr", string(message))
117113
}
118114

119115
func TestGetAuthInfo_BasicAuth(t *testing.T) {

Diff for: pkg/gofr/factory.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88

99
"gofr.dev/pkg/gofr/cmd/terminal"
1010
"gofr.dev/pkg/gofr/container"
11-
"gofr.dev/pkg/gofr/http/middleware"
1211
"gofr.dev/pkg/gofr/logging"
1312
)
1413

@@ -38,7 +37,7 @@ func New() *App {
3837
port = defaultHTTPPort
3938
}
4039

41-
app.httpServer = newHTTPServer(app.container, port, middleware.GetConfigs(app.Config))
40+
app.httpServer = newHTTPServer(port)
4241
app.httpServer.certFile = app.Config.GetOrDefault("CERT_FILE", "")
4342
app.httpServer.keyFile = app.Config.GetOrDefault("KEY_FILE", "")
4443

Diff for: pkg/gofr/gofr.go

+10-5
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import (
2323
"gofr.dev/pkg/gofr/service"
2424
)
2525

26+
const (
27+
configLocation = "./configs"
28+
)
29+
2630
// App is the main application in the GoFr framework.
2731
type App struct {
2832
// Config can be used by applications to fetch custom configurations from environment or file.
@@ -136,18 +140,19 @@ func (a *App) startSubscriptions(ctx context.Context) error {
136140

137141
// readConfig reads the configuration from the default location.
138142
func (a *App) readConfig(isAppCMD bool) {
139-
var configLocation string
140-
if _, err := os.Stat("./configs"); err == nil {
141-
configLocation = "./configs"
143+
var location string
144+
145+
if _, err := os.Stat(configLocation); err == nil {
146+
location = configLocation
142147
}
143148

144149
if isAppCMD {
145-
a.Config = config.NewEnvFile(configLocation, logging.NewFileLogger(""))
150+
a.Config = config.NewEnvFile(location, logging.NewFileLogger(""))
146151

147152
return
148153
}
149154

150-
a.Config = config.NewEnvFile(configLocation, logging.NewLogger(logging.INFO))
155+
a.Config = config.NewEnvFile(location, logging.NewLogger(logging.INFO))
151156
}
152157

153158
// AddHTTPService registers HTTP service in container.

Diff for: pkg/gofr/http_server.go

+16-10
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,10 @@ var (
2929
errInvalidKeyFile = errors.New("invalid key file")
3030
)
3131

32-
func newHTTPServer(c *container.Container, port int, middlewareConfigs map[string]string) *httpServer {
32+
func newHTTPServer(port int) *httpServer {
3333
r := gofrHTTP.NewRouter()
3434
wsManager := websocket.New()
3535

36-
r.Use(
37-
middleware.WSHandlerUpgrade(c, wsManager),
38-
middleware.Tracer,
39-
middleware.Logging(c.Logger),
40-
middleware.CORS(middlewareConfigs, r.RegisteredRoutes),
41-
middleware.Metrics(c.Metrics()),
42-
)
43-
4436
return &httpServer{
4537
router: r,
4638
port: port,
@@ -70,7 +62,21 @@ func (s *httpServer) RegisterProfilingRoutes() {
7062
s.router.NewRoute().Methods(http.MethodGet).PathPrefix("/debug/pprof/").HandlerFunc(pprof.Index)
7163
}
7264

73-
func (s *httpServer) Run(c *container.Container) {
65+
func (s *httpServer) run(c *container.Container, middlewareConfigs map[string]string) {
66+
// Developer Note:
67+
// WebSocket connections do not inherently support authentication mechanisms.
68+
// It is recommended to authenticate users before upgrading to a WebSocket connection.
69+
// Hence, we are registering middlewares here, to ensure that authentication or other
70+
// middleware logic is executed during the initial HTTP handshake request, prior to upgrading
71+
// the connection to WebSocket, if any.
72+
s.router.Use(
73+
middleware.WSHandlerUpgrade(c, s.ws),
74+
middleware.Tracer,
75+
middleware.CORS(middlewareConfigs, s.router.RegisteredRoutes),
76+
middleware.Logging(c.Logger),
77+
middleware.Metrics(c.Metrics()),
78+
)
79+
7480
if s.srv != nil {
7581
c.Logf("Server already running on port: %d", s.port)
7682
return

Diff for: pkg/gofr/http_server_test.go

+36-7
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,14 @@ import (
1010
"testing"
1111
"time"
1212

13+
"github.com/gorilla/mux"
1314
"github.com/stretchr/testify/assert"
1415
"github.com/stretchr/testify/require"
1516

17+
"gofr.dev/pkg/gofr/config"
1618
"gofr.dev/pkg/gofr/container"
1719
gofrHTTP "gofr.dev/pkg/gofr/http"
20+
"gofr.dev/pkg/gofr/http/middleware"
1821
"gofr.dev/pkg/gofr/logging"
1922
"gofr.dev/pkg/gofr/testutil"
2023
)
@@ -28,10 +31,24 @@ func TestRun_ServerStartsListening(t *testing.T) {
2831
w.WriteHeader(http.StatusOK)
2932
}))
3033

34+
// adding registered routes for applying middlewares
35+
var registeredMethods []string
36+
37+
_ = router.Walk(func(route *mux.Route, _ *mux.Router, _ []*mux.Route) error {
38+
met, _ := route.GetMethods()
39+
for _, method := range met {
40+
if !contains(registeredMethods, method) { // Check for uniqueness before adding
41+
registeredMethods = append(registeredMethods, method)
42+
}
43+
}
44+
45+
return nil
46+
})
47+
48+
router.RegisteredRoutes = &registeredMethods
49+
3150
// Create a mock container
32-
c := &container.Container{
33-
Logger: logging.NewLogger(logging.INFO),
34-
}
51+
c := container.NewContainer(getConfigs(t))
3552

3653
// Create an instance of httpServer
3754
server := &httpServer{
@@ -40,7 +57,7 @@ func TestRun_ServerStartsListening(t *testing.T) {
4057
}
4158

4259
// Start the server
43-
go server.Run(c)
60+
go server.run(c, middleware.GetConfigs(getConfigs(t)))
4461

4562
// Wait for the server to start listening
4663
time.Sleep(100 * time.Millisecond)
@@ -61,6 +78,18 @@ func TestRun_ServerStartsListening(t *testing.T) {
6178
resp.Body.Close()
6279
}
6380

81+
func getConfigs(t *testing.T) config.Config {
82+
t.Helper()
83+
84+
var configLocation string
85+
86+
if _, err := os.Stat("./configs"); err == nil {
87+
configLocation = "./configs"
88+
}
89+
90+
return config.NewEnvFile(configLocation, logging.NewLogger(logging.INFO))
91+
}
92+
6493
func TestRegisterProfillingRoutes(t *testing.T) {
6594
port := testutil.GetFreePort(t)
6695

@@ -75,7 +104,7 @@ func TestRegisterProfillingRoutes(t *testing.T) {
75104

76105
server.RegisterProfilingRoutes()
77106

78-
go server.Run(c)
107+
go server.run(c, middleware.GetConfigs(getConfigs(t)))
79108

80109
// Test if the expected handlers are registered for the pprof endpoints
81110
expectedRoutes := []string{
@@ -114,7 +143,7 @@ func TestShutdown_ServerStopsListening(t *testing.T) {
114143
}
115144

116145
// Start the server
117-
go server.Run(c)
146+
go server.run(c, middleware.GetConfigs(getConfigs(t)))
118147

119148
// Create a context with a timeout to test the shutdown
120149
ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond)
@@ -150,7 +179,7 @@ func TestShutdown_ServerContextDeadline(t *testing.T) {
150179
}
151180

152181
// Start the server
153-
go server.Run(c)
182+
go server.run(c, middleware.GetConfigs(getConfigs(t)))
154183

155184
// Create a context with a timeout to test the shutdown
156185
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)

Diff for: pkg/gofr/run.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"os/signal"
88
"sync"
99
"syscall"
10+
11+
"gofr.dev/pkg/gofr/http/middleware"
1012
)
1113

1214
// Run starts the application. If it is an HTTP server, it will start the server.
@@ -68,7 +70,7 @@ func (a *App) Run() {
6870

6971
go func(s *httpServer) {
7072
defer wg.Done()
71-
s.Run(a.container)
73+
s.run(a.container, middleware.GetConfigs(a.Config))
7274
}(a.httpServer)
7375
}
7476

0 commit comments

Comments
 (0)