Skip to content

Commit 18700ad

Browse files
jongioCopilot
andauthored
refactor: wave 4 - consolidate duplicates, centralize parsing, extract helpers (#279)
Code smells (#274): - Reduce ServiceInfo duplication across 5 files - Centralize azure.yaml parsing via service.ParseAzureYaml - Introduce DetectedProjects struct for 3-parameter data clump - Extract runReqsFix into cohesive helper methods - Replace parallel switch statements in query_builder.go with config map Architecture (#265): - Extract shared Azure log level conversion to eliminate duplication - Remove global mutable cmdOrchestrator, make injectable Closes #274, #265 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 027e8c7 commit 18700ad

21 files changed

Lines changed: 407 additions & 462 deletions

cli/src/cmd/app/commands/add.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ func listAvailableServices() error {
155155
sort.Strings(names)
156156

157157
if cliout.IsJSON() {
158-
type ServiceInfo struct {
158+
type AvailableServiceInfo struct {
159159
Name string `json:"name"`
160160
DisplayName string `json:"displayName"`
161161
Description string `json:"description"`
@@ -164,10 +164,10 @@ func listAvailableServices() error {
164164
Ports []string `json:"ports"`
165165
ConnStrings map[string]string `json:"connectionStrings"`
166166
}
167-
services := make([]ServiceInfo, 0, len(names))
167+
services := make([]AvailableServiceInfo, 0, len(names))
168168
for _, name := range names {
169169
def := wellknown.Get(name)
170-
services = append(services, ServiceInfo{
170+
services = append(services, AvailableServiceInfo{
171171
Name: def.Name,
172172
DisplayName: def.DisplayName,
173173
Description: def.Description,

cli/src/cmd/app/commands/core.go

Lines changed: 31 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,37 @@ import (
1010
"github.com/jongio/azd-core/cliout"
1111
)
1212

13-
// Global orchestrator instance shared across all commands.
14-
var cmdOrchestrator *orchestrator.Orchestrator
13+
func newCommandOrchestrator() *orchestrator.Orchestrator {
14+
cmdOrchestrator := orchestrator.NewOrchestrator()
15+
16+
register := func(command *orchestrator.Command) {
17+
if err := cmdOrchestrator.Register(command); err != nil {
18+
fmt.Fprintf(os.Stderr, "Warning: Failed to register %s command: %v\n", command.Name, err)
19+
}
20+
}
21+
22+
register(&orchestrator.Command{
23+
Name: "reqs",
24+
Execute: executeReqs,
25+
})
26+
register(&orchestrator.Command{
27+
Name: "deps",
28+
Dependencies: []string{"reqs"},
29+
Execute: executeDeps,
30+
})
31+
register(&orchestrator.Command{
32+
Name: "run",
33+
Dependencies: []string{"deps"},
34+
Execute: executeRun,
35+
})
36+
register(&orchestrator.Command{
37+
Name: "test",
38+
Dependencies: []string{"deps"},
39+
Execute: executeTest,
40+
})
41+
42+
return cmdOrchestrator
43+
}
1544

1645
// ExecutionContext holds runtime configuration for command execution.
1746
type ExecutionContext struct {
@@ -65,48 +94,6 @@ func SetCacheEnabled(enabled bool) {
6594
execContext.CacheEnabled = enabled
6695
}
6796

68-
// init initializes the command orchestrator and registers all commands.
69-
func init() {
70-
cmdOrchestrator = orchestrator.NewOrchestrator()
71-
72-
// Register commands with their dependencies
73-
// reqs has no dependencies
74-
if err := cmdOrchestrator.Register(&orchestrator.Command{
75-
Name: "reqs",
76-
Execute: executeReqs,
77-
}); err != nil {
78-
// Log error but don't exit - let the app handle it gracefully
79-
fmt.Fprintf(os.Stderr, "Warning: Failed to register reqs command: %v\n", err)
80-
}
81-
82-
// deps depends on reqs
83-
if err := cmdOrchestrator.Register(&orchestrator.Command{
84-
Name: "deps",
85-
Dependencies: []string{"reqs"},
86-
Execute: executeDeps,
87-
}); err != nil {
88-
fmt.Fprintf(os.Stderr, "Warning: Failed to register deps command: %v\n", err)
89-
}
90-
91-
// run depends on deps (which depends on reqs)
92-
if err := cmdOrchestrator.Register(&orchestrator.Command{
93-
Name: "run",
94-
Dependencies: []string{"deps"},
95-
Execute: executeRun,
96-
}); err != nil {
97-
fmt.Fprintf(os.Stderr, "Warning: Failed to register run command: %v\n", err)
98-
}
99-
100-
// test depends on deps (test tools like jest/vitest must be installed first)
101-
if err := cmdOrchestrator.Register(&orchestrator.Command{
102-
Name: "test",
103-
Dependencies: []string{"deps"},
104-
Execute: executeTest,
105-
}); err != nil {
106-
fmt.Fprintf(os.Stderr, "Warning: Failed to register test command: %v\n", err)
107-
}
108-
}
109-
11097
// executeReqs is the core logic for the reqs command.
11198
func executeReqs() error {
11299
cliout.CommandHeader("reqs", "Check required prerequisites")

cli/src/cmd/app/commands/core_deps.go

Lines changed: 62 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ import (
1818
// DependencyInstaller handles installation of project dependencies.
1919
type DependencyInstaller struct {
2020
searchRoot string
21-
nodeProjects []types.NodeProject // Pre-filtered Node.js projects (optional)
22-
pythonProjects []types.PythonProject // Pre-filtered Python projects (optional)
23-
dotnetProjects []types.DotnetProject // Pre-filtered .NET projects (optional)
21+
projects DetectedProjects // Pre-filtered projects (optional)
22+
nodeProjects []types.NodeProject
23+
pythonProjects []types.PythonProject
24+
dotnetProjects []types.DotnetProject
2425
}
2526

2627
// NewDependencyInstaller creates a new dependency installer.
@@ -84,22 +85,27 @@ func (di *DependencyInstaller) InstallAll() ([]InstallResult, error) {
8485
func (di *DependencyInstaller) InstallAllFiltered() ([]InstallResult, error) {
8586
var results []InstallResult
8687

87-
// Install Node.js dependencies from pre-filtered list
88-
if len(di.nodeProjects) > 0 {
89-
nodeResults := di.installNodeProjectList(di.nodeProjects)
90-
results = append(results, nodeResults...)
88+
nodeProjects := di.projects.Node
89+
pythonProjects := di.projects.Python
90+
dotnetProjects := di.projects.Dotnet
91+
if nodeProjects == nil {
92+
nodeProjects = di.nodeProjects
9193
}
92-
93-
// Install Python dependencies from pre-filtered list
94-
if len(di.pythonProjects) > 0 {
95-
pythonResults := di.installPythonProjectList(di.pythonProjects)
96-
results = append(results, pythonResults...)
94+
if pythonProjects == nil {
95+
pythonProjects = di.pythonProjects
96+
}
97+
if dotnetProjects == nil {
98+
dotnetProjects = di.dotnetProjects
9799
}
98100

99-
// Install .NET dependencies from pre-filtered list
100-
if len(di.dotnetProjects) > 0 {
101-
dotnetResults := di.installDotnetProjectList(di.dotnetProjects)
102-
results = append(results, dotnetResults...)
101+
if len(nodeProjects) > 0 {
102+
results = append(results, di.installNodeProjectList(nodeProjects)...)
103+
}
104+
if len(pythonProjects) > 0 {
105+
results = append(results, di.installPythonProjectList(pythonProjects)...)
106+
}
107+
if len(dotnetProjects) > 0 {
108+
results = append(results, di.installDotnetProjectList(dotnetProjects)...)
103109
}
104110

105111
return results, nil
@@ -261,53 +267,44 @@ func (di *DependencyInstaller) installProject(projectType, dir, manager string,
261267
return result
262268
}
263269

264-
// filterProjectsByService filters projects to only include those matching the specified service names.
265-
func filterProjectsByService(
266-
nodeProjects []types.NodeProject,
267-
pythonProjects []types.PythonProject,
268-
dotnetProjects []types.DotnetProject,
269-
services []string,
270-
searchRoot string,
271-
) ([]types.NodeProject, []types.PythonProject, []types.DotnetProject) {
270+
// filterDetectedProjectsByService filters grouped projects to only include those matching the specified service names.
271+
func filterDetectedProjectsByService(projects DetectedProjects, services []string, searchRoot string) DetectedProjects {
272272
// Build a set of service paths from azure.yaml
273273
servicePaths := make(map[string]bool)
274274

275275
azureYamlPath, err := detector.FindAzureYaml(searchRoot)
276276
if err != nil || azureYamlPath == "" {
277277
// No azure.yaml found, can't filter by service
278-
return nodeProjects, pythonProjects, dotnetProjects
278+
return projects
279279
}
280280

281281
azureYaml, err := parseAzureYaml(azureYamlPath)
282282
if err != nil {
283-
return nodeProjects, pythonProjects, dotnetProjects
283+
return projects
284284
}
285285

286-
azureYamlDir := filepath.Dir(azureYamlPath)
287-
288286
// Build map of service name to absolute path
289287
for name, svc := range azureYaml.Services {
290-
// Check if this service is in the filter list
291288
for _, filterName := range services {
292-
if name == filterName {
293-
svcPath := filepath.Join(azureYamlDir, svc.Project)
294-
absPath, err := filepath.Abs(svcPath)
295-
if err != nil {
296-
// Log warning but continue processing other services
297-
if !cliout.IsJSON() {
298-
cliout.Warning("Failed to resolve absolute path for service %s: %v", name, err)
299-
}
300-
continue
289+
if name != filterName {
290+
continue
291+
}
292+
293+
absPath, err := filepath.Abs(svc.Project)
294+
if err != nil {
295+
if !cliout.IsJSON() {
296+
cliout.Warning("Failed to resolve absolute path for service %s: %v", name, err)
301297
}
302-
servicePaths[absPath] = true
303-
break
298+
continue
304299
}
300+
servicePaths[absPath] = true
301+
break
305302
}
306303
}
307304

308305
// Filter Node.js projects
309306
var filteredNode []types.NodeProject
310-
for _, p := range nodeProjects {
307+
for _, p := range projects.Node {
311308
absDir, _ := filepath.Abs(p.Dir)
312309
if servicePaths[absDir] || isSubdirectory(absDir, servicePaths) {
313310
filteredNode = append(filteredNode, p)
@@ -316,7 +313,7 @@ func filterProjectsByService(
316313

317314
// Filter Python projects
318315
var filteredPython []types.PythonProject
319-
for _, p := range pythonProjects {
316+
for _, p := range projects.Python {
320317
absDir, _ := filepath.Abs(p.Dir)
321318
if servicePaths[absDir] || isSubdirectory(absDir, servicePaths) {
322319
filteredPython = append(filteredPython, p)
@@ -325,15 +322,30 @@ func filterProjectsByService(
325322

326323
// Filter .NET projects
327324
var filteredDotnet []types.DotnetProject
328-
for _, p := range dotnetProjects {
325+
for _, p := range projects.Dotnet {
329326
absPath, _ := filepath.Abs(p.Path)
330327
absDir := filepath.Dir(absPath)
331328
if servicePaths[absDir] || isSubdirectory(absDir, servicePaths) {
332329
filteredDotnet = append(filteredDotnet, p)
333330
}
334331
}
335332

336-
return filteredNode, filteredPython, filteredDotnet
333+
return DetectedProjects{
334+
Node: filteredNode,
335+
Python: filteredPython,
336+
Dotnet: filteredDotnet,
337+
}
338+
}
339+
340+
// filterProjectsByService preserves the legacy test-facing signature while delegating to grouped project filtering.
341+
func filterProjectsByService(nodeProjects []types.NodeProject, pythonProjects []types.PythonProject, dotnetProjects []types.DotnetProject, services []string, searchRoot string) ([]types.NodeProject, []types.PythonProject, []types.DotnetProject) {
342+
filtered := filterDetectedProjectsByService(DetectedProjects{
343+
Node: nodeProjects,
344+
Python: pythonProjects,
345+
Dotnet: dotnetProjects,
346+
}, services, searchRoot)
347+
348+
return filtered.Node, filtered.Python, filtered.Dotnet
337349
}
338350

339351
// detectProjectsFromAzureYaml reads azure.yaml and detects project types directly from
@@ -462,23 +474,23 @@ func isSubdirectory(path string, parentPaths map[string]bool) bool {
462474
}
463475

464476
// runParallelInstallation runs the parallel installer for non-JSON mode.
465-
func runParallelInstallation(nodeProjects []types.NodeProject, pythonProjects []types.PythonProject, dotnetProjects []types.DotnetProject, verbose bool) error {
477+
func runParallelInstallation(projects DetectedProjects, verbose bool) error {
466478
parallelInstaller := installer.NewParallelInstaller()
467479
parallelInstaller.Verbose = verbose
468480

469481
// Handle npm/yarn/pnpm workspace scenarios using workspace handler
470482
// When a workspace root exists, only install at the root level to avoid race conditions
471483
// on Windows where parallel npm installs compete for the same node_modules directory
472484
workspaceHandler := workspace.NewHandler()
473-
filteredNodeProjects := workspaceHandler.FilterNodeProjects(nodeProjects)
485+
filteredNodeProjects := workspaceHandler.FilterNodeProjects(projects.Node)
474486

475487
for _, project := range filteredNodeProjects {
476488
parallelInstaller.AddNodeProject(project)
477489
}
478-
for _, project := range pythonProjects {
490+
for _, project := range projects.Python {
479491
parallelInstaller.AddPythonProject(project)
480492
}
481-
for _, project := range dotnetProjects {
493+
for _, project := range projects.Dotnet {
482494
parallelInstaller.AddDotnetProject(project)
483495
}
484496

@@ -500,11 +512,9 @@ func runParallelInstallation(nodeProjects []types.NodeProject, pythonProjects []
500512
}
501513

502514
// runJSONInstallation runs installation in JSON mode with sequential cliout.
503-
func runJSONInstallation(searchRoot string, nodeProjects []types.NodeProject, pythonProjects []types.PythonProject, dotnetProjects []types.DotnetProject) error {
515+
func runJSONInstallation(searchRoot string, projects DetectedProjects) error {
504516
depInstaller := NewDependencyInstaller(searchRoot)
505-
depInstaller.nodeProjects = nodeProjects
506-
depInstaller.pythonProjects = pythonProjects
507-
depInstaller.dotnetProjects = dotnetProjects
517+
depInstaller.projects = projects
508518

509519
results, err := depInstaller.InstallAllFiltered()
510520
if err != nil {

0 commit comments

Comments
 (0)