diff --git a/src/java/containers/groovy_utils.go b/src/java/containers/groovy_utils.go index 1d9f43783..9403204f1 100644 --- a/src/java/containers/groovy_utils.go +++ b/src/java/containers/groovy_utils.go @@ -77,34 +77,6 @@ func (g *GroovyUtils) IsBeans(filePath string) bool { return beansPattern.Match(content) } -// HasMainMethod checks if a Groovy file contains a static void main() method -func HasMainMethod(filePath string) (bool, error) { - content, err := os.ReadFile(filePath) - if err != nil { - return false, err - } - return mainMethodPattern.Match(content), nil -} - -// IsPOGO checks if a Groovy file is a Plain Old Groovy Object (contains a class definition) -// POGOs are NOT standalone runnable scripts - they need to be instantiated -func IsPOGO(filePath string) (bool, error) { - content, err := os.ReadFile(filePath) - if err != nil { - return false, err - } - return pogoPattern.Match(content), nil -} - -// HasShebang checks if a Groovy file has a shebang line (#!/...) -func HasShebang(filePath string) (bool, error) { - content, err := os.ReadFile(filePath) - if err != nil { - return false, err - } - return shebangPattern.Match(content), nil -} - // isValidGroovyFile checks if a file is a valid, readable Groovy script // Filters out binary files, empty files, and files with invalid content func isValidGroovyFile(filePath string) bool { @@ -149,60 +121,23 @@ func isPartOfUTF8Sequence(content []byte, i int) bool { return false } -// FindMainGroovyScript determines which Groovy script should be executed -// Following Ruby buildpack logic: -// 1. Files with static void main() method -// 2. Non-POGO files (simple scripts without class definitions) -// 3. Files with shebang -// Returns the single candidate if exactly one matches, empty string otherwise +// FindMainGroovyScript determines which Groovy script should be executed. +// Following Ruby buildpack logic, a file is a candidate if it has a static +// void main() method, is not a POGO, or has a shebang. +// Returns the single candidate if exactly one matches, empty string otherwise. func FindMainGroovyScript(scripts []string) (string, error) { + g := &GroovyUtils{} candidates := make(map[string]bool) - // Filter out invalid files first - validScripts := make([]string, 0, len(scripts)) for _, script := range scripts { - if isValidGroovyFile(script) { - validScripts = append(validScripts, script) - } - } - - // Check for main method - for _, script := range validScripts { - hasMain, err := HasMainMethod(script) - if err != nil { - // Skip files that can't be read (like binary files) - continue - } - if hasMain { - candidates[script] = true - } - } - - // Check for non-POGOs (simple scripts) - for _, script := range validScripts { - isPOGO, err := IsPOGO(script) - if err != nil { - // Skip files that can't be read - continue - } - if !isPOGO { - candidates[script] = true - } - } - - // Check for shebang - for _, script := range validScripts { - hasShebang, err := HasShebang(script) - if err != nil { - // Skip files that can't be read + if !isValidGroovyFile(script) { continue } - if hasShebang { + if g.HasMainMethod(script) || !g.IsPOGO(script) || g.HasShebang(script) { candidates[script] = true } } - // Return the candidate if exactly one matches if len(candidates) == 1 { for script := range candidates { return script, nil diff --git a/src/java/containers/groovy_utils_test.go b/src/java/containers/groovy_utils_test.go index f2426bb0d..bfbd5464a 100644 --- a/src/java/containers/groovy_utils_test.go +++ b/src/java/containers/groovy_utils_test.go @@ -10,98 +10,108 @@ import ( "github.com/cloudfoundry/java-buildpack/src/java/containers" ) -var _ = Describe("HasMainMethod", func() { - DescribeTable("detecting main method in Groovy files", - func(content string, expected bool) { - tmpFile, err := os.CreateTemp("", "test-*.groovy") - Expect(err).NotTo(HaveOccurred()) - defer os.Remove(tmpFile.Name()) +// groovyFile writes content to a temp file and returns its path. +// The file is removed when the current spec ends. +func groovyFile(content string) string { + tmpFile, err := os.CreateTemp("", "test-*.groovy") + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(os.Remove, tmpFile.Name()) + _, err = tmpFile.WriteString(content) + Expect(err).NotTo(HaveOccurred()) + Expect(tmpFile.Close()).To(Succeed()) + return tmpFile.Name() +} + +var _ = Describe("GroovyUtils", func() { + var g *containers.GroovyUtils - _, err = tmpFile.WriteString(content) - Expect(err).NotTo(HaveOccurred()) - tmpFile.Close() + BeforeEach(func() { + g = &containers.GroovyUtils{} + }) - result, err := containers.HasMainMethod(tmpFile.Name()) - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(Equal(expected)) - }, - Entry("has static void main", `class MyApp { + Describe("HasMainMethod", func() { + DescribeTable("detecting main method in Groovy files", + func(content string, expected bool) { + Expect(g.HasMainMethod(groovyFile(content))).To(Equal(expected)) + }, + Entry("has static void main", `class MyApp { static void main(String[] args) { println "Hello" } }`, true), - Entry("has static void main with whitespace variations", `class MyApp { + Entry("has static void main with extra whitespace", `class MyApp { static void main ( String[] args ) { println "Hello" } }`, true), - Entry("no main method", `class Alpha { + Entry("no main method", `class Alpha { }`, false), - Entry("simple script no main", `println 'Hello World'`, false), - Entry("instance method not static main", `class Test { + Entry("simple script no main", `println 'Hello World'`, false), + Entry("instance method not static main", `class Test { void main() { println "Not static" } }`, false), - ) -}) - -var _ = Describe("IsPOGO", func() { - DescribeTable("detecting Plain Old Groovy Objects", - func(content string, expected bool) { - tmpFile, err := os.CreateTemp("", "test-*.groovy") - Expect(err).NotTo(HaveOccurred()) - defer os.Remove(tmpFile.Name()) + ) - _, err = tmpFile.WriteString(content) - Expect(err).NotTo(HaveOccurred()) - tmpFile.Close() + It("returns false for an unreadable file", func() { + Expect(g.HasMainMethod("/nonexistent/file.groovy")).To(BeFalse()) + }) + }) - result, err := containers.IsPOGO(tmpFile.Name()) - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(Equal(expected)) - }, - Entry("simple class definition", `class Alpha { + Describe("IsPOGO", func() { + DescribeTable("detecting Plain Old Groovy Objects", + func(content string, expected bool) { + Expect(g.IsPOGO(groovyFile(content))).To(Equal(expected)) + }, + Entry("simple class definition", `class Alpha { }`, true), - Entry("class with inheritance", `class MyApp extends BaseApp { + Entry("class with inheritance", `class MyApp extends BaseApp { void run() {} }`, true), - Entry("simple script no class", `println 'Hello World'`, false), - Entry("script with variables no class", `def name = "World" + Entry("simple script no class", `println 'Hello World'`, false), + Entry("script with variables no class", `def name = "World" println "Hello $name"`, false), - Entry("class keyword in comment", `// This is not a class + Entry("class keyword in comment", `// This is not a class println 'Hello'`, false), - Entry("class keyword in string", `println "This mentions class but isn't one"`, false), - ) -}) - -var _ = Describe("HasShebang", func() { - DescribeTable("detecting shebang in Groovy files", - func(content string, expected bool) { - tmpFile, err := os.CreateTemp("", "test-*.groovy") - Expect(err).NotTo(HaveOccurred()) - defer os.Remove(tmpFile.Name()) + Entry("class keyword in string", `println "This mentions class but isn't one"`, false), + ) - _, err = tmpFile.WriteString(content) - Expect(err).NotTo(HaveOccurred()) - tmpFile.Close() + It("returns false for an unreadable file", func() { + Expect(g.IsPOGO("/nonexistent/file.groovy")).To(BeFalse()) + }) + }) - result, err := containers.HasShebang(tmpFile.Name()) - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(Equal(expected)) - }, - Entry("has shebang", `#!/usr/bin/env groovy -println 'Hello World'`, true), - Entry("has groovy shebang", `#!/usr/bin/groovy -println 'Hello'`, true), - Entry("no shebang", `class Alpha { + Describe("HasShebang", func() { + DescribeTable("detecting shebang in Groovy files", + func(content string, expected bool) { + Expect(g.HasShebang(groovyFile(content))).To(Equal(expected)) + }, + Entry("has shebang", "#!/usr/bin/env groovy\nprintln 'Hello World'", true), + Entry("has groovy shebang", "#!/usr/bin/groovy\nprintln 'Hello'", true), + Entry("no shebang", `class Alpha { }`, false), - Entry("shebang not at start", ` -#!/usr/bin/env groovy + Entry("shebang not at start", "\n#!/usr/bin/env groovy\nprintln 'Hello'", false), + Entry("comment mentioning shebang", `// Use #!/usr/bin/env groovy at the top println 'Hello'`, false), - Entry("comment mentioning shebang", `// Use #!/usr/bin/env groovy at the top -println 'Hello'`, false), - ) + ) + + It("returns false for an unreadable file", func() { + Expect(g.HasShebang("/nonexistent/file.groovy")).To(BeFalse()) + }) + }) + + Describe("IsBeans", func() { + DescribeTable("detecting beans-style configuration", + func(content string, expected bool) { + Expect(g.IsBeans(groovyFile(content))).To(Equal(expected)) + }, + Entry("has beans block", `beans { + bean(MyBean) +}`, true), + Entry("no beans block", `class Alpha {}`, false), + ) + }) }) var _ = Describe("FindMainGroovyScript", func() { @@ -117,32 +127,24 @@ var _ = Describe("FindMainGroovyScript", func() { os.RemoveAll(tmpDir) }) + writeFile := func(name, content string) string { + path := filepath.Join(tmpDir, name) + Expect(os.WriteFile(path, []byte(content), 0644)).To(Succeed()) + return path + } + Context("with various Groovy script types", func() { var pogoFile, nonPogoFile, mainMethodFile, shebangFile string BeforeEach(func() { - var err error - - pogoFile = filepath.Join(tmpDir, "Alpha.groovy") - err = os.WriteFile(pogoFile, []byte("class Alpha {}"), 0644) - Expect(err).NotTo(HaveOccurred()) - - nonPogoFile = filepath.Join(tmpDir, "Application.groovy") - err = os.WriteFile(nonPogoFile, []byte("println 'Hello World'"), 0644) - Expect(err).NotTo(HaveOccurred()) - - mainMethodFile = filepath.Join(tmpDir, "Main.groovy") - mainContent := `class Main { + pogoFile = writeFile("Alpha.groovy", "class Alpha {}") + nonPogoFile = writeFile("Application.groovy", "println 'Hello World'") + mainMethodFile = writeFile("Main.groovy", `class Main { static void main(String[] args) { println "Main" } -}` - err = os.WriteFile(mainMethodFile, []byte(mainContent), 0644) - Expect(err).NotTo(HaveOccurred()) - - shebangFile = filepath.Join(tmpDir, "Script.groovy") - err = os.WriteFile(shebangFile, []byte("#!/usr/bin/env groovy\nprintln 'Script'"), 0644) - Expect(err).NotTo(HaveOccurred()) +}`) + shebangFile = writeFile("Script.groovy", "#!/usr/bin/env groovy\nprintln 'Script'") }) DescribeTable("finding the main Groovy script", @@ -176,17 +178,11 @@ var _ = Describe("FindMainGroovyScript", func() { }) Context("with invalid files", func() { - It("should skip invalid files and select valid ones", func() { - invalidFile := filepath.Join(tmpDir, "invalid.groovy") - err := os.WriteFile(invalidFile, []byte{0xff, 0xfe}, 0644) - Expect(err).NotTo(HaveOccurred()) - - validFile := filepath.Join(tmpDir, "valid.groovy") - err = os.WriteFile(validFile, []byte("println 'Hello'"), 0644) - Expect(err).NotTo(HaveOccurred()) + It("skips invalid files and selects the valid one", func() { + invalidFile := writeFile("invalid.groovy", string([]byte{0xff, 0xfe})) + validFile := writeFile("valid.groovy", "println 'Hello'") - scripts := []string{invalidFile, validFile} - result, err := containers.FindMainGroovyScript(scripts) + result, err := containers.FindMainGroovyScript([]string{invalidFile, validFile}) Expect(err).NotTo(HaveOccurred()) Expect(result).To(Equal(validFile)) }) diff --git a/src/java/containers/play.go b/src/java/containers/play.go index 0ef20af50..b88183ab4 100644 --- a/src/java/containers/play.go +++ b/src/java/containers/play.go @@ -31,41 +31,48 @@ func (p *PlayContainer) Detect() (string, error) { p.context.Log.Debug("Play: Checking buildDir: %s", buildDir) - // First, validate that we don't have ambiguous configuration (hybrid apps) - if err := p.Validate(); err != nil { - p.context.Log.Debug("Play: Validation failed: %v", err) - return "", err - } - - // Try to detect Play Framework type in order of specificity - // Order matters to avoid ambiguous detection - // Check staged apps (more specific - lib/staged only) before dist apps (less specific - has start scripts) - - // 1. Try Pre22Staged (Play 2.0-2.1 staged app - only staged/ with JARs) - p.context.Log.Debug("Play: Trying Pre22Staged detection") - if p.detectPre22Staged(buildDir) { - p.context.Log.Info("Play: Detected Pre22Staged - version %s", p.playVersion) - return "Play", nil - } - - // 2. Try Post22Staged (Play 2.2+ staged app - only lib/ with JARs) - p.context.Log.Debug("Play: Trying Post22Staged detection") - if p.detectPost22Staged(buildDir) { - p.context.Log.Info("Play: Detected Post22Staged - version %s", p.playVersion) - return "Play", nil + // Run each detector once, collecting matches. Using a temporary container per + // detector avoids mutating p until we know exactly one type matched. + type candidate struct { + name string + c *PlayContainer + } + var matches []candidate + + detectors := []struct { + name string + fn func(*PlayContainer, string) bool + }{ + {"Pre22Staged", (*PlayContainer).detectPre22Staged}, + {"Post22Staged", (*PlayContainer).detectPost22Staged}, + {"Post22Dist", (*PlayContainer).detectPost22Dist}, + {"Pre22Dist", (*PlayContainer).detectPre22Dist}, + } + + for _, d := range detectors { + p.context.Log.Debug("Play: Trying %s detection", d.name) + tmp := &PlayContainer{context: p.context} + if d.fn(tmp, buildDir) { + p.context.Log.Debug("Play: %s matched (version %s)", d.name, tmp.playVersion) + matches = append(matches, candidate{d.name, tmp}) + } } - // 3. Try Post22Dist (Play 2.2+ distributed app in application-root/bin) - p.context.Log.Debug("Play: Trying Post22Dist detection") - if p.detectPost22Dist(buildDir) { - p.context.Log.Info("Play: Detected Post22Dist - version %s", p.playVersion) - return "Play", nil + if len(matches) > 1 { + names := make([]string, len(matches)) + for i, m := range matches { + names[i] = m.name + } + return "", fmt.Errorf("Play Framework application version cannot be determined: %v", names) } - // 4. Try Pre22Dist (Play 2.0-2.1 distributed app in application-root/) - p.context.Log.Debug("Play: Trying Pre22Dist detection") - if p.detectPre22Dist(buildDir) { - p.context.Log.Info("Play: Detected Pre22Dist - version %s", p.playVersion) + if len(matches) == 1 { + m := matches[0] + p.playType = m.c.playType + p.playVersion = m.c.playVersion + p.startScript = m.c.startScript + p.libDir = m.c.libDir + p.context.Log.Info("Play: Detected %s - version %s", m.name, p.playVersion) return "Play", nil } @@ -73,6 +80,12 @@ func (p *PlayContainer) Detect() (string, error) { return "", nil } +// Validate checks for ambiguous Play configurations without mutating the receiver. +func (p *PlayContainer) Validate() error { + _, err := p.Detect() + return err +} + // detectPost22Dist detects Play 2.2+ distributed applications // Structure: application-root/bin/