Skip to content

Commit 78c6dbe

Browse files
committed
Refactoring
1 parent 375718c commit 78c6dbe

9 files changed

Lines changed: 160 additions & 206 deletions

File tree

internal/api/submit/request.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ type System struct {
66

77
type OS struct {
88
Architecture string `json:"architecture"`
9-
Id string `json:"id"`
9+
ID string `json:"id"`
1010
}
1111

1212
type Pacman struct {
@@ -31,7 +31,7 @@ type PackageManager interface {
3131
type SystemInfo interface {
3232
GetCpuArchitecture() (string, error)
3333
GetArchitecture() (string, error)
34-
GetOSID(...string) (string, error)
34+
GetOSID() (string, error)
3535
}
3636

3737
func CreateRequest(p PackageManager, s SystemInfo) (*Request, error) {
@@ -61,7 +61,7 @@ func CreateRequest(p PackageManager, s SystemInfo) (*Request, error) {
6161
return &Request{
6262
Version: Version,
6363
System: System{Architecture: cpuArchitecture},
64-
OS: OS{Architecture: architecture, Id: osId},
64+
OS: OS{Architecture: architecture, ID: osId},
6565
Pacman: Pacman{Packages: packages, Mirror: mirror},
6666
}, nil
6767
}

internal/system/system_goos.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@ func (s *System) GetArchitecture() (string, error) {
1717
}
1818
}
1919

20-
func (s *System) GetOSID(_ ...string) (string, error) {
20+
func (s *System) GetOSID() (string, error) {
2121
return runtime.GOOS, nil
2222
}

internal/system/system_linux.go

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,42 +19,42 @@ func (s *System) GetArchitecture() (string, error) {
1919
return string(utsname.Machine[:bytes.IndexByte(utsname.Machine[:], 0)]), nil
2020
}
2121

22-
func (s *System) GetOSID(osReleasePaths ...string) (string, error) {
23-
var id string
24-
25-
if len(osReleasePaths) == 0 {
26-
osReleasePaths = []string{"/etc/os-release", "/usr/lib/os-release"}
27-
}
28-
29-
for _, osReleasePath := range osReleasePaths {
30-
content, err := os.ReadFile(osReleasePath)
22+
func (s *System) GetOSID() (string, error) {
23+
for _, path := range []string{"/etc/os-release", "/usr/lib/os-release"} {
24+
content, err := os.ReadFile(path)
3125
if err != nil {
3226
continue
3327
}
34-
35-
lines := strings.Split(string(content), "\n")
36-
const keyValueParts = 2
37-
for _, line := range lines {
38-
if trimmedLine := strings.TrimSpace(line); trimmedLine == "" || strings.HasPrefix(trimmedLine, "#") {
39-
continue
40-
}
41-
42-
parts := strings.SplitN(line, "=", keyValueParts)
43-
if len(parts) != keyValueParts {
44-
continue
45-
}
46-
47-
key := strings.TrimSpace(parts[0])
48-
if key == "ID" {
49-
id = strings.Trim(strings.TrimSpace(parts[1]), `"'`)
50-
}
28+
if id := ParseOSID(content); id != "" {
29+
return id, nil
5130
}
5231
break
5332
}
5433

55-
if id == "" {
56-
id = runtime.GOOS
34+
return runtime.GOOS, nil
35+
}
36+
37+
// ParseOSID parses the ID field from os-release file content.
38+
func ParseOSID(content []byte) string {
39+
var id string
40+
41+
lines := strings.Split(string(content), "\n")
42+
const keyValueParts = 2
43+
for _, line := range lines {
44+
if trimmedLine := strings.TrimSpace(line); trimmedLine == "" || strings.HasPrefix(trimmedLine, "#") {
45+
continue
46+
}
47+
48+
parts := strings.SplitN(line, "=", keyValueParts)
49+
if len(parts) != keyValueParts {
50+
continue
51+
}
52+
53+
key := strings.TrimSpace(parts[0])
54+
if key == "ID" {
55+
id = strings.Trim(strings.TrimSpace(parts[1]), `"'`)
56+
}
5757
}
5858

59-
return id, nil
59+
return id
6060
}

tests/integration/integration_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ func TestShowInformationToBeSent(t *testing.T) {
6565
if request.OS.Architecture != osArchitecture {
6666
t.Errorf("Expected OS architecture '%s', got %v", osArchitecture, request.OS.Architecture)
6767
}
68-
if request.OS.Id != osId {
69-
t.Errorf("Expected OS id '%s', got %v", osId, request.OS.Id)
68+
if request.OS.ID != osId {
69+
t.Errorf("Expected OS id '%s', got %v", osId, request.OS.ID)
7070
}
7171
if !strings.HasPrefix(request.Pacman.Mirror, "https://") {
7272
t.Errorf("Expected pacman mirror to start with 'https://', got %v", request.Pacman.Mirror)

tests/integration/mock_server_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ func validateSubmitRequest(w http.ResponseWriter, request *submit.Request) {
122122
http.Error(w, fmt.Sprintf("Expected OS architecture %s, got %s", osArchitecture, request.OS.Architecture), http.StatusBadRequest)
123123
return
124124
}
125-
if request.OS.Id != osId {
126-
http.Error(w, fmt.Sprintf("Expected OS id %s, got %s", osId, request.OS.Id), http.StatusBadRequest)
125+
if request.OS.ID != osId {
126+
http.Error(w, fmt.Sprintf("Expected OS id %s, got %s", osId, request.OS.ID), http.StatusBadRequest)
127127
return
128128
}
129129
if request.System.Architecture != cpuArchitecture {

tests/unit/internal/api/submit/client_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func TestSendRequest(t *testing.T) {
3333
request := &submit.Request{
3434
Version: submit.Version,
3535
System: submit.System{Architecture: system.X86_64},
36-
OS: submit.OS{Architecture: system.I686, Id: runtime.GOOS},
36+
OS: submit.OS{Architecture: system.I686, ID: runtime.GOOS},
3737
Pacman: submit.Pacman{Packages: []string{"pacman", "linux"}, Mirror: mirror},
3838
}
3939
err := client.SendRequest(*request)
@@ -75,7 +75,7 @@ func validateRequest(t *testing.T, req *http.Request) {
7575
if request.OS.Architecture != system.I686 {
7676
t.Error("Invalid arch value")
7777
}
78-
if request.OS.Id != runtime.GOOS {
78+
if request.OS.ID != runtime.GOOS {
7979
t.Error("Invalid id value")
8080
}
8181
if request.Pacman.Mirror != mirror {
@@ -105,7 +105,7 @@ func TestSendRequestFollowsRedirect(t *testing.T) {
105105
request := &submit.Request{
106106
Version: submit.Version,
107107
System: submit.System{Architecture: system.X86_64},
108-
OS: submit.OS{Architecture: system.I686, Id: runtime.GOOS},
108+
OS: submit.OS{Architecture: system.I686, ID: runtime.GOOS},
109109
Pacman: submit.Pacman{Packages: []string{"pacman", "linux"}, Mirror: mirror},
110110
}
111111
err := client.SendRequest(*request)
@@ -132,7 +132,7 @@ func TestReturnServerErrorOnFailure(t *testing.T) {
132132
request := &submit.Request{
133133
Version: submit.Version,
134134
System: submit.System{Architecture: system.X86_64},
135-
OS: submit.OS{Architecture: system.I686, Id: runtime.GOOS},
135+
OS: submit.OS{Architecture: system.I686, ID: runtime.GOOS},
136136
Pacman: submit.Pacman{Packages: []string{"pacman", "linux"}, Mirror: mirror},
137137
}
138138
err := client.SendRequest(*request)
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
//go:build linux
2+
3+
package system_test
4+
5+
import (
6+
"testing"
7+
8+
"pkgstats-cli/internal/system"
9+
)
10+
11+
func TestParseOSID(t *testing.T) {
12+
testCases := []struct {
13+
name string
14+
content string
15+
expectedOSID string
16+
}{
17+
{
18+
name: "should return ID from simple content",
19+
content: `
20+
NAME="Test OS"
21+
VERSION="1.0"
22+
ID=testos
23+
ID_LIKE=anotheros
24+
`,
25+
expectedOSID: "testos",
26+
},
27+
{
28+
name: "should return ID from double-quoted syntax with whitespaces",
29+
content: `
30+
NAME="Test OS"
31+
VERSION="1.0"
32+
ID = "testos"
33+
ID_LIKE=anotheros
34+
`,
35+
expectedOSID: "testos",
36+
},
37+
{
38+
name: "should return ID with whitespaces",
39+
content: `
40+
NAME="Test OS"
41+
VERSION="1.0"
42+
ID = testos
43+
ID_LIKE=anotheros
44+
`,
45+
expectedOSID: "testos",
46+
},
47+
{
48+
name: "should return ID from single-quoted syntax",
49+
content: `
50+
NAME="Test OS"
51+
VERSION="1.0"
52+
ID='testos'
53+
ID_LIKE=anotheros
54+
`,
55+
expectedOSID: "testos",
56+
},
57+
{
58+
name: "should return the last ID when duplicates exist",
59+
content: `
60+
ID=firstid
61+
NAME="Test OS"
62+
ID=secondid
63+
VERSION="1.0"
64+
ID=lastid
65+
`,
66+
expectedOSID: "lastid",
67+
},
68+
{
69+
name: "should return empty string for empty content",
70+
content: "",
71+
expectedOSID: "",
72+
},
73+
{
74+
name: "should return empty string for content with no ID",
75+
content: `
76+
NAME="Test OS"
77+
VERSION="1.0"
78+
`,
79+
expectedOSID: "",
80+
},
81+
{
82+
name: "should ignore comments",
83+
content: `
84+
#ID=commented
85+
NAME="Test OS"
86+
ID=actual
87+
`,
88+
expectedOSID: "actual",
89+
},
90+
{
91+
name: "should handle ID with embedded equals",
92+
content: `
93+
ID=some=value
94+
`,
95+
expectedOSID: "some=value",
96+
},
97+
}
98+
99+
for _, tc := range testCases {
100+
t.Run(tc.name, func(t *testing.T) {
101+
osID := system.ParseOSID([]byte(tc.content))
102+
if osID != tc.expectedOSID {
103+
t.Errorf("expected OSID %q, got %q", tc.expectedOSID, osID)
104+
}
105+
})
106+
}
107+
}

tests/unit/internal/system/system_goos_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestGetArchitecture(t *testing.T) {
2020
}
2121

2222
func TestGetOSID(t *testing.T) {
23-
osId, err := system.NewSystem().GetOSID("")
23+
osId, err := system.NewSystem().GetOSID()
2424
if err != nil {
2525
t.Error(err)
2626
}

0 commit comments

Comments
 (0)