Skip to content

Commit a740c1e

Browse files
man90esman90es
authored
Improve validation error messages (#33)
* Improve validation error messages * Bump version number
1 parent d9b5ae7 commit a740c1e

15 files changed

+132
-96
lines changed

‎handlers/catchall.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ import (
55
)
66

77
func catchall(w http.ResponseWriter, r *http.Request) {
8-
giveBadRequestResponse(w)
8+
giveBadRequestResponse(w, "Requested route is invalid. See documentation "+docsLink)
99
}

‎handlers/getAdventurer.go

+8-4
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,15 @@ import (
1313
var profilesCache = cache.NewCache[models.Profile]()
1414

1515
func getAdventurer(w http.ResponseWriter, r *http.Request) {
16-
profileTarget, profileTargetOk := validators.ValidateProfileTargetQueryParam(r.URL.Query()["profileTarget"])
17-
region, regionOk := validators.ValidateRegionQueryParam(r.URL.Query()["region"])
16+
profileTarget, profileTargetOk, profileTargetValidationMessage := validators.ValidateProfileTargetQueryParam(r.URL.Query()["profileTarget"])
17+
if !profileTargetOk {
18+
giveBadRequestResponse(w, profileTargetValidationMessage)
19+
return
20+
}
1821

19-
if !profileTargetOk || !regionOk {
20-
giveBadRequestResponse(w)
22+
region, regionOk, regionValidationMessage := validators.ValidateRegionQueryParam(r.URL.Query()["region"])
23+
if !regionOk {
24+
giveBadRequestResponse(w, regionValidationMessage)
2125
return
2226
}
2327

‎handlers/getAdventurerSearch.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ import (
1515
var profileSearchCache = cache.NewCache[[]models.Profile]()
1616

1717
func getAdventurerSearch(w http.ResponseWriter, r *http.Request) {
18-
region, regionOk := validators.ValidateRegionQueryParam(r.URL.Query()["region"])
18+
region, regionOk, regionValidationMessage := validators.ValidateRegionQueryParam(r.URL.Query()["region"])
1919
if !regionOk {
20-
giveBadRequestResponse(w)
20+
giveBadRequestResponse(w, regionValidationMessage)
2121
return
2222
}
2323

24-
query, queryOk := validators.ValidateAdventurerNameQueryParam(r.URL.Query()["query"], region)
24+
query, queryOk, queryValidationMessage := validators.ValidateAdventurerNameQueryParam(r.URL.Query()["query"], region)
2525
if !queryOk {
26-
giveBadRequestResponse(w)
26+
giveBadRequestResponse(w, queryValidationMessage)
2727
return
2828
}
2929

‎handlers/getGuild.go

+8-4
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,15 @@ import (
1414
var guildProfilesCache = cache.NewCache[models.GuildProfile]()
1515

1616
func getGuild(w http.ResponseWriter, r *http.Request) {
17-
name, nameOk := validators.ValidateGuildNameQueryParam(r.URL.Query()["guildName"])
18-
region, regionOk := validators.ValidateRegionQueryParam(r.URL.Query()["region"])
17+
name, nameOk, nameValidationMessage := validators.ValidateGuildNameQueryParam(r.URL.Query()["guildName"])
18+
if !nameOk {
19+
giveBadRequestResponse(w, nameValidationMessage)
20+
return
21+
}
1922

20-
if !nameOk || !regionOk {
21-
giveBadRequestResponse(w)
23+
region, regionOk, regionValidationMessage := validators.ValidateRegionQueryParam(r.URL.Query()["region"])
24+
if !regionOk {
25+
giveBadRequestResponse(w, regionValidationMessage)
2226
return
2327
}
2428

‎handlers/getGuildSearch.go

+9-4
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@ import (
1515
var guildSearchCache = cache.NewCache[[]models.GuildProfile]()
1616

1717
func getGuildSearch(w http.ResponseWriter, r *http.Request) {
18-
name, nameOk := validators.ValidateGuildNameQueryParam(r.URL.Query()["query"])
18+
name, nameOk, nameValidationMessage := validators.ValidateGuildNameQueryParam(r.URL.Query()["query"])
19+
if !nameOk {
20+
giveBadRequestResponse(w, nameValidationMessage)
21+
return
22+
}
23+
1924
page := validators.ValidatePageQueryParam(r.URL.Query()["page"])
20-
region, regionOk := validators.ValidateRegionQueryParam(r.URL.Query()["region"])
2125

22-
if !nameOk || !regionOk {
23-
giveBadRequestResponse(w)
26+
region, regionOk, regionValidationMessage := validators.ValidateRegionQueryParam(r.URL.Query()["region"])
27+
if !regionOk {
28+
giveBadRequestResponse(w, regionValidationMessage)
2429
return
2530
}
2631

‎handlers/getStatus.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
)
1111

1212
var initTime = time.Now()
13-
var version = "1.9.1"
13+
var version = "1.9.3"
1414

1515
func getStatus(w http.ResponseWriter, r *http.Request) {
1616
json.NewEncoder(w).Encode(map[string]interface{}{

‎handlers/giveBadRequestResponse.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ import (
77

88
const docsLink = "https://man90es.github.io/BDO-REST-API"
99

10-
func giveBadRequestResponse(w http.ResponseWriter) {
10+
func giveBadRequestResponse(w http.ResponseWriter, message string) {
1111
w.WriteHeader(http.StatusBadRequest)
1212

1313
json.NewEncoder(w).Encode(map[string]string{
14-
"message": "Route or parameter is invalid. See documentation " + docsLink,
14+
"message": message,
1515
})
1616
}

‎validators/ValidateAdventurerNameQueryParam.go

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
package validators
22

33
import (
4+
"fmt"
45
"strings"
56
"unicode"
67
)
78

89
// The naming policies in BDO are fucked up
910
// This function only checks the length and allowed symbols
10-
func ValidateAdventurerNameQueryParam(query []string, region string) (name string, ok bool) {
11+
func ValidateAdventurerNameQueryParam(query []string, region string) (name string, ok bool, errorMessage string) {
1112
if 1 > len(query) {
12-
return "", false
13+
return "", false, "Adventurer name is missing from request"
1314
}
1415

1516
minLength := map[string]int{
@@ -19,7 +20,7 @@ func ValidateAdventurerNameQueryParam(query []string, region string) (name strin
1920
}[region]
2021

2122
if len(query[0]) < minLength || len(query[0]) > 16 {
22-
return query[0], false
23+
return query[0], false, fmt.Sprintf("Adventurer name should be between %v and 16 symbols long", minLength)
2324
}
2425

2526
// Returns false for allowed characters
@@ -48,5 +49,9 @@ func ValidateAdventurerNameQueryParam(query []string, region string) (name strin
4849
return true
4950
}
5051

51-
return query[0], strings.IndexFunc(query[0], f) == -1
52+
if i := strings.IndexFunc(query[0], f); i != -1 {
53+
return query[0], false, fmt.Sprintf("Adventurer name contains a forbidden symbol at position %v: %q", i+1, query[0][i])
54+
}
55+
56+
return query[0], true, ""
5257
}

‎validators/ValidateAdventurerNameQueryParam_test.go

+21-20
Original file line numberDiff line numberDiff line change
@@ -4,32 +4,33 @@ import "testing"
44

55
func TestValidateAdventurerNameQueryParam(t *testing.T) {
66
tests := []struct {
7-
expectedName string
8-
expectedOk bool
9-
input []string
10-
region string
7+
expectedName string
8+
expectedOk bool
9+
expectedMessage string
10+
input []string
11+
region string
1112
}{
12-
{input: []string{"1Number"}, region: "EU", expectedName: "1Number", expectedOk: true}, // Starts with a number
13-
{input: []string{"Adventurer_123"}, region: "EU", expectedName: "Adventurer_123", expectedOk: true},
14-
{input: []string{"JohnDoe"}, region: "EU", expectedName: "JohnDoe", expectedOk: true},
15-
{input: []string{"Name1", "Name2"}, region: "EU", expectedName: "Name1", expectedOk: true},
16-
{input: []string{"고대신"}, region: "EU", expectedName: "고대신", expectedOk: true}, // Adventurer name with Korean characters
13+
{input: []string{"1Number"}, region: "EU", expectedName: "1Number", expectedOk: true, expectedMessage: ""},
14+
{input: []string{"Adventurer_123"}, region: "EU", expectedName: "Adventurer_123", expectedOk: true, expectedMessage: ""},
15+
{input: []string{"JohnDoe"}, region: "EU", expectedName: "JohnDoe", expectedOk: true, expectedMessage: ""},
16+
{input: []string{"Name1", "Name2"}, region: "EU", expectedName: "Name1", expectedOk: true, expectedMessage: ""},
17+
{input: []string{"고대신"}, region: "EU", expectedName: "고대신", expectedOk: true, expectedMessage: ""},
1718

18-
{input: []string{""}, region: "EU", expectedName: "", expectedOk: false}, // Empty adventurer name
19-
{input: []string{"Ad"}, region: "EU", expectedName: "Ad", expectedOk: false}, // Too short
20-
{input: []string{"Adventurer With Spaces"}, region: "EU", expectedName: "Adventurer With Spaces", expectedOk: false}, // Contains spaces
21-
{input: []string{"AdventurerNameTooLong12345"}, region: "EU", expectedName: "AdventurerNameTooLong12345", expectedOk: false}, // Too long
22-
{input: []string{"Name$"}, region: "EU", expectedName: "Name$", expectedOk: false}, // Contains an invalid symbol
23-
{input: []string{}, region: "EU", expectedName: "", expectedOk: false},
19+
{input: []string{""}, region: "EU", expectedName: "", expectedOk: false, expectedMessage: "Adventurer name should be between 3 and 16 symbols long"},
20+
{input: []string{"Ad"}, region: "EU", expectedName: "Ad", expectedOk: false, expectedMessage: "Adventurer name should be between 3 and 16 symbols long"},
21+
{input: []string{"With Spaces"}, region: "EU", expectedName: "With Spaces", expectedOk: false, expectedMessage: "Adventurer name contains a forbidden symbol at position 5: ' '"},
22+
{input: []string{"AdventurerNameTooLong12345"}, region: "EU", expectedName: "AdventurerNameTooLong12345", expectedOk: false, expectedMessage: "Adventurer name should be between 3 and 16 symbols long"},
23+
{input: []string{"Name$"}, region: "EU", expectedName: "Name$", expectedOk: false, expectedMessage: "Adventurer name contains a forbidden symbol at position 5: '$'"},
24+
{input: []string{}, region: "EU", expectedName: "", expectedOk: false, expectedMessage: "Adventurer name is missing from request"},
2425

25-
{input: []string{""}, region: "SA", expectedName: "", expectedOk: false},
26-
{input: []string{"Ad"}, region: "SA", expectedName: "Ad", expectedOk: true},
26+
{input: []string{""}, region: "SA", expectedName: "", expectedOk: false, expectedMessage: "Adventurer name should be between 2 and 16 symbols long"},
27+
{input: []string{"Ad"}, region: "SA", expectedName: "Ad", expectedOk: true, expectedMessage: ""},
2728
}
2829

2930
for _, test := range tests {
30-
name, ok := ValidateAdventurerNameQueryParam(test.input, test.region)
31-
if name != test.expectedName || ok != test.expectedOk {
32-
t.Errorf("Input: %v %v, Expected: %v %v, Got: %v %v", test.input, test.region, test.expectedName, test.expectedOk, name, ok)
31+
name, ok, message := ValidateAdventurerNameQueryParam(test.input, test.region)
32+
if name != test.expectedName || ok != test.expectedOk || message != test.expectedMessage {
33+
t.Errorf("Input: %v %v, Expected: %v %v %v, Got: %v %v %v", test.input, test.region, test.expectedName, test.expectedOk, test.expectedMessage, name, ok, message)
3334
}
3435
}
3536
}

‎validators/ValidateGuildNameQueryParam.go

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,23 @@
11
package validators
22

33
import (
4+
"fmt"
45
"strings"
56
"unicode"
67
)
78

89
// The naming policies in BDO are fucked up
910
// This function only checks the length and allowed symbols
1011
// I also assumed that the allowed symbols are the same as for adventurer names
11-
func ValidateGuildNameQueryParam(query []string) (guildName string, ok bool) {
12+
func ValidateGuildNameQueryParam(query []string) (guildName string, ok bool, errorMessage string) {
1213
if 1 > len(query) {
13-
return "", false
14+
return "", false, "Guild name is missing from request"
1415
}
1516

1617
guildName = strings.ToLower(query[0])
1718

1819
if len(guildName) < 2 {
19-
return guildName, false
20+
return guildName, false, "Guild name can't be shorter than 2 symbols"
2021
}
2122

2223
// Returns false for allowed characters
@@ -45,5 +46,9 @@ func ValidateGuildNameQueryParam(query []string) (guildName string, ok bool) {
4546
return true
4647
}
4748

48-
return guildName, strings.IndexFunc(guildName, f) == -1
49+
if i := strings.IndexFunc(guildName, f); i != -1 {
50+
return guildName, false, fmt.Sprintf("Guild name contains a forbidden symbol at position %v: %q", i+1, guildName[i])
51+
}
52+
53+
return guildName, true, ""
4954
}

‎validators/ValidateGuildNameQueryParam_test.go

+17-16
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,28 @@ import "testing"
44

55
func TestValidateGuildNameQueryParam(t *testing.T) {
66
tests := []struct {
7-
expectedName string
8-
expectedOk bool
9-
input []string
7+
expectedName string
8+
expectedOk bool
9+
expectedMessage string
10+
input []string
1011
}{
11-
{input: []string{"1NumberGuild"}, expectedName: "1numberguild", expectedOk: true}, // Contains a number
12-
{input: []string{"Adventure_Guild"}, expectedName: "adventure_guild", expectedOk: true},
13-
{input: []string{"FirstGuild", "SecondGuild"}, expectedName: "firstguild", expectedOk: true},
14-
{input: []string{"MyGuild"}, expectedName: "myguild", expectedOk: true},
15-
{input: []string{"고대신"}, expectedName: "고대신", expectedOk: true}, // Guild name with Korean characters
12+
{input: []string{"1NumberGuild"}, expectedName: "1numberguild", expectedOk: true, expectedMessage: ""}, // Contains a number
13+
{input: []string{"Adventure_Guild"}, expectedName: "adventure_guild", expectedOk: true, expectedMessage: ""},
14+
{input: []string{"FirstGuild", "SecondGuild"}, expectedName: "firstguild", expectedOk: true, expectedMessage: ""},
15+
{input: []string{"MyGuild"}, expectedName: "myguild", expectedOk: true, expectedMessage: ""},
16+
{input: []string{"고대신"}, expectedName: "고대신", expectedOk: true, expectedMessage: ""}, // Guild name with Korean characters
1617

17-
{input: []string{""}, expectedName: "", expectedOk: false}, // Empty guild name
18-
{input: []string{"A Guild With Spaces"}, expectedName: "a guild with spaces", expectedOk: false}, // Contains spaces
19-
{input: []string{"Some$"}, expectedName: "some$", expectedOk: false}, // Contains an invalid symbol
20-
{input: []string{"x"}, expectedName: "x", expectedOk: false}, // Too short
21-
{input: []string{}, expectedName: "", expectedOk: false},
18+
{input: []string{""}, expectedName: "", expectedOk: false, expectedMessage: "Guild name can't be shorter than 2 symbols"},
19+
{input: []string{"A Guild With Spaces"}, expectedName: "a guild with spaces", expectedOk: false, expectedMessage: "Guild name contains a forbidden symbol at position 2: ' '"},
20+
{input: []string{"Some$"}, expectedName: "some$", expectedOk: false, expectedMessage: "Guild name contains a forbidden symbol at position 5: '$'"},
21+
{input: []string{"x"}, expectedName: "x", expectedOk: false, expectedMessage: "Guild name can't be shorter than 2 symbols"},
22+
{input: []string{}, expectedName: "", expectedOk: false, expectedMessage: "Guild name is missing from request"},
2223
}
2324

2425
for _, test := range tests {
25-
name, ok := ValidateGuildNameQueryParam(test.input)
26-
if name != test.expectedName || ok != test.expectedOk {
27-
t.Errorf("Input: %v, Expected: %v %v, Got: %v %v", test.input, test.expectedName, test.expectedOk, name, ok)
26+
name, ok, message := ValidateGuildNameQueryParam(test.input)
27+
if name != test.expectedName || ok != test.expectedOk || message != test.expectedMessage {
28+
t.Errorf("Input: %v, Expected: %v %v %v, Got: %v %v %v", test.input, test.expectedName, test.expectedOk, test.expectedMessage, name, ok, message)
2829
}
2930
}
3031
}

‎validators/ValidateProfileTargetQueryParam.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,14 @@ package validators
22

33
// Check that the length is at least 150 characters
44
// I don't actually know how long it should be, but the length varies
5-
func ValidateProfileTargetQueryParam(query []string) (profileTarget string, ok bool) {
5+
func ValidateProfileTargetQueryParam(query []string) (profileTarget string, ok bool, errorMessage string) {
66
if 1 > len(query) {
7-
return "", false
7+
return "", false, "Profile target is missing from the request"
88
}
99

10-
return query[0], len(query[0]) >= 150
10+
if ok := len(query[0]) >= 150; ok {
11+
return query[0], true, ""
12+
}
13+
14+
return query[0], false, "Profile target has to be at least 150 characters long"
1115
}

‎validators/ValidateProfileTargetQueryParam_test.go

+14-13
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,31 @@ import "testing"
44

55
func TestValidateProfileTargetQueryParam(t *testing.T) {
66
tests := []struct {
7-
expectedOk bool
8-
expectedPT string
9-
input []string
7+
expectedOk bool
8+
expectedPT string
9+
expectedMessage string
10+
input []string
1011
}{
1112
// Valid profile targets with lengths >= 150
12-
{input: []string{repeat("A", 150)}, expectedPT: repeat("A", 150), expectedOk: true},
13-
{input: []string{repeat("A", 200)}, expectedPT: repeat("A", 200), expectedOk: true},
13+
{input: []string{repeat("A", 150)}, expectedPT: repeat("A", 150), expectedOk: true, expectedMessage: ""},
14+
{input: []string{repeat("A", 200)}, expectedPT: repeat("A", 200), expectedOk: true, expectedMessage: ""},
1415

1516
// Invalid profile targets with lengths < 150
16-
{input: []string{""}, expectedPT: "", expectedOk: false},
17-
{input: []string{"Short"}, expectedPT: "Short", expectedOk: false},
18-
{input: []string{repeat("A", 149)}, expectedPT: repeat("A", 149), expectedOk: false},
17+
{input: []string{""}, expectedPT: "", expectedOk: false, expectedMessage: "Profile target has to be at least 150 characters long"},
18+
{input: []string{"Short"}, expectedPT: "Short", expectedOk: false, expectedMessage: "Profile target has to be at least 150 characters long"},
19+
{input: []string{repeat("A", 149)}, expectedPT: repeat("A", 149), expectedOk: false, expectedMessage: "Profile target has to be at least 150 characters long"},
1920

2021
// Query param not provided
21-
{input: []string{}, expectedPT: "", expectedOk: false},
22+
{input: []string{}, expectedPT: "", expectedOk: false, expectedMessage: "Profile target is missing from the request"},
2223

2324
// Several profileTargets provided
24-
{input: []string{repeat("A", 150), repeat("B", 150)}, expectedPT: repeat("A", 150), expectedOk: true},
25+
{input: []string{repeat("A", 150), repeat("B", 150)}, expectedPT: repeat("A", 150), expectedOk: true, expectedMessage: ""},
2526
}
2627

2728
for _, test := range tests {
28-
pT, ok := ValidateProfileTargetQueryParam(test.input)
29-
if pT != test.expectedPT || ok != test.expectedOk {
30-
t.Errorf("Input: %v, Expected: %v %v, Got: %v %v", test.input, test.expectedPT, test.expectedOk, pT, ok)
29+
pT, ok, message := ValidateProfileTargetQueryParam(test.input)
30+
if pT != test.expectedPT || ok != test.expectedOk || message != test.expectedMessage {
31+
t.Errorf("Input: %v, Expected: %v %v %v, Got: %v %v %v", test.input, test.expectedPT, test.expectedOk, test.expectedMessage, pT, ok, message)
3132
}
3233
}
3334
}

‎validators/ValidateRegionQueryParam.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
package validators
22

33
import (
4+
"fmt"
45
"slices"
56
"strings"
67
)
78

8-
func ValidateRegionQueryParam(query []string) (region string, ok bool) {
9+
func ValidateRegionQueryParam(query []string) (region string, ok bool, errorMessage string) {
910
if 1 > len(query) {
10-
return "EU", true
11+
return "EU", true, ""
1112
}
1213

1314
region = strings.ToUpper(query[0])
1415

1516
// TODO: Add KR region once the translations are ready
16-
return region, slices.Contains([]string{"EU", "NA", "SA"}, region)
17+
if !slices.Contains([]string{"EU", "NA", "SA"}, region) {
18+
return region, false, fmt.Sprintf("Region %v is not supported", region)
19+
}
20+
21+
return region, true, ""
1722
}

0 commit comments

Comments
 (0)