Skip to content

Commit f7be606

Browse files
committed
feat: Fix header validation issues, migrations and tests
1 parent 6d262bb commit f7be606

17 files changed

Lines changed: 286 additions & 169 deletions

File tree

backend/plugins/bitbucket/api/connection_api_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -206,22 +206,22 @@ func TestMergeFromRequest_HandlesUsesApiToken(t *testing.T) {
206206
// After merge, UsesApiToken should be updated
207207
// This is a structural test - actual merge logic is in the connection.go MergeFromRequest method
208208
assert.True(t, connection.UsesApiToken, "Initial value should be true")
209-
209+
210210
// If we were to apply the merge:
211211
connection.UsesApiToken = newValues["usesApiToken"].(bool)
212212
connection.Username = newValues["username"].(string)
213-
213+
214214
assert.False(t, connection.UsesApiToken, "After merge, should be false")
215215
assert.Equal(t, "new_username", connection.Username)
216216
}
217217

218218
func TestConnectionStatusCodes(t *testing.T) {
219219
// Test expected status code handling
220220
tests := []struct {
221-
name string
222-
statusCode int
223-
expectedError bool
224-
errorType string
221+
name string
222+
statusCode int
223+
expectedError bool
224+
errorType string
225225
}{
226226
{
227227
name: "Success - 200 OK",

backend/plugins/claude_code/api/connection.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,10 @@ func validateConnection(connection *models.ClaudeCodeConnection) errors.Error {
9696
return errors.BadInput.New("connection is required")
9797
}
9898
hasToken := strings.TrimSpace(connection.Token) != ""
99-
hasCustomHeaders := len(connection.CustomHeaders) > 0
99+
if connection.HasIncompleteCustomHeaders() {
100+
return errors.BadInput.New("custom headers must include both key and value")
101+
}
102+
hasCustomHeaders := connection.HasUsableCustomHeaders()
100103
if !hasToken && !hasCustomHeaders {
101104
return errors.BadInput.New("either token or at least one custom header is required")
102105
}

backend/plugins/claude_code/api/connection_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,38 @@ func TestValidateConnectionCustomHeadersWithoutToken(t *testing.T) {
8888
assert.NoError(t, err)
8989
}
9090

91+
func TestValidateConnectionRejectsBlankCustomHeaders(t *testing.T) {
92+
connection := &models.ClaudeCodeConnection{
93+
ClaudeCodeConn: models.ClaudeCodeConn{
94+
Organization: testOrganization,
95+
CustomHeaders: []models.CustomHeader{
96+
{Key: "", Value: ""},
97+
},
98+
},
99+
}
100+
connection.Normalize()
101+
102+
err := validateConnection(connection)
103+
assert.Error(t, err)
104+
assert.Contains(t, err.Error(), "either token or at least one custom header is required")
105+
}
106+
107+
func TestValidateConnectionRejectsIncompleteCustomHeaders(t *testing.T) {
108+
connection := &models.ClaudeCodeConnection{
109+
ClaudeCodeConn: models.ClaudeCodeConn{
110+
Organization: testOrganization,
111+
CustomHeaders: []models.CustomHeader{
112+
{Key: "Ocp-Apim-Subscription-Key", Value: ""},
113+
},
114+
},
115+
}
116+
connection.Normalize()
117+
118+
err := validateConnection(connection)
119+
assert.Error(t, err)
120+
assert.Contains(t, err.Error(), "custom headers must include both key and value")
121+
}
122+
91123
func TestValidateConnectionInvalidRateLimit(t *testing.T) {
92124
connection := &models.ClaudeCodeConnection{
93125
ClaudeCodeConn: models.ClaudeCodeConn{

backend/plugins/claude_code/models/connection.go

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,32 @@ type ClaudeCodeConn struct {
5050
CustomHeaders []CustomHeader `mapstructure:"customHeaders" json:"customHeaders" gorm:"type:json;serializer:json"`
5151
}
5252

53+
func (conn *ClaudeCodeConn) HasUsableCustomHeaders() bool {
54+
if conn == nil {
55+
return false
56+
}
57+
for _, header := range conn.CustomHeaders {
58+
if strings.TrimSpace(header.Key) != "" && strings.TrimSpace(header.Value) != "" {
59+
return true
60+
}
61+
}
62+
return false
63+
}
64+
65+
func (conn *ClaudeCodeConn) HasIncompleteCustomHeaders() bool {
66+
if conn == nil {
67+
return false
68+
}
69+
for _, header := range conn.CustomHeaders {
70+
hasKey := strings.TrimSpace(header.Key) != ""
71+
hasValue := strings.TrimSpace(header.Value) != ""
72+
if hasKey != hasValue {
73+
return true
74+
}
75+
}
76+
return false
77+
}
78+
5379
// SetupAuthentication implements plugin.ApiAuthenticator so helper.NewApiClientFromConnection
5480
// can attach the required headers for Anthropic Admin API requests.
5581
//
@@ -68,7 +94,7 @@ func (conn *ClaudeCodeConn) SetupAuthentication(request *http.Request) errors.Er
6894
}
6995

7096
for _, h := range conn.CustomHeaders {
71-
if strings.TrimSpace(h.Key) != "" {
97+
if strings.TrimSpace(h.Key) != "" && strings.TrimSpace(h.Value) != "" {
7298
request.Header.Set(h.Key, h.Value)
7399
}
74100
}
@@ -117,6 +143,16 @@ func (connection *ClaudeCodeConnection) Normalize() {
117143
if connection.RateLimitPerHour <= 0 {
118144
connection.RateLimitPerHour = DefaultRateLimitPerHour
119145
}
146+
normalizedHeaders := make([]CustomHeader, 0, len(connection.CustomHeaders))
147+
for _, header := range connection.CustomHeaders {
148+
key := strings.TrimSpace(header.Key)
149+
value := strings.TrimSpace(header.Value)
150+
if key == "" && value == "" {
151+
continue
152+
}
153+
normalizedHeaders = append(normalizedHeaders, CustomHeader{Key: key, Value: header.Value})
154+
}
155+
connection.CustomHeaders = normalizedHeaders
120156
}
121157

122158
// MergeFromRequest applies a partial update from an HTTP PATCH body,

backend/plugins/claude_code/models/migrationscripts/20260319_replace_analytics_tables.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ import (
2727
"github.com/apache/incubator-devlake/helpers/migrationhelper"
2828
)
2929

30-
// replaceClaudeCodeAnalyticsTables drops the old deprecated endpoint tables and
31-
// creates the five new analytics endpoint tables.
30+
// replaceClaudeCodeAnalyticsTables creates the five analytics endpoint tables
31+
// and their raw ingestion tables for the updated API surface.
3232
type replaceClaudeCodeAnalyticsTables struct{}
3333

3434
func (*replaceClaudeCodeAnalyticsTables) Up(basicRes context.BasicRes) errors.Error {

backend/plugins/claude_code/service/connection_test_helper.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,22 +49,25 @@ func TestConnection(ctx stdctx.Context, br corectx.BasicRes, connection *models.
4949
connection.Normalize()
5050

5151
hasToken := strings.TrimSpace(connection.Token) != ""
52-
hasCustomHeaders := len(connection.CustomHeaders) > 0
52+
if connection.HasIncompleteCustomHeaders() {
53+
return nil, errors.BadInput.New("custom headers must include both key and value")
54+
}
55+
hasCustomHeaders := connection.HasUsableCustomHeaders()
5356
if !hasToken && !hasCustomHeaders {
5457
return nil, errors.BadInput.New("either token or at least one custom header is required")
5558
}
5659
if strings.TrimSpace(connection.Organization) == "" {
57-
return nil, errors.BadInput.New("organizationId is required")
60+
return nil, errors.BadInput.New("organization is required")
5861
}
5962

6063
apiClient, err := helper.NewApiClientFromConnection(ctx, br, connection)
6164
if err != nil {
6265
return nil, err
6366
}
6467

65-
// Use today's date for the test request.
66-
today := time.Now().UTC().Format("2006-01-02")
67-
endpoint := fmt.Sprintf("v1/organizations/usage_report/claude_code?starting_at=%s&limit=1", today)
68+
// Use a date safely outside the analytics availability lag window for the test request.
69+
testDate := time.Now().UTC().AddDate(0, 0, -7).Format("2006-01-02")
70+
endpoint := fmt.Sprintf("v1/organizations/usage_report/claude_code?starting_at=%s&limit=1", testDate)
6871

6972
res, err := apiClient.Get(endpoint, nil, nil)
7073
if err != nil {

backend/plugins/gh-copilot/models/models_test.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,24 +22,24 @@ import "testing"
2222
func TestGetTablesInfo(t *testing.T) {
2323
tables := GetTablesInfo()
2424
expected := map[string]bool{
25-
(&GhCopilotConnection{}).TableName(): false,
26-
(&GhCopilotScope{}).TableName(): false,
27-
(&GhCopilotScopeConfig{}).TableName(): false,
28-
(&GhCopilotOrgMetrics{}).TableName(): false,
29-
(&GhCopilotLanguageMetrics{}).TableName(): false,
30-
(&GhCopilotEnterpriseDailyMetrics{}).TableName(): false,
31-
(&GhCopilotMetricsByIde{}).TableName(): false,
32-
(&GhCopilotMetricsByFeature{}).TableName(): false,
33-
(&GhCopilotMetricsByLanguageFeature{}).TableName(): false,
34-
(&GhCopilotMetricsByLanguageModel{}).TableName(): false,
35-
(&GhCopilotMetricsByModelFeature{}).TableName(): false,
36-
(&GhCopilotUserDailyMetrics{}).TableName(): false,
37-
(&GhCopilotUserMetricsByIde{}).TableName(): false,
38-
(&GhCopilotUserMetricsByFeature{}).TableName(): false,
25+
(&GhCopilotConnection{}).TableName(): false,
26+
(&GhCopilotScope{}).TableName(): false,
27+
(&GhCopilotScopeConfig{}).TableName(): false,
28+
(&GhCopilotOrgMetrics{}).TableName(): false,
29+
(&GhCopilotLanguageMetrics{}).TableName(): false,
30+
(&GhCopilotEnterpriseDailyMetrics{}).TableName(): false,
31+
(&GhCopilotMetricsByIde{}).TableName(): false,
32+
(&GhCopilotMetricsByFeature{}).TableName(): false,
33+
(&GhCopilotMetricsByLanguageFeature{}).TableName(): false,
34+
(&GhCopilotMetricsByLanguageModel{}).TableName(): false,
35+
(&GhCopilotMetricsByModelFeature{}).TableName(): false,
36+
(&GhCopilotUserDailyMetrics{}).TableName(): false,
37+
(&GhCopilotUserMetricsByIde{}).TableName(): false,
38+
(&GhCopilotUserMetricsByFeature{}).TableName(): false,
3939
(&GhCopilotUserMetricsByLanguageFeature{}).TableName(): false,
40-
(&GhCopilotUserMetricsByLanguageModel{}).TableName(): false,
41-
(&GhCopilotUserMetricsByModelFeature{}).TableName(): false,
42-
(&GhCopilotSeat{}).TableName(): false,
40+
(&GhCopilotUserMetricsByLanguageModel{}).TableName(): false,
41+
(&GhCopilotUserMetricsByModelFeature{}).TableName(): false,
42+
(&GhCopilotSeat{}).TableName(): false,
4343
}
4444

4545
if len(tables) != len(expected) {

backend/plugins/gh-copilot/models/scope_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func TestGhCopilotScope_BeforeSave_BaselinePeriodDays(t *testing.T) {
136136
Id: "test-org",
137137
BaselinePeriodDays: tt.input,
138138
}
139-
err := s.BeforeSave(nil)
139+
err := s.BeforeSave(nil)
140140
if err != nil {
141141
t.Errorf("BeforeSave() error = %v, want nil", err)
142142
}

backend/plugins/github/tasks/cicd_job_convertor_test.go

Lines changed: 56 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -68,63 +68,63 @@ func BenchmarkGenJobID(b *testing.B) {
6868
}
6969

7070
func TestConvertJobs_SkipNoStartedAt(t *testing.T) {
71-
job := &models.GithubJob{
72-
ID: 123,
73-
RunID: 456,
74-
Name: "test-job",
75-
StartedAt: nil,
76-
}
77-
78-
convert := func(inputRow interface{}) ([]interface{}, error) {
79-
line := inputRow.(*models.GithubJob)
80-
if line.StartedAt == nil {
81-
return nil, nil
82-
}
83-
createdAt := *line.StartedAt
84-
domainJob := &devops.CICDTask{
85-
Name: line.Name,
86-
TaskDatesInfo: devops.TaskDatesInfo{
87-
CreatedDate: createdAt,
88-
StartedDate: line.StartedAt,
89-
FinishedDate: line.CompletedAt,
90-
},
91-
}
92-
return []interface{}{domainJob}, nil
93-
}
94-
95-
result, err := convert(job)
96-
assert.Nil(t, err)
97-
assert.Nil(t, result)
71+
job := &models.GithubJob{
72+
ID: 123,
73+
RunID: 456,
74+
Name: "test-job",
75+
StartedAt: nil,
76+
}
77+
78+
convert := func(inputRow interface{}) ([]interface{}, error) {
79+
line := inputRow.(*models.GithubJob)
80+
if line.StartedAt == nil {
81+
return nil, nil
82+
}
83+
createdAt := *line.StartedAt
84+
domainJob := &devops.CICDTask{
85+
Name: line.Name,
86+
TaskDatesInfo: devops.TaskDatesInfo{
87+
CreatedDate: createdAt,
88+
StartedDate: line.StartedAt,
89+
FinishedDate: line.CompletedAt,
90+
},
91+
}
92+
return []interface{}{domainJob}, nil
93+
}
94+
95+
result, err := convert(job)
96+
assert.Nil(t, err)
97+
assert.Nil(t, result)
9898
}
9999

100100
func TestConvertJobs_WithStartedAt(t *testing.T) {
101-
now := time.Now()
102-
job := &models.GithubJob{
103-
ID: 123,
104-
RunID: 456,
105-
Name: "test-job",
106-
StartedAt: &now,
107-
}
108-
109-
convert := func(inputRow interface{}) ([]interface{}, error) {
110-
line := inputRow.(*models.GithubJob)
111-
if line.StartedAt == nil {
112-
return nil, nil
113-
}
114-
createdAt := *line.StartedAt
115-
domainJob := &devops.CICDTask{
116-
Name: line.Name,
117-
TaskDatesInfo: devops.TaskDatesInfo{
118-
CreatedDate: createdAt,
119-
StartedDate: line.StartedAt,
120-
FinishedDate: line.CompletedAt,
121-
},
122-
}
123-
return []interface{}{domainJob}, nil
124-
}
125-
126-
result, err := convert(job)
127-
assert.Nil(t, err)
128-
assert.NotNil(t, result)
129-
assert.Equal(t, "test-job", result[0].(*devops.CICDTask).Name)
101+
now := time.Now()
102+
job := &models.GithubJob{
103+
ID: 123,
104+
RunID: 456,
105+
Name: "test-job",
106+
StartedAt: &now,
107+
}
108+
109+
convert := func(inputRow interface{}) ([]interface{}, error) {
110+
line := inputRow.(*models.GithubJob)
111+
if line.StartedAt == nil {
112+
return nil, nil
113+
}
114+
createdAt := *line.StartedAt
115+
domainJob := &devops.CICDTask{
116+
Name: line.Name,
117+
TaskDatesInfo: devops.TaskDatesInfo{
118+
CreatedDate: createdAt,
119+
StartedDate: line.StartedAt,
120+
FinishedDate: line.CompletedAt,
121+
},
122+
}
123+
return []interface{}{domainJob}, nil
124+
}
125+
126+
result, err := convert(job)
127+
assert.Nil(t, err)
128+
assert.NotNil(t, result)
129+
assert.Equal(t, "test-job", result[0].(*devops.CICDTask).Name)
130130
}

0 commit comments

Comments
 (0)