Skip to content

Commit 70ad544

Browse files
fmartingrfmartingr
committed
refactor: remove custom message output in json apis
1 parent 1bc94a6 commit 70ad544

25 files changed

Lines changed: 517 additions & 544 deletions

‎internal/http/handlers/api/v1/accounts.go‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func HandleListAccounts(deps model.Dependencies, c model.WebContext) {
4444
return
4545
}
4646

47-
response.Send(c, http.StatusOK, accounts)
47+
response.SendJSON(c, http.StatusOK, accounts)
4848
}
4949

5050
// @Summary Create an account
@@ -63,18 +63,18 @@ func HandleCreateAccount(deps model.Dependencies, c model.WebContext) {
6363

6464
var payload createAccountPayload
6565
if err := json.NewDecoder(c.Request().Body).Decode(&payload); err != nil {
66-
response.SendError(c, http.StatusBadRequest, "invalid json", nil)
66+
response.SendError(c, http.StatusBadRequest, "invalid json")
6767
return
6868
}
6969

7070
account, err := deps.Domains().Accounts().CreateAccount(c.Request().Context(), payload.ToAccountDTO())
7171
if err, isValidationErr := err.(model.ValidationError); isValidationErr {
72-
response.SendError(c, http.StatusBadRequest, err.Error(), nil)
72+
response.SendError(c, http.StatusBadRequest, err.Error())
7373
return
7474
}
7575

7676
if errors.Is(err, model.ErrAlreadyExists) {
77-
response.SendError(c, http.StatusConflict, "account already exists", nil)
77+
response.SendError(c, http.StatusConflict, "account already exists")
7878
return
7979
}
8080

@@ -84,7 +84,7 @@ func HandleCreateAccount(deps model.Dependencies, c model.WebContext) {
8484
return
8585
}
8686

87-
response.Send(c, http.StatusCreated, account)
87+
response.SendJSON(c, http.StatusCreated, account)
8888
}
8989

9090
// @Summary Delete an account
@@ -103,13 +103,13 @@ func HandleDeleteAccount(deps model.Dependencies, c model.WebContext) {
103103

104104
id, err := strconv.Atoi(c.Request().PathValue("id"))
105105
if err != nil {
106-
response.SendError(c, http.StatusBadRequest, "invalid id", nil)
106+
response.SendError(c, http.StatusBadRequest, "invalid id")
107107
return
108108
}
109109

110110
err = deps.Domains().Accounts().DeleteAccount(c.Request().Context(), id)
111111
if errors.Is(err, model.ErrNotFound) {
112-
response.SendError(c, http.StatusNotFound, "account not found", nil)
112+
response.SendError(c, http.StatusNotFound, "account not found")
113113
return
114114
}
115115

@@ -119,7 +119,7 @@ func HandleDeleteAccount(deps model.Dependencies, c model.WebContext) {
119119
return
120120
}
121121

122-
response.Send(c, http.StatusNoContent, nil)
122+
response.SendJSON(c, http.StatusNoContent, nil)
123123
}
124124

125125
// @Summary Update an account
@@ -141,13 +141,13 @@ func HandleUpdateAccount(deps model.Dependencies, c model.WebContext) {
141141

142142
accountID, err := strconv.Atoi(c.Request().PathValue("id"))
143143
if err != nil {
144-
response.SendError(c, http.StatusBadRequest, "invalid id", nil)
144+
response.SendError(c, http.StatusBadRequest, "invalid id")
145145
return
146146
}
147147

148148
var payload updateAccountPayload
149149
if err := json.NewDecoder(c.Request().Body).Decode(&payload); err != nil {
150-
response.SendError(c, http.StatusBadRequest, "invalid json", nil)
150+
response.SendError(c, http.StatusBadRequest, "invalid json")
151151
return
152152
}
153153

@@ -156,15 +156,15 @@ func HandleUpdateAccount(deps model.Dependencies, c model.WebContext) {
156156

157157
account, err := deps.Domains().Accounts().UpdateAccount(c.Request().Context(), updatedAccount)
158158
if errors.Is(err, model.ErrNotFound) {
159-
response.SendError(c, http.StatusNotFound, "account not found", nil)
159+
response.SendError(c, http.StatusNotFound, "account not found")
160160
return
161161
}
162162
if errors.Is(err, model.ErrAlreadyExists) {
163-
response.SendError(c, http.StatusConflict, "account already exists", nil)
163+
response.SendError(c, http.StatusConflict, "account already exists")
164164
return
165165
}
166166
if err, isValidationErr := err.(model.ValidationError); isValidationErr {
167-
response.SendError(c, http.StatusBadRequest, err.Error(), nil)
167+
response.SendError(c, http.StatusBadRequest, err.Error())
168168
return
169169
}
170170

@@ -174,5 +174,5 @@ func HandleUpdateAccount(deps model.Dependencies, c model.WebContext) {
174174
return
175175
}
176176

177-
response.Send(c, http.StatusOK, account)
177+
response.SendJSON(c, http.StatusOK, account)
178178
}

‎internal/http/handlers/api/v1/accounts_test.go‎

Lines changed: 43 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,9 @@ func TestHandleListAccounts(t *testing.T) {
5858
HandleListAccounts(deps, c)
5959
require.Equal(t, http.StatusOK, w.Code)
6060

61-
response, err := testutil.NewTestResponseFromReader(w.Body)
62-
require.NoError(t, err)
61+
response := testutil.NewTestResponseFromRecorder(w)
6362
response.AssertOk(t)
64-
require.Len(t, response.Response.Message, 1) // Admin + created account
63+
response.AssertMessageIsListLength(t, 1) // Admin + created account
6564
})
6665
}
6766

@@ -146,11 +145,11 @@ func TestHandleCreateAccount(t *testing.T) {
146145
}, "POST", "/api/v1/accounts", testutil.WithBody(body))
147146
require.Equal(t, http.StatusCreated, w.Code)
148147

149-
response, err := testutil.NewTestResponseFromReader(w.Body)
150-
require.NoError(t, err)
148+
response := testutil.NewTestResponseFromRecorder(w)
151149
response.AssertOk(t)
152-
response.AssertMessageContains(t, "id")
153-
require.NotZero(t, response.Response.Message.(map[string]interface{})["id"])
150+
response.AssertMessageJSONKeyValue(t, "id", func(t *testing.T, value any) {
151+
require.NotZero(t, value)
152+
})
154153
})
155154
}
156155

@@ -279,11 +278,11 @@ func TestHandleUpdateAccount(t *testing.T) {
279278
}, "PATCH", "/api/v1/accounts/"+strconv.Itoa(int(account.ID)), testutil.WithBody(body))
280279
require.Equal(t, http.StatusOK, w.Code)
281280

282-
response, err := testutil.NewTestResponseFromReader(w.Body)
283-
require.NoError(t, err)
281+
response := testutil.NewTestResponseFromRecorder(w)
284282
response.AssertOk(t)
285-
response.AssertMessageContains(t, "owner")
286-
require.True(t, response.Response.Message.(map[string]any)["owner"].(bool))
283+
response.AssertMessageJSONKeyValue(t, "owner", func(t *testing.T, value any) {
284+
require.True(t, value.(bool))
285+
})
287286
})
288287

289288
t.Run("update with empty payload", func(t *testing.T) {
@@ -310,8 +309,7 @@ func TestHandleUpdateAccount(t *testing.T) {
310309
require.Equal(t, http.StatusBadRequest, w.Code)
311310

312311
// Verify no changes were made
313-
response, err := testutil.NewTestResponseFromReader(w.Body)
314-
require.NoError(t, err)
312+
response := testutil.NewTestResponseFromRecorder(w)
315313
response.AssertNotOk(t)
316314
})
317315

@@ -332,10 +330,11 @@ func TestHandleUpdateAccount(t *testing.T) {
332330
}, "PATCH", "/api/v1/accounts/"+strconv.Itoa(int(account.ID)), testutil.WithBody(body))
333331
require.Equal(t, http.StatusOK, w.Code)
334332

335-
response, err := testutil.NewTestResponseFromReader(w.Body)
336-
require.NoError(t, err)
333+
response := testutil.NewTestResponseFromRecorder(w)
337334
response.AssertOk(t)
338-
require.Equal(t, "newname", response.Response.Message.(map[string]any)["username"])
335+
response.AssertMessageJSONKeyValue(t, "username", func(t *testing.T, value any) {
336+
require.Equal(t, "newname", value)
337+
})
339338
})
340339

341340
t.Run("update password only", func(t *testing.T) {
@@ -412,20 +411,20 @@ func TestHandleUpdateAccount(t *testing.T) {
412411
}, "PATCH", "/api/v1/accounts/"+strconv.Itoa(int(account.ID)), testutil.WithBody(body))
413412
require.Equal(t, http.StatusOK, w.Code)
414413

415-
response, err := testutil.NewTestResponseFromReader(w.Body)
416-
require.NoError(t, err)
414+
response := testutil.NewTestResponseFromRecorder(w)
417415
response.AssertOk(t)
418-
419-
config := response.Response.Message.(map[string]any)["config"].(map[string]any)
420-
require.True(t, config["ShowId"].(bool))
421-
require.True(t, config["ListMode"].(bool))
422-
require.True(t, config["HideThumbnail"].(bool))
423-
require.True(t, config["HideExcerpt"].(bool))
424-
require.Equal(t, "dark", config["Theme"])
425-
require.True(t, config["KeepMetadata"].(bool))
426-
require.True(t, config["UseArchive"].(bool))
427-
require.True(t, config["CreateEbook"].(bool))
428-
require.True(t, config["MakePublic"].(bool))
416+
response.AssertMessageJSONKeyValue(t, "config", func(t *testing.T, value any) {
417+
config := value.(map[string]any)
418+
require.True(t, config["ShowId"].(bool))
419+
require.True(t, config["ListMode"].(bool))
420+
require.True(t, config["HideThumbnail"].(bool))
421+
require.True(t, config["HideExcerpt"].(bool))
422+
require.Equal(t, "dark", config["Theme"])
423+
require.True(t, config["KeepMetadata"].(bool))
424+
require.True(t, config["UseArchive"].(bool))
425+
require.True(t, config["CreateEbook"].(bool))
426+
require.True(t, config["MakePublic"].(bool))
427+
})
429428
})
430429

431430
t.Run("update all fields", func(t *testing.T) {
@@ -461,20 +460,22 @@ func TestHandleUpdateAccount(t *testing.T) {
461460
}, "PATCH", "/api/v1/accounts/"+strconv.Itoa(int(account.ID)), testutil.WithBody(body))
462461
require.Equal(t, http.StatusOK, w.Code)
463462

464-
response, err := testutil.NewTestResponseFromReader(w.Body)
465-
require.NoError(t, err)
463+
response := testutil.NewTestResponseFromRecorder(w)
466464
response.AssertOk(t)
467-
468-
msg := response.Response.Message.(map[string]any)
469-
require.Equal(t, "updated", msg["username"])
470-
require.True(t, msg["owner"].(bool))
471-
472-
config := msg["config"].(map[string]any)
473-
require.True(t, config["ShowId"].(bool))
474-
require.True(t, config["ListMode"].(bool))
475-
require.True(t, config["HideThumbnail"].(bool))
476-
require.True(t, config["HideExcerpt"].(bool))
477-
require.Equal(t, "dark", config["Theme"])
465+
response.AssertMessageJSONKeyValue(t, "username", func(t *testing.T, value any) {
466+
require.Equal(t, "updated", value)
467+
})
468+
response.AssertMessageJSONKeyValue(t, "owner", func(t *testing.T, value any) {
469+
require.True(t, value.(bool))
470+
})
471+
response.AssertMessageJSONKeyValue(t, "config", func(t *testing.T, value any) {
472+
config := value.(map[string]any)
473+
require.True(t, config["ShowId"].(bool))
474+
require.True(t, config["ListMode"].(bool))
475+
require.True(t, config["HideThumbnail"].(bool))
476+
require.True(t, config["HideExcerpt"].(bool))
477+
require.Equal(t, "dark", config["Theme"])
478+
})
478479

479480
// Verify password change
480481
loginBody := `{"username": "updated", "password": "newpass"}`

‎internal/http/handlers/api/v1/auth.go‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,18 @@ type loginResponseMessage struct {
4343
func HandleLogin(deps model.Dependencies, c model.WebContext) {
4444
var payload loginRequestPayload
4545
if err := json.NewDecoder(c.Request().Body).Decode(&payload); err != nil {
46-
response.SendError(c, http.StatusBadRequest, "Invalid JSON payload", nil)
46+
response.SendError(c, http.StatusBadRequest, "Invalid JSON payload")
4747
return
4848
}
4949

5050
if err := payload.IsValid(); err != nil {
51-
response.SendError(c, http.StatusBadRequest, err.Error(), nil)
51+
response.SendError(c, http.StatusBadRequest, err.Error())
5252
return
5353
}
5454

5555
account, err := deps.Domains().Auth().GetAccountFromCredentials(c.Request().Context(), payload.Username, payload.Password)
5656
if err != nil {
57-
response.SendError(c, http.StatusBadRequest, err.Error(), nil)
57+
response.SendError(c, http.StatusBadRequest, err.Error())
5858
return
5959
}
6060

@@ -71,7 +71,7 @@ func HandleLogin(deps model.Dependencies, c model.WebContext) {
7171
return
7272
}
7373

74-
response.Send(c, http.StatusOK, loginResponseMessage{
74+
response.SendJSON(c, http.StatusOK, loginResponseMessage{
7575
Token: token,
7676
Expiration: expirationTime.Unix(),
7777
})
@@ -89,16 +89,17 @@ func HandleRefreshToken(deps model.Dependencies, c model.WebContext) {
8989
return
9090
}
9191

92-
expiration := time.Now().Add(time.Hour * 72)
92+
expiration := time.Now().UTC().Add(time.Hour * 24 * 30)
9393
account := c.GetAccount()
9494
token, err := deps.Domains().Auth().CreateTokenForAccount(account, expiration)
9595
if err != nil {
9696
response.SendInternalServerError(c)
9797
return
9898
}
9999

100-
response.Send(c, http.StatusAccepted, loginResponseMessage{
101-
Token: token,
100+
response.SendJSON(c, http.StatusAccepted, loginResponseMessage{
101+
Token: token,
102+
Expiration: expiration.Unix(),
102103
})
103104
}
104105

@@ -113,7 +114,7 @@ func HandleGetMe(deps model.Dependencies, c model.WebContext) {
113114
if err := middleware.RequireLoggedInUser(deps, c); err != nil {
114115
return
115116
}
116-
response.Send(c, http.StatusOK, c.GetAccount())
117+
response.SendJSON(c, http.StatusOK, c.GetAccount())
117118
}
118119

119120
type updateAccountPayload struct {
@@ -132,7 +133,9 @@ func (p *updateAccountPayload) IsValid() error {
132133
}
133134

134135
func (p *updateAccountPayload) ToAccountDTO() model.AccountDTO {
135-
account := model.AccountDTO{}
136+
account := model.AccountDTO{
137+
Config: p.Config,
138+
}
136139
if p.NewPassword != "" {
137140
account.Password = p.NewPassword
138141
}
@@ -168,7 +171,7 @@ func HandleUpdateLoggedAccount(deps model.Dependencies, c model.WebContext) {
168171
}
169172

170173
if err := payload.IsValid(); err != nil {
171-
response.SendError(c, http.StatusBadRequest, err.Error(), nil)
174+
response.SendError(c, http.StatusBadRequest, err.Error())
172175
return
173176
}
174177

@@ -177,11 +180,14 @@ func HandleUpdateLoggedAccount(deps model.Dependencies, c model.WebContext) {
177180
if payload.NewPassword != "" {
178181
_, err := deps.Domains().Auth().GetAccountFromCredentials(c.Request().Context(), account.Username, payload.OldPassword)
179182
if err != nil {
180-
response.SendError(c, http.StatusBadRequest, "Old password is incorrect", nil)
183+
response.SendError(c, http.StatusBadRequest, "Old password is incorrect")
181184
return
182185
}
183186
}
184187

188+
// TODO: Use a method in the AccountDTO to apply the updates directly:
189+
// account := domains.Accounts().GetAccount(...)
190+
// account.ApplyUpdates(payload)
185191
updatedAccount := payload.ToAccountDTO()
186192
updatedAccount.ID = account.ID
187193

@@ -192,7 +198,7 @@ func HandleUpdateLoggedAccount(deps model.Dependencies, c model.WebContext) {
192198
return
193199
}
194200

195-
response.Send(c, http.StatusOK, account)
201+
response.SendJSON(c, http.StatusOK, account)
196202
}
197203

198204
// @Summary Logout from the current session
@@ -213,5 +219,5 @@ func HandleLogout(deps model.Dependencies, c model.WebContext) {
213219
Value: "",
214220
})
215221

216-
response.Send(c, http.StatusOK, nil)
222+
response.SendJSON(c, http.StatusOK, nil)
217223
}

0 commit comments

Comments
 (0)