Skip to content

Commit 2009474

Browse files
runcomruncom
committed
fix: remove mutexes around ownerinfo/rvinfo
Signed-off-by: Antonio Murdaca <[email protected]>
1 parent e3238a7 commit 2009474

File tree

5 files changed

+234
-67
lines changed

5 files changed

+234
-67
lines changed

‎api/handlers/ownerinfo.go‎

Lines changed: 14 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,20 @@ import (
88
"io"
99
"log/slog"
1010
"net/http"
11-
"sync"
1211

1312
"github.com/fido-device-onboard/go-fdo-server/internal/db"
1413
"gorm.io/gorm"
1514
)
1615

1716
func OwnerInfoHandler(w http.ResponseWriter, r *http.Request) {
18-
var mu sync.Mutex
1917
slog.Debug("Received OwnerInfo request", "method", r.Method, "path", r.URL.Path)
2018
switch r.Method {
2119
case http.MethodGet:
2220
getOwnerInfo(w, r)
2321
case http.MethodPost:
24-
createOwnerInfo(w, r, &mu)
22+
createOwnerInfo(w, r)
2523
case http.MethodPut:
26-
updateOwnerInfo(w, r, &mu)
24+
updateOwnerInfo(w, r)
2725
default:
2826
slog.Debug("Method not allowed", "method", r.Method, "path", r.URL.Path)
2927
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
@@ -48,28 +46,20 @@ func getOwnerInfo(w http.ResponseWriter, _ *http.Request) {
4846
w.Write(ownerInfoJSON)
4947
}
5048

51-
func createOwnerInfo(w http.ResponseWriter, r *http.Request, mu *sync.Mutex) {
52-
mu.Lock()
53-
defer mu.Unlock()
54-
49+
func createOwnerInfo(w http.ResponseWriter, r *http.Request) {
5550
ownerInfo, err := io.ReadAll(r.Body)
5651
if err != nil {
5752
slog.Debug("Error reading body", "error", err)
5853
http.Error(w, "Error reading body", http.StatusInternalServerError)
5954
return
6055
}
6156

62-
if _, err := db.FetchOwnerInfoJSON(); err == nil {
63-
slog.Debug("ownerInfo already exists, cannot create new entry")
64-
http.Error(w, "ownerInfo already exists", http.StatusConflict)
65-
return
66-
} else if !errors.Is(err, gorm.ErrRecordNotFound) {
67-
slog.Debug("Error checking ownerInfo existence", "error", err)
68-
http.Error(w, "Error processing ownerInfo", http.StatusInternalServerError)
69-
return
70-
}
71-
7257
if err := db.InsertOwnerInfo(ownerInfo); err != nil {
58+
if errors.Is(err, gorm.ErrDuplicatedKey) {
59+
slog.Debug("ownerInfo already exists (constraint)", "error", err)
60+
http.Error(w, "ownerInfo already exists", http.StatusConflict)
61+
return
62+
}
7363
slog.Debug("Error inserting ownerInfo", "error", err)
7464
http.Error(w, "Error inserting ownerInfo", http.StatusInternalServerError)
7565
return
@@ -82,27 +72,20 @@ func createOwnerInfo(w http.ResponseWriter, r *http.Request, mu *sync.Mutex) {
8272
w.Write(ownerInfo)
8373
}
8474

85-
func updateOwnerInfo(w http.ResponseWriter, r *http.Request, mu *sync.Mutex) {
86-
mu.Lock()
87-
defer mu.Unlock()
88-
75+
func updateOwnerInfo(w http.ResponseWriter, r *http.Request) {
8976
ownerInfo, err := io.ReadAll(r.Body)
9077
if err != nil {
9178
slog.Debug("Error reading body", "error", err)
9279
http.Error(w, "Error reading body", http.StatusInternalServerError)
9380
return
9481
}
95-
if _, err := db.FetchOwnerInfoJSON(); errors.Is(err, gorm.ErrRecordNotFound) {
96-
slog.Debug("ownerInfo does not exist, cannot update")
97-
http.Error(w, "ownerInfo does not exist", http.StatusNotFound)
98-
return
99-
} else if err != nil {
100-
slog.Debug("Error checking ownerInfo existence", "error", err)
101-
http.Error(w, "Error processing ownerInfo", http.StatusInternalServerError)
102-
return
103-
}
10482

10583
if err := db.UpdateOwnerInfo(ownerInfo); err != nil {
84+
if errors.Is(err, gorm.ErrRecordNotFound) {
85+
slog.Debug("ownerInfo does not exist, cannot update")
86+
http.Error(w, "ownerInfo does not exist", http.StatusNotFound)
87+
return
88+
}
10689
slog.Debug("Error updating ownerInfo", "error", err)
10790
http.Error(w, "Error updating ownerInfo", http.StatusInternalServerError)
10891
return

‎api/handlers/rvinfo.go‎

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,21 @@ import (
88
"io"
99
"log/slog"
1010
"net/http"
11-
"sync"
1211

1312
"github.com/fido-device-onboard/go-fdo-server/internal/db"
1413
"gorm.io/gorm"
1514
)
1615

1716
func RvInfoHandler() http.HandlerFunc {
18-
var mu sync.Mutex
1917
return func(w http.ResponseWriter, r *http.Request) {
2018
slog.Debug("Received RV request", "method", r.Method, "path", r.URL.Path)
2119
switch r.Method {
2220
case http.MethodGet:
2321
getRvInfo(w, r)
2422
case http.MethodPost:
25-
createRvInfo(w, r, &mu)
23+
createRvInfo(w, r)
2624
case http.MethodPut:
27-
updateRvInfo(w, r, &mu)
25+
updateRvInfo(w, r)
2826
default:
2927
slog.Debug("Method not allowed", "method", r.Method, "path", r.URL.Path)
3028
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
@@ -50,28 +48,20 @@ func getRvInfo(w http.ResponseWriter, _ *http.Request) {
5048
w.Write(rvInfoJSON)
5149
}
5250

53-
func createRvInfo(w http.ResponseWriter, r *http.Request, mu *sync.Mutex) {
54-
mu.Lock()
55-
defer mu.Unlock()
56-
51+
func createRvInfo(w http.ResponseWriter, r *http.Request) {
5752
rvInfo, err := io.ReadAll(r.Body)
5853
if err != nil {
5954
slog.Debug("Error reading body", "error", err)
6055
http.Error(w, "Error reading body", http.StatusInternalServerError)
6156
return
6257
}
6358

64-
if _, err := db.FetchRvInfoJSON(); err == nil {
65-
slog.Debug("rvInfo already exists, cannot create new entry")
66-
http.Error(w, "rvInfo already exists", http.StatusConflict)
67-
return
68-
} else if !errors.Is(err, gorm.ErrRecordNotFound) {
69-
slog.Debug("Error checking rvInfo existence", "error", err)
70-
http.Error(w, "Error processing rvInfo", http.StatusInternalServerError)
71-
return
72-
}
73-
7459
if err := db.InsertRvInfo(rvInfo); err != nil {
60+
if errors.Is(err, gorm.ErrDuplicatedKey) {
61+
slog.Debug("rvInfo already exists (constraint)", "error", err)
62+
http.Error(w, "rvInfo already exists", http.StatusConflict)
63+
return
64+
}
7565
slog.Debug("Error inserting rvInfo", "error", err)
7666
http.Error(w, "Error inserting rvInfo", http.StatusInternalServerError)
7767
return
@@ -84,28 +74,20 @@ func createRvInfo(w http.ResponseWriter, r *http.Request, mu *sync.Mutex) {
8474
w.Write(rvInfo)
8575
}
8676

87-
func updateRvInfo(w http.ResponseWriter, r *http.Request, mu *sync.Mutex) {
88-
mu.Lock()
89-
defer mu.Unlock()
90-
77+
func updateRvInfo(w http.ResponseWriter, r *http.Request) {
9178
rvInfo, err := io.ReadAll(r.Body)
9279
if err != nil {
9380
slog.Debug("Error reading body", "error", err)
9481
http.Error(w, "Error reading body", http.StatusInternalServerError)
9582
return
9683
}
9784

98-
if _, err := db.FetchRvInfoJSON(); errors.Is(err, gorm.ErrRecordNotFound) {
99-
slog.Debug("rvInfo does not exist, cannot update")
100-
http.Error(w, "rvInfo does not exist", http.StatusNotFound)
101-
return
102-
} else if err != nil {
103-
slog.Debug("Error checking rvInfo existence", "error", err)
104-
http.Error(w, "Error processing rvInfo", http.StatusInternalServerError)
105-
return
106-
}
107-
10885
if err := db.UpdateRvInfo(rvInfo); err != nil {
86+
if errors.Is(err, gorm.ErrRecordNotFound) {
87+
slog.Debug("rvInfo does not exist, cannot update")
88+
http.Error(w, "rvInfo does not exist", http.StatusNotFound)
89+
return
90+
}
10991
slog.Debug("Error updating rvInfo", "error", err)
11092
http.Error(w, "Error updating rvInfo", http.StatusInternalServerError)
11193
return

‎api/handlersTest/ownerinfo_test.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// SPDX-FileCopyrightText: (C) 2024 Intel Corporation
2+
// SPDX-License-Identifier: Apache 2.0
3+
package handlersTest
4+
5+
import (
6+
"bytes"
7+
"net/http"
8+
"net/http/httptest"
9+
"testing"
10+
11+
"github.com/fido-device-onboard/go-fdo-server/api/handlers"
12+
)
13+
14+
func TestOwnerInfo_PostConflictOnDuplicate(t *testing.T) {
15+
setupTestDB(t)
16+
17+
handler := http.HandlerFunc(handlers.OwnerInfoHandler)
18+
body := []byte(`[{"dns":"owner.example","port":"8082","protocol":"http"}]`)
19+
20+
// First POST should create (201)
21+
req1 := httptest.NewRequest(http.MethodPost, "/api/v1/owner/redirect", bytes.NewReader(body))
22+
rec1 := httptest.NewRecorder()
23+
handler.ServeHTTP(rec1, req1)
24+
if rec1.Code != http.StatusCreated {
25+
t.Fatalf("expected 201 on first POST, got %d", rec1.Code)
26+
}
27+
28+
// Second POST should conflict (409)
29+
req2 := httptest.NewRequest(http.MethodPost, "/api/v1/owner/redirect", bytes.NewReader(body))
30+
rec2 := httptest.NewRecorder()
31+
handler.ServeHTTP(rec2, req2)
32+
if rec2.Code != http.StatusConflict {
33+
t.Fatalf("expected 409 on second POST, got %d", rec2.Code)
34+
}
35+
}
36+
37+
func TestOwnerInfo_Put404ThenCreateThenUpdateAndGet(t *testing.T) {
38+
setupTestDB(t)
39+
40+
get := func() *httptest.ResponseRecorder {
41+
req := httptest.NewRequest(http.MethodGet, "/api/v1/owner/redirect", nil)
42+
rec := httptest.NewRecorder()
43+
handlers.OwnerInfoHandler(rec, req)
44+
return rec
45+
}
46+
put := func(body []byte) *httptest.ResponseRecorder {
47+
req := httptest.NewRequest(http.MethodPut, "/api/v1/owner/redirect", bytes.NewReader(body))
48+
rec := httptest.NewRecorder()
49+
handlers.OwnerInfoHandler(rec, req)
50+
return rec
51+
}
52+
post := func(body []byte) *httptest.ResponseRecorder {
53+
req := httptest.NewRequest(http.MethodPost, "/api/v1/owner/redirect", bytes.NewReader(body))
54+
rec := httptest.NewRecorder()
55+
handlers.OwnerInfoHandler(rec, req)
56+
return rec
57+
}
58+
59+
// PUT before create -> 404
60+
putBody := []byte(`[{"dns":"owner.example","port":"8082","protocol":"http"}]`)
61+
if rec := put(putBody); rec.Code != http.StatusNotFound {
62+
t.Fatalf("expected 404 on PUT before create, got %d", rec.Code)
63+
}
64+
65+
// POST create -> 201
66+
postBody := []byte(`[{"dns":"owner.example","port":"8082","protocol":"http"}]`)
67+
if rec := post(postBody); rec.Code != http.StatusCreated {
68+
t.Fatalf("expected 201 on POST create, got %d", rec.Code)
69+
}
70+
71+
// PUT update -> 200
72+
updateBody := []byte(`[{"dns":"owner-updated","port":"9090","protocol":"http"}]`)
73+
if rec := put(updateBody); rec.Code != http.StatusOK {
74+
t.Fatalf("expected 200 on PUT update, got %d", rec.Code)
75+
}
76+
77+
// GET -> updated value present
78+
rec := get()
79+
if rec.Code != http.StatusOK {
80+
t.Fatalf("expected 200 on GET, got %d", rec.Code)
81+
}
82+
if got := rec.Body.String(); got != string(updateBody) {
83+
t.Fatalf("expected body %q, got %q", string(updateBody), got)
84+
}
85+
86+
}
87+
88+

‎api/handlersTest/rvinfo_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// SPDX-FileCopyrightText: (C) 2024 Intel Corporation
2+
// SPDX-License-Identifier: Apache 2.0
3+
package handlersTest
4+
5+
import (
6+
"bytes"
7+
"net/http"
8+
"net/http/httptest"
9+
"testing"
10+
11+
"github.com/fido-device-onboard/go-fdo-server/api/handlers"
12+
)
13+
14+
func TestRvInfo_PostConflictOnDuplicate(t *testing.T) {
15+
setupTestDB(t)
16+
17+
handler := handlers.RvInfoHandler()
18+
body := []byte(`[{"dns":"rv.example","device_port":"8082","owner_port":"8082","protocol":"http"}]`)
19+
20+
// First POST should create (201)
21+
req1 := httptest.NewRequest(http.MethodPost, "/api/v1/rv", bytes.NewReader(body))
22+
rec1 := httptest.NewRecorder()
23+
handler.ServeHTTP(rec1, req1)
24+
if rec1.Code != http.StatusCreated {
25+
t.Fatalf("expected 201 on first POST, got %d", rec1.Code)
26+
}
27+
28+
// Second POST should conflict (409)
29+
req2 := httptest.NewRequest(http.MethodPost, "/api/v1/rv", bytes.NewReader(body))
30+
rec2 := httptest.NewRecorder()
31+
handler.ServeHTTP(rec2, req2)
32+
if rec2.Code != http.StatusConflict {
33+
t.Fatalf("expected 409 on second POST, got %d", rec2.Code)
34+
}
35+
}
36+
37+
func TestRvInfo_Put404ThenCreateThenUpdateAndGet(t *testing.T) {
38+
setupTestDB(t)
39+
40+
get := func() *httptest.ResponseRecorder {
41+
req := httptest.NewRequest(http.MethodGet, "/api/v1/rv", nil)
42+
rec := httptest.NewRecorder()
43+
handlers.RvInfoHandler()(rec, req)
44+
return rec
45+
}
46+
put := func(body []byte) *httptest.ResponseRecorder {
47+
req := httptest.NewRequest(http.MethodPut, "/api/v1/rv", bytes.NewReader(body))
48+
rec := httptest.NewRecorder()
49+
handlers.RvInfoHandler()(rec, req)
50+
return rec
51+
}
52+
post := func(body []byte) *httptest.ResponseRecorder {
53+
req := httptest.NewRequest(http.MethodPost, "/api/v1/rv", bytes.NewReader(body))
54+
rec := httptest.NewRecorder()
55+
handlers.RvInfoHandler()(rec, req)
56+
return rec
57+
}
58+
59+
// PUT before create -> 404
60+
putBody := []byte(`[{"dns":"rv.example","device_port":"8082","owner_port":"8082","protocol":"http"}]`)
61+
if rec := put(putBody); rec.Code != http.StatusNotFound {
62+
t.Fatalf("expected 404 on PUT before create, got %d", rec.Code)
63+
}
64+
65+
// POST create -> 201
66+
postBody := []byte(`[{"dns":"rv.example","device_port":"8082","owner_port":"8082","protocol":"http"}]`)
67+
if rec := post(postBody); rec.Code != http.StatusCreated {
68+
t.Fatalf("expected 201 on POST create, got %d", rec.Code)
69+
}
70+
71+
// PUT update -> 200
72+
updateBody := []byte(`[{"dns":"rv-updated","device_port":"9090","owner_port":"9090","protocol":"http"}]`)
73+
if rec := put(updateBody); rec.Code != http.StatusOK {
74+
t.Fatalf("expected 200 on PUT update, got %d", rec.Code)
75+
}
76+
77+
// GET -> updated value present
78+
rec := get()
79+
if rec.Code != http.StatusOK {
80+
t.Fatalf("expected 200 on GET, got %d", rec.Code)
81+
}
82+
if got := rec.Body.String(); got != string(updateBody) {
83+
t.Fatalf("expected body %q, got %q", string(updateBody), got)
84+
}
85+
}

0 commit comments

Comments
 (0)