Regex patterns are not efficient enough (#242)

Author: yahavi

commands/utils/mavenutils.go

@@ -8,51 +8,80 @@ import (
 	"strings"
 )
 
+const (
+	modulePattern       = "<module>(\\S|\\s)*?<\\/module>"
+	dependenciesPattern = "(<dependency>(\\S|\\s)*?<\\/dependency>)|(<plugin>(\\S|\\s)*?<\\/plugin>)"
+)
+
+var (
+	moduleRegexp       = regexp.MustCompile(modulePattern)
+	dependenciesRegexp = regexp.MustCompile(dependenciesPattern)
+)
+
 func GetVersionProperties(projectPath string, depToPropertyMap map[string][]string) error {
 	contentBytes, err := os.ReadFile(filepath.Join(projectPath, "pom.xml")) // #nosec G304
 	if err != nil {
 		return err
 	}
-	dependenciesPattern := "(<dependency>(.|\\s)*?<\\/dependency>)|(<plugin>(.|\\s)*?<\\/plugin>)"
-	dependenciesRegexp, err := regexp.Compile(dependenciesPattern)
+	mavenDependencies, err := getDependenciesFromPomXml(contentBytes)
 	if err != nil {
 		return err
 	}
-	dependencyStrings := dependenciesRegexp.FindAll(contentBytes, -1)
-	for _, depStr := range dependencyStrings {
-		dep := &mavenDependency{}
-		err = xml.Unmarshal(depStr, dep)
-		if err != nil {
-			return err
-		}
-		depName := dep.GroupId + ":" + dep.ArtifactId
+	for _, mavenDependency := range mavenDependencies {
+		depName := mavenDependency.GroupId + ":" + mavenDependency.ArtifactId
 		if _, exist := depToPropertyMap[depName]; !exist {
 			depToPropertyMap[depName] = []string{}
 		}
-		if strings.HasPrefix(dep.Version, "${") {
-			depToPropertyMap[depName] = append(depToPropertyMap[dep.GroupId+":"+dep.ArtifactId], strings.TrimPrefix(strings.TrimSuffix(dep.Version, "}"), "${"))
+		if strings.HasPrefix(mavenDependency.Version, "${") {
+			depToPropertyMap[depName] = append(depToPropertyMap[mavenDependency.GroupId+":"+mavenDependency.ArtifactId], strings.TrimPrefix(strings.TrimSuffix(mavenDependency.Version, "}"), "${"))
 		}
 	}
 
-	modulePattern := "<module>(.|\\s)*?<\\/module>"
-	moduleRegexp, err := regexp.Compile(modulePattern)
-	if err != nil {
-		return err
-	}
-	moduleStrings := moduleRegexp.FindAllString(string(contentBytes), -1)
-	for _, moduleStr := range moduleStrings {
-		modulePath := strings.TrimPrefix(strings.TrimSuffix(moduleStr, "</module>"), "<module>")
-		modulePath = strings.TrimSpace(modulePath)
-		err = GetVersionProperties(filepath.Join(projectPath, modulePath), depToPropertyMap)
-		if err != nil {
+	for _, moduleStr := range getMavenModuleFromPomXml(contentBytes) {
+		if err = GetVersionProperties(filepath.Join(projectPath, moduleStr), depToPropertyMap); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
+// Extract all dependencies from the input pom.xml
+// pomXmlContent - The pom.xml content
+func getDependenciesFromPomXml(pomXmlContent []byte) ([]mavenDependency, error) {
+	var results []mavenDependency
+	dependencyStrings := dependenciesRegexp.FindAll(pomXmlContent, -1)
+	for _, depStr := range dependencyStrings {
+		dep := &mavenDependency{}
+		err := xml.Unmarshal(depStr, dep)
+		if err != nil {
+			return []mavenDependency{}, err
+		}
+		dep.trimSpaces()
+		results = append(results, *dep)
+	}
+	return results, nil
+}
+
+// Extract all modules from pom.xml
+// pomXmlContent - The pom.xml content
+func getMavenModuleFromPomXml(pomXmlContent []byte) []string {
+	var results []string
+	moduleStrings := moduleRegexp.FindAllString(string(pomXmlContent), -1)
+	for _, moduleStr := range moduleStrings {
+		modulePath := strings.TrimPrefix(strings.TrimSuffix(moduleStr, "</module>"), "<module>")
+		results = append(results, strings.TrimSpace(modulePath))
+	}
+	return results
+}
+
 type mavenDependency struct {
 	GroupId    string `xml:"groupId"`
 	ArtifactId string `xml:"artifactId"`
 	Version    string `xml:"version"`
 }
+
+func (md *mavenDependency) trimSpaces() {
+	md.GroupId = strings.TrimSpace(md.GroupId)
+	md.ArtifactId = strings.TrimSpace(md.ArtifactId)
+	md.Version = strings.TrimSpace(md.Version)
+}

commands/utils/mavenutils_test.go

@@ -0,0 +1,92 @@
+package utils
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetDependenciesFromPomXmlSingleDependency(t *testing.T) {
+	testCases := []string{`<dependency> 
+	<groupId>org.apache.commons</groupId>
+	<artifactId>commons-email</artifactId>
+	<version>1.1</version>
+	<scope>compile</scope>
+</dependency>`,
+		`<dependency> 
+	<groupId> 	org.apache.commons</groupId>
+	<artifactId>commons-email	 </artifactId>
+	<version>  1.1  </version>
+	<scope> compile		</scope>
+</dependency>`,
+	}
+
+	for _, testCase := range testCases {
+		result, err := getDependenciesFromPomXml([]byte(testCase))
+		assert.NoError(t, err)
+
+		assert.Len(t, result, 1)
+		assert.Equal(t, "org.apache.commons", result[0].GroupId)
+		assert.Equal(t, "commons-email", result[0].ArtifactId)
+		assert.Equal(t, "1.1", result[0].Version)
+	}
+}
+
+func TestGetDependenciesFromPomXmlMultiDependency(t *testing.T) {
+	testCases := []string{`<dependencies>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-email</artifactId>
+            <version>1.1</version>
+            <scope>compile</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+            <artifactId>plexus-utils</artifactId>
+            <version>1.5.1</version>
+        </dependency>
+	</dependencies>`,
+		`<dependencies><dependency><groupId>org.apache.commons</groupId><artifactId>commons-email</artifactId>
+            <version>1.1</version><scope>compile</scope>
+        </dependency>
+
+		some-xml-tags
+
+        <dependency>
+            <groupId>org.codehaus.plexus</groupId>
+        <artifactId>plexus-utils</artifactId>
+            	<version>1.5.1</version>
+        </dependency></dependencies>`,
+	}
+
+	for _, testCase := range testCases {
+		result, err := getDependenciesFromPomXml([]byte(testCase))
+		assert.NoError(t, err)
+
+		assert.Len(t, result, 2)
+		assert.Equal(t, "org.apache.commons", result[0].GroupId)
+		assert.Equal(t, "commons-email", result[0].ArtifactId)
+		assert.Equal(t, "1.1", result[0].Version)
+
+		assert.Equal(t, "org.codehaus.plexus", result[1].GroupId)
+		assert.Equal(t, "plexus-utils", result[1].ArtifactId)
+		assert.Equal(t, "1.5.1", result[1].Version)
+	}
+}
+
+func TestGetMavenModuleFromPomXml(t *testing.T) {
+	testCases := []string{`<module>multi-1</module>`, ` <module>  	multi-1 </module>	`}
+	for _, testCase := range testCases {
+		modules := getMavenModuleFromPomXml([]byte(testCase))
+		assert.ElementsMatch(t, modules, []string{"multi-1"})
+	}
+}
+
+func TestGetMavenModuleFromPomXmlMultiModules(t *testing.T) {
+	testCases := []string{`<module>multi-1</module><module>multi-2</module>`,
+		`<module>multi-1</module>	some-xml-tags  <module>multi-2</module>`}
+	for _, testCase := range testCases {
+		modules := getMavenModuleFromPomXml([]byte(testCase))
+		assert.ElementsMatch(t, modules, []string{"multi-1", "multi-2"})
+	}
+}