From 7a9cc9d7526fecb836965e0e2ebeceed7dd1424c Mon Sep 17 00:00:00 2001 From: Vibhor Mahajan Date: Tue, 7 Oct 2025 15:06:41 +0530 Subject: [PATCH 1/2] fix: normalize deployment type comparison in epic and issue collectors --- .../jira/tasks/deployment_type_test.go | 85 +++++++++++++++++++ backend/plugins/jira/tasks/epic_collector.go | 4 +- backend/plugins/jira/tasks/issue_collector.go | 3 +- 3 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 backend/plugins/jira/tasks/deployment_type_test.go diff --git a/backend/plugins/jira/tasks/deployment_type_test.go b/backend/plugins/jira/tasks/deployment_type_test.go new file mode 100644 index 00000000000..14ae1ed1c27 --- /dev/null +++ b/backend/plugins/jira/tasks/deployment_type_test.go @@ -0,0 +1,85 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "strings" + "testing" + + "github.com/apache/incubator-devlake/plugins/jira/models" +) + +func TestDeploymentTypeComparison(t *testing.T) { + tests := []struct { + name string + deploymentType models.DeploymentType + wantServer bool + }{ + { + name: "lowercase server", + deploymentType: models.DeploymentServer, + wantServer: true, + }, + { + name: "uppercase Server", + deploymentType: "Server", + wantServer: true, + }, + { + name: "mixed case SeRvEr", + deploymentType: "SeRvEr", + wantServer: true, + }, + { + name: "lowercase cloud", + deploymentType: models.DeploymentCloud, + wantServer: false, + }, + { + name: "uppercase Cloud", + deploymentType: "Cloud", + wantServer: false, + }, + { + name: "mixed case ClOuD", + deploymentType: "ClOuD", + wantServer: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the case-insensitive comparison logic used in the collectors + // We compare lowercase versions because the API might return "Cloud" or "Server" with capital letters + isServer := strings.ToLower(string(tt.deploymentType)) == strings.ToLower(string(models.DeploymentServer)) + if isServer != tt.wantServer { + t.Errorf("DeploymentType comparison for %q: got isServer=%v, want %v", tt.deploymentType, isServer, tt.wantServer) + } + }) + } +} + +func TestDeploymentTypeConstants(t *testing.T) { + // Verify the constants have the expected values + if models.DeploymentServer != "Server" { + t.Errorf("DeploymentServer constant = %q, want %q", models.DeploymentServer, "Server") + } + if models.DeploymentCloud != "Cloud" { + t.Errorf("DeploymentCloud constant = %q, want %q", models.DeploymentCloud, "Cloud") + } +} diff --git a/backend/plugins/jira/tasks/epic_collector.go b/backend/plugins/jira/tasks/epic_collector.go index 2e777f92c98..8c7b333d7ac 100644 --- a/backend/plugins/jira/tasks/epic_collector.go +++ b/backend/plugins/jira/tasks/epic_collector.go @@ -51,7 +51,7 @@ func CollectEpics(taskCtx plugin.SubTaskContext) errors.Error { data := taskCtx.GetData().(*JiraTaskData) logger := taskCtx.GetLogger() batchSize := 100 - if data.JiraServerInfo.DeploymentType == models.DeploymentServer && len(data.JiraServerInfo.VersionNumbers) == 3 && data.JiraServerInfo.VersionNumbers[0] <= 8 { + if strings.ToLower(string(data.JiraServerInfo.DeploymentType)) == string(models.DeploymentServer) && len(data.JiraServerInfo.VersionNumbers) == 3 && data.JiraServerInfo.VersionNumbers[0] <= 8 { batchSize = 1 } epicIterator, err := GetEpicKeysIterator(db, data, batchSize) @@ -83,7 +83,7 @@ func CollectEpics(taskCtx plugin.SubTaskContext) errors.Error { } // Choose API endpoint based on JIRA deployment type - if data.JiraServerInfo.DeploymentType == models.DeploymentServer { + if strings.ToLower(string(data.JiraServerInfo.DeploymentType)) == string(models.DeploymentServer) { logger.Info("Using api/2/search for JIRA Server") err = setupApiV2Collector(apiCollector, data, epicIterator, jql) } else { diff --git a/backend/plugins/jira/tasks/issue_collector.go b/backend/plugins/jira/tasks/issue_collector.go index 84ac2d7273f..471971f9ead 100644 --- a/backend/plugins/jira/tasks/issue_collector.go +++ b/backend/plugins/jira/tasks/issue_collector.go @@ -23,6 +23,7 @@ import ( "io" "net/http" "net/url" + "strings" "time" "github.com/apache/incubator-devlake/core/dal" @@ -172,7 +173,7 @@ func getTimeZone(taskCtx plugin.SubTaskContext) (*time.Location, errors.Error) { var resp *http.Response var path string var query url.Values - if data.JiraServerInfo.DeploymentType == models.DeploymentServer { + if strings.ToLower(string(data.JiraServerInfo.DeploymentType)) == string(models.DeploymentServer) { path = "api/2/user" query = url.Values{"username": []string{conn.Username}} } else { From 9dd0315449709ff3bf37202df3cbf5aa3af1144a Mon Sep 17 00:00:00 2001 From: Vibhor Mahajan Date: Mon, 13 Oct 2025 09:46:21 +0530 Subject: [PATCH 2/2] fix: use strings.EqualFold for case-insensitive deployment type comparison - Replace strings.ToLower(deploymentType) == constant with strings.EqualFold - Fixes issue where case-insensitive comparison was only converting one side - Update tests to reflect the new comparison logic using strings.EqualFold - Affects epic_collector.go and issue_collector.go --- .../jira/tasks/deployment_type_test.go | 85 ------- backend/plugins/jira/tasks/epic_collector.go | 4 +- .../plugins/jira/tasks/epic_collector_test.go | 217 ++++++++++++++++++ backend/plugins/jira/tasks/issue_collector.go | 2 +- 4 files changed, 220 insertions(+), 88 deletions(-) delete mode 100644 backend/plugins/jira/tasks/deployment_type_test.go create mode 100644 backend/plugins/jira/tasks/epic_collector_test.go diff --git a/backend/plugins/jira/tasks/deployment_type_test.go b/backend/plugins/jira/tasks/deployment_type_test.go deleted file mode 100644 index 14ae1ed1c27..00000000000 --- a/backend/plugins/jira/tasks/deployment_type_test.go +++ /dev/null @@ -1,85 +0,0 @@ -/* -Licensed to the Apache Software Foundation (ASF) under one or more -contributor license agreements. See the NOTICE file distributed with -this work for additional information regarding copyright ownership. -The ASF licenses this file to You under the Apache License, Version 2.0 -(the "License"); you may not use this file except in compliance with -the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package tasks - -import ( - "strings" - "testing" - - "github.com/apache/incubator-devlake/plugins/jira/models" -) - -func TestDeploymentTypeComparison(t *testing.T) { - tests := []struct { - name string - deploymentType models.DeploymentType - wantServer bool - }{ - { - name: "lowercase server", - deploymentType: models.DeploymentServer, - wantServer: true, - }, - { - name: "uppercase Server", - deploymentType: "Server", - wantServer: true, - }, - { - name: "mixed case SeRvEr", - deploymentType: "SeRvEr", - wantServer: true, - }, - { - name: "lowercase cloud", - deploymentType: models.DeploymentCloud, - wantServer: false, - }, - { - name: "uppercase Cloud", - deploymentType: "Cloud", - wantServer: false, - }, - { - name: "mixed case ClOuD", - deploymentType: "ClOuD", - wantServer: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Test the case-insensitive comparison logic used in the collectors - // We compare lowercase versions because the API might return "Cloud" or "Server" with capital letters - isServer := strings.ToLower(string(tt.deploymentType)) == strings.ToLower(string(models.DeploymentServer)) - if isServer != tt.wantServer { - t.Errorf("DeploymentType comparison for %q: got isServer=%v, want %v", tt.deploymentType, isServer, tt.wantServer) - } - }) - } -} - -func TestDeploymentTypeConstants(t *testing.T) { - // Verify the constants have the expected values - if models.DeploymentServer != "Server" { - t.Errorf("DeploymentServer constant = %q, want %q", models.DeploymentServer, "Server") - } - if models.DeploymentCloud != "Cloud" { - t.Errorf("DeploymentCloud constant = %q, want %q", models.DeploymentCloud, "Cloud") - } -} diff --git a/backend/plugins/jira/tasks/epic_collector.go b/backend/plugins/jira/tasks/epic_collector.go index 8c7b333d7ac..d7fc45f3b73 100644 --- a/backend/plugins/jira/tasks/epic_collector.go +++ b/backend/plugins/jira/tasks/epic_collector.go @@ -51,7 +51,7 @@ func CollectEpics(taskCtx plugin.SubTaskContext) errors.Error { data := taskCtx.GetData().(*JiraTaskData) logger := taskCtx.GetLogger() batchSize := 100 - if strings.ToLower(string(data.JiraServerInfo.DeploymentType)) == string(models.DeploymentServer) && len(data.JiraServerInfo.VersionNumbers) == 3 && data.JiraServerInfo.VersionNumbers[0] <= 8 { + if strings.EqualFold(string(data.JiraServerInfo.DeploymentType), string(models.DeploymentServer)) && len(data.JiraServerInfo.VersionNumbers) == 3 && data.JiraServerInfo.VersionNumbers[0] <= 8 { batchSize = 1 } epicIterator, err := GetEpicKeysIterator(db, data, batchSize) @@ -83,7 +83,7 @@ func CollectEpics(taskCtx plugin.SubTaskContext) errors.Error { } // Choose API endpoint based on JIRA deployment type - if strings.ToLower(string(data.JiraServerInfo.DeploymentType)) == string(models.DeploymentServer) { + if strings.EqualFold(string(data.JiraServerInfo.DeploymentType), string(models.DeploymentServer)) { logger.Info("Using api/2/search for JIRA Server") err = setupApiV2Collector(apiCollector, data, epicIterator, jql) } else { diff --git a/backend/plugins/jira/tasks/epic_collector_test.go b/backend/plugins/jira/tasks/epic_collector_test.go new file mode 100644 index 00000000000..8587f0dc28f --- /dev/null +++ b/backend/plugins/jira/tasks/epic_collector_test.go @@ -0,0 +1,217 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "strings" + "testing" + + "github.com/apache/incubator-devlake/plugins/jira/models" +) + +func TestEpicCollectorBatchSizeLogic(t *testing.T) { + tests := []struct { + name string + deploymentType models.DeploymentType + versionNumbers []int + expectedBatch int + }{ + { + name: "JIRA Server v8 should use batch size 1", + deploymentType: models.DeploymentServer, + versionNumbers: []int{8, 0, 0}, + expectedBatch: 1, + }, + { + name: "JIRA Server v7 should use batch size 1", + deploymentType: models.DeploymentServer, + versionNumbers: []int{7, 5, 0}, + expectedBatch: 1, + }, + { + name: "JIRA Server v9 should use default batch size", + deploymentType: models.DeploymentServer, + versionNumbers: []int{9, 0, 0}, + expectedBatch: 100, + }, + { + name: "JIRA Cloud should use default batch size", + deploymentType: models.DeploymentCloud, + versionNumbers: []int{8, 0, 0}, // Version shouldn't matter for cloud + expectedBatch: 100, + }, + { + name: "Case insensitive server comparison", + deploymentType: "server", // lowercase + versionNumbers: []int{8, 0, 0}, + expectedBatch: 1, + }, + { + name: "Case insensitive cloud comparison", + deploymentType: "CLOUD", // uppercase + versionNumbers: []int{8, 0, 0}, + expectedBatch: 100, + }, + { + name: "Mixed case server comparison", + deploymentType: "SeRvEr", // mixed case + versionNumbers: []int{7, 0, 0}, + expectedBatch: 1, + }, + { + name: "JIRA Server with incomplete version numbers", + deploymentType: models.DeploymentServer, + versionNumbers: []int{8, 0}, // Only 2 elements instead of 3 + expectedBatch: 100, + }, + { + name: "JIRA Server with empty version numbers", + deploymentType: models.DeploymentServer, + versionNumbers: []int{}, + expectedBatch: 100, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the batch size logic from CollectEpics function + batchSize := 100 + + // Replicate the exact logic from CollectEpics + if strings.EqualFold(string(tt.deploymentType), string(models.DeploymentServer)) && + len(tt.versionNumbers) == 3 && + tt.versionNumbers[0] <= 8 { + batchSize = 1 + } + + if batchSize != tt.expectedBatch { + t.Errorf("Batch size for %s with version %v: got %d, want %d", + tt.deploymentType, tt.versionNumbers, batchSize, tt.expectedBatch) + } + }) + } +} + +func TestEpicCollectorDeploymentTypeLogic(t *testing.T) { + tests := []struct { + name string + deploymentType models.DeploymentType + expectServer bool + }{ + { + name: "JIRA Server constant should be detected as server", + deploymentType: models.DeploymentServer, + expectServer: true, + }, + { + name: "JIRA Cloud constant should not be detected as server", + deploymentType: models.DeploymentCloud, + expectServer: false, + }, + { + name: "Lowercase server should be detected as server", + deploymentType: "server", + expectServer: true, + }, + { + name: "Uppercase SERVER should be detected as server", + deploymentType: "SERVER", + expectServer: true, + }, + { + name: "Mixed case SeRvEr should be detected as server", + deploymentType: "SeRvEr", + expectServer: true, + }, + { + name: "Lowercase cloud should not be detected as server", + deploymentType: "cloud", + expectServer: false, + }, + { + name: "Uppercase CLOUD should not be detected as server", + deploymentType: "CLOUD", + expectServer: false, + }, + { + name: "Mixed case ClOuD should not be detected as server", + deploymentType: "ClOuD", + expectServer: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the deployment type logic from CollectEpics function + // This replicates the exact comparison used in the collector + isServer := strings.EqualFold(string(tt.deploymentType), string(models.DeploymentServer)) + + if isServer != tt.expectServer { + t.Errorf("Deployment type detection for %s: got isServer=%v, want %v", + tt.deploymentType, isServer, tt.expectServer) + } + }) + } +} + +func TestEpicCollectorApiEndpointSelection(t *testing.T) { + tests := []struct { + name string + deploymentType models.DeploymentType + expectedEndpoint string + }{ + { + name: "JIRA Server should use api/2/search", + deploymentType: models.DeploymentServer, + expectedEndpoint: "api/2/search", + }, + { + name: "JIRA Cloud should use api/3/search/jql", + deploymentType: models.DeploymentCloud, + expectedEndpoint: "api/3/search/jql", + }, + { + name: "Lowercase server should use api/2/search", + deploymentType: "server", + expectedEndpoint: "api/2/search", + }, + { + name: "Uppercase CLOUD should use api/3/search/jql", + deploymentType: "CLOUD", + expectedEndpoint: "api/3/search/jql", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the API endpoint selection logic from CollectEpics function + var selectedEndpoint string + + if strings.EqualFold(string(tt.deploymentType), string(models.DeploymentServer)) { + selectedEndpoint = "api/2/search" + } else { + selectedEndpoint = "api/3/search/jql" + } + + if selectedEndpoint != tt.expectedEndpoint { + t.Errorf("API endpoint selection for %s: got %s, want %s", + tt.deploymentType, selectedEndpoint, tt.expectedEndpoint) + } + }) + } +} \ No newline at end of file diff --git a/backend/plugins/jira/tasks/issue_collector.go b/backend/plugins/jira/tasks/issue_collector.go index 471971f9ead..9a361cbbcca 100644 --- a/backend/plugins/jira/tasks/issue_collector.go +++ b/backend/plugins/jira/tasks/issue_collector.go @@ -173,7 +173,7 @@ func getTimeZone(taskCtx plugin.SubTaskContext) (*time.Location, errors.Error) { var resp *http.Response var path string var query url.Values - if strings.ToLower(string(data.JiraServerInfo.DeploymentType)) == string(models.DeploymentServer) { + if strings.EqualFold(string(data.JiraServerInfo.DeploymentType), string(models.DeploymentServer)) { path = "api/2/user" query = url.Values{"username": []string{conn.Username}} } else {