From 0e950767d9b02f50148654733ea6fbd5736faf2b Mon Sep 17 00:00:00 2001 From: James Pogran Date: Tue, 16 Jul 2024 14:57:03 -0400 Subject: [PATCH 1/3] Load Stack component sources from metadata This change adds a new job to load the component sources of a stack after parsing the stack metadata. For a component with a module source, the job will trigger a `did_open` event for the Modules feature to handle. This will allow the Modules feature to load the module and its dependencies, leading to eventually being able to complete Module references. This only handles local modules at the moment. Remote modules are not supported yet. --- .../ENHANCEMENTS-20240717-114614.yaml | 6 ++ go.mod | 2 +- go.sum | 4 +- internal/features/stacks/events.go | 21 +++++++ .../features/stacks/jobs/component_sources.go | 63 +++++++++++++++++++ .../module/operation/op_type_string.go | 7 ++- .../terraform/module/operation/operation.go | 1 + 7 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 .changes/unreleased/ENHANCEMENTS-20240717-114614.yaml create mode 100644 internal/features/stacks/jobs/component_sources.go diff --git a/.changes/unreleased/ENHANCEMENTS-20240717-114614.yaml b/.changes/unreleased/ENHANCEMENTS-20240717-114614.yaml new file mode 100644 index 00000000..f1ba39d8 --- /dev/null +++ b/.changes/unreleased/ENHANCEMENTS-20240717-114614.yaml @@ -0,0 +1,6 @@ +kind: ENHANCEMENTS +body: Load Stack component sources from metadata +time: 2024-07-17T11:46:14.048412-04:00 +custom: + Issue: "1768" + Repository: terraform-ls diff --git a/go.mod b/go.mod index 77c5dc74..989b6612 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( github.com/hashicorp/terraform-exec v0.21.0 github.com/hashicorp/terraform-json v0.22.1 github.com/hashicorp/terraform-registry-address v0.2.3 - github.com/hashicorp/terraform-schema v0.0.0-20240717123934-4ae973d1b11b + github.com/hashicorp/terraform-schema v0.0.0-20240717154107-7d112f69e8e0 github.com/mcuadros/go-defaults v1.2.0 github.com/mh-cbon/go-fmt-fail v0.0.0-20160815164508-67765b3fbcb5 github.com/mitchellh/cli v1.1.5 diff --git a/go.sum b/go.sum index 1d642943..3dacc9b5 100644 --- a/go.sum +++ b/go.sum @@ -229,8 +229,8 @@ github.com/hashicorp/terraform-json v0.22.1 h1:xft84GZR0QzjPVWs4lRUwvTcPnegqlyS7 github.com/hashicorp/terraform-json v0.22.1/go.mod h1:JbWSQCLFSXFFhg42T7l9iJwdGXBYV8fmmD6o/ML4p3A= github.com/hashicorp/terraform-registry-address v0.2.3 h1:2TAiKJ1A3MAkZlH1YI/aTVcLZRu7JseiXNRHbOAyoTI= github.com/hashicorp/terraform-registry-address v0.2.3/go.mod h1:lFHA76T8jfQteVfT7caREqguFrW3c4MFSPhZB7HHgUM= -github.com/hashicorp/terraform-schema v0.0.0-20240717123934-4ae973d1b11b h1:CI+WVmvNp63uPudDQy6d66y3ad2n8bY+gyePuwMeT0E= -github.com/hashicorp/terraform-schema v0.0.0-20240717123934-4ae973d1b11b/go.mod h1:ar787Bv/qD6tlnjtzH8fQ1Yi6c/B5LsnpFlO8c92Atg= +github.com/hashicorp/terraform-schema v0.0.0-20240717154107-7d112f69e8e0 h1:qgbO+ZzDng3AOKZ8GQ+BuoF24hYNM6EzaQMK8ICPbuw= +github.com/hashicorp/terraform-schema v0.0.0-20240717154107-7d112f69e8e0/go.mod h1:ar787Bv/qD6tlnjtzH8fQ1Yi6c/B5LsnpFlO8c92Atg= github.com/hashicorp/terraform-svchost v0.1.1 h1:EZZimZ1GxdqFRinZ1tpJwVxxt49xc/S52uzrw4x0jKQ= github.com/hashicorp/terraform-svchost v0.1.1/go.mod h1:mNsjQfZyf/Jhz35v6/0LWcv26+X7JPS+buii2c9/ctc= github.com/hexops/autogold v1.3.1 h1:YgxF9OHWbEIUjhDbpnLhgVsjUDsiHDTyDfy2lrfdlzo= diff --git a/internal/features/stacks/events.go b/internal/features/stacks/events.go index d12b10d8..4d42b306 100644 --- a/internal/features/stacks/events.go +++ b/internal/features/stacks/events.go @@ -195,6 +195,27 @@ func (f *StacksFeature) decodeStack(ctx context.Context, dir document.DirHandle, Type: operation.OpTypeLoadStackMetadata.String(), DependsOn: job.IDs{parseId}, IgnoreState: ignoreState, + Defer: func(ctx context.Context, jobErr error) (job.IDs, error) { + deferIds := make(job.IDs, 0) + if jobErr != nil { + f.logger.Printf("loading module metadata returned error: %s", jobErr) + } + + componentSourcesId, err := f.stateStore.JobStore.EnqueueJob(ctx, job.Job{ + Dir: dir, + Func: func(ctx context.Context) error { + return jobs.LoadStackComponentSources(ctx, f.store, f.bus, path) + }, + Type: operation.OpTypeLoadStackComponentSources.String(), + IgnoreState: ignoreState, + }) + if err != nil { + return deferIds, err + } + deferIds = append(deferIds, componentSourcesId) + + return deferIds, nil + }, }) if err != nil { return ids, err diff --git a/internal/features/stacks/jobs/component_sources.go b/internal/features/stacks/jobs/component_sources.go new file mode 100644 index 00000000..3776b8d7 --- /dev/null +++ b/internal/features/stacks/jobs/component_sources.go @@ -0,0 +1,63 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package jobs + +import ( + "context" + "path/filepath" + + "github.com/hashicorp/terraform-ls/internal/document" + "github.com/hashicorp/terraform-ls/internal/eventbus" + "github.com/hashicorp/terraform-ls/internal/features/stacks/state" + "github.com/hashicorp/terraform-ls/internal/lsp" + tfaddr "github.com/hashicorp/terraform-registry-address" + "github.com/hashicorp/terraform-schema/module" +) + +// LoadStackComponentSources will trigger parsing the local terraform modules for a stack in the ModulesFeature +func LoadStackComponentSources(ctx context.Context, stackStore *state.StackStore, bus *eventbus.EventBus, stackPath string) error { + record, err := stackStore.StackRecordByPath(stackPath) + if err != nil { + return err + } + + // iterate over each component in the stack and find local terraform modules + for _, component := range record.Meta.Components { + if component.Source == "" { + // no source recorded, skip + continue + } + + var fullPath string + // detect if component.Source is a local module + switch component.SourceAddr.(type) { + case module.LocalSourceAddr: + fullPath = filepath.Join(stackPath, filepath.FromSlash(component.Source)) + case tfaddr.Module: + continue + case module.RemoteSourceAddr: + continue + default: + // Unknown source address, we can't resolve the path + continue + } + + if fullPath == "" { + // Unknown source address, we can't resolve the path + continue + } + + dh := document.DirHandleFromPath(fullPath) + + // notify the event bus that a new Component with a + // local modules has been opened + bus.DidOpen(eventbus.DidOpenEvent{ + Context: ctx, + Dir: dh, + LanguageID: lsp.Terraform.String(), + }) + } + + return nil +} diff --git a/internal/terraform/module/operation/op_type_string.go b/internal/terraform/module/operation/op_type_string.go index 705f0a96..67090f50 100644 --- a/internal/terraform/module/operation/op_type_string.go +++ b/internal/terraform/module/operation/op_type_string.go @@ -29,12 +29,13 @@ func _() { _ = x[OpTypeTerraformValidate-18] _ = x[OpTypeParseStackConfiguration-19] _ = x[OpTypeLoadStackMetadata-20] - _ = x[OpTypeLoadStackRequiredTerraformVersion-21] + _ = x[OpTypeLoadStackComponentSources-21] + _ = x[OpTypeLoadStackRequiredTerraformVersion-22] } -const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeGetInstalledTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeStacksPreloadEmbeddedSchemaOpTypeSchemaModuleValidationOpTypeSchemaVarsValidationOpTypeReferenceValidationOpTypeTerraformValidateOpTypeParseStackConfigurationOpTypeLoadStackMetadataOpTypeLoadStackRequiredTerraformVersion" +const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeGetInstalledTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeStacksPreloadEmbeddedSchemaOpTypeSchemaModuleValidationOpTypeSchemaVarsValidationOpTypeReferenceValidationOpTypeTerraformValidateOpTypeParseStackConfigurationOpTypeLoadStackMetadataOpTypeLoadStackComponentSourcesOpTypeLoadStackRequiredTerraformVersion" -var _OpType_index = [...]uint16{0, 13, 38, 72, 90, 120, 140, 165, 189, 217, 245, 271, 302, 329, 356, 389, 417, 443, 468, 491, 520, 543, 582} +var _OpType_index = [...]uint16{0, 13, 38, 72, 90, 120, 140, 165, 189, 217, 245, 271, 302, 329, 356, 389, 417, 443, 468, 491, 520, 543, 574, 613} func (i OpType) String() string { if i >= OpType(len(_OpType_index)-1) { diff --git a/internal/terraform/module/operation/operation.go b/internal/terraform/module/operation/operation.go index 1043cdc9..34cfcaa4 100644 --- a/internal/terraform/module/operation/operation.go +++ b/internal/terraform/module/operation/operation.go @@ -38,5 +38,6 @@ const ( OpTypeTerraformValidate OpTypeParseStackConfiguration OpTypeLoadStackMetadata + OpTypeLoadStackComponentSources OpTypeLoadStackRequiredTerraformVersion ) From 26d28aa190f41e84c458f18382f6700caf14135d Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Fri, 19 Jul 2024 18:16:57 +0200 Subject: [PATCH 2/3] refactor: support returning job ids when handling events in order to allow jobs to wait for kicked-off events to settle Use this new functionality in LoadStackComponentSources to allow waiting for parsing local modules used in stacks components before other jobs run that need the parsing results --- internal/eventbus/bus.go | 17 +++++++++++---- internal/eventbus/did_change.go | 2 +- internal/eventbus/did_change_watched.go | 2 +- internal/eventbus/did_open.go | 7 ++++--- internal/eventbus/discover.go | 2 +- internal/eventbus/lockfile_change.go | 2 +- internal/eventbus/manifest_change.go | 2 +- internal/features/modules/modules_feature.go | 19 +++++++++-------- .../rootmodules/root_modules_feature.go | 19 +++++++++-------- internal/features/stacks/events.go | 21 +++++++------------ .../features/stacks/jobs/component_sources.go | 13 ++++++++---- internal/features/stacks/stacks_feature.go | 19 +++++++++-------- .../features/variables/variables_feature.go | 19 +++++++++-------- internal/job/job.go | 2 ++ 14 files changed, 80 insertions(+), 66 deletions(-) diff --git a/internal/eventbus/bus.go b/internal/eventbus/bus.go index 2efb0d3c..78c08869 100644 --- a/internal/eventbus/bus.go +++ b/internal/eventbus/bus.go @@ -7,6 +7,8 @@ import ( "io" "log" "sync" + + "github.com/hashicorp/terraform-ls/internal/job" ) const ChannelSize = 10 @@ -52,6 +54,8 @@ type Topic[T any] struct { mutex sync.Mutex } +type DoneChannel <-chan job.IDs + // Subscriber represents a subscriber to a topic type Subscriber[T any] struct { // channel is the channel to which all events of the topic are sent @@ -59,7 +63,7 @@ type Subscriber[T any] struct { // doneChannel is an optional channel that the subscriber can use to signal // that it is done processing the event - doneChannel <-chan struct{} + doneChannel DoneChannel } // NewTopic creates a new topic @@ -70,7 +74,7 @@ func NewTopic[T any]() *Topic[T] { } // Subscribe adds a subscriber to a topic -func (eb *Topic[T]) Subscribe(doneChannel <-chan struct{}) <-chan T { +func (eb *Topic[T]) Subscribe(doneChannel <-chan job.IDs) <-chan T { channel := make(chan T, ChannelSize) eb.mutex.Lock() defer eb.mutex.Unlock() @@ -83,7 +87,9 @@ func (eb *Topic[T]) Subscribe(doneChannel <-chan struct{}) <-chan T { } // Publish sends an event to all subscribers of a specific topic -func (eb *Topic[T]) Publish(event T) { +func (eb *Topic[T]) Publish(event T) job.IDs { + ids := make(job.IDs, 0) + eb.mutex.Lock() defer eb.mutex.Unlock() @@ -93,7 +99,10 @@ func (eb *Topic[T]) Publish(event T) { if subscriber.doneChannel != nil { // And wait until the subscriber is done processing it - <-subscriber.doneChannel + spawnedIds := <-subscriber.doneChannel + ids = append(ids, spawnedIds...) } } + + return ids } diff --git a/internal/eventbus/did_change.go b/internal/eventbus/did_change.go index 02e20c32..79e0d827 100644 --- a/internal/eventbus/did_change.go +++ b/internal/eventbus/did_change.go @@ -20,7 +20,7 @@ type DidChangeEvent struct { LanguageID string } -func (n *EventBus) OnDidChange(identifier string, doneChannel <-chan struct{}) <-chan DidChangeEvent { +func (n *EventBus) OnDidChange(identifier string, doneChannel DoneChannel) <-chan DidChangeEvent { n.logger.Printf("bus: %q subscribed to OnDidChange", identifier) return n.didChangeTopic.Subscribe(doneChannel) } diff --git a/internal/eventbus/did_change_watched.go b/internal/eventbus/did_change_watched.go index 9ddce994..995298c6 100644 --- a/internal/eventbus/did_change_watched.go +++ b/internal/eventbus/did_change_watched.go @@ -23,7 +23,7 @@ type DidChangeWatchedEvent struct { ChangeType protocol.FileChangeType } -func (n *EventBus) OnDidChangeWatched(identifier string, doneChannel <-chan struct{}) <-chan DidChangeWatchedEvent { +func (n *EventBus) OnDidChangeWatched(identifier string, doneChannel DoneChannel) <-chan DidChangeWatchedEvent { n.logger.Printf("bus: %q subscribed to OnDidChangeWatched", identifier) return n.didChangeWatchedTopic.Subscribe(doneChannel) } diff --git a/internal/eventbus/did_open.go b/internal/eventbus/did_open.go index ff5836c9..1c32bf81 100644 --- a/internal/eventbus/did_open.go +++ b/internal/eventbus/did_open.go @@ -7,6 +7,7 @@ import ( "context" "github.com/hashicorp/terraform-ls/internal/document" + "github.com/hashicorp/terraform-ls/internal/job" ) // DidOpenEvent is an event to signal that a directory is open in the editor @@ -23,12 +24,12 @@ type DidOpenEvent struct { LanguageID string } -func (n *EventBus) OnDidOpen(identifier string, doneChannel <-chan struct{}) <-chan DidOpenEvent { +func (n *EventBus) OnDidOpen(identifier string, doneChannel <-chan job.IDs) <-chan DidOpenEvent { n.logger.Printf("bus: %q subscribed to OnDidOpen", identifier) return n.didOpenTopic.Subscribe(doneChannel) } -func (n *EventBus) DidOpen(e DidOpenEvent) { +func (n *EventBus) DidOpen(e DidOpenEvent) job.IDs { n.logger.Printf("bus: -> DidOpen %s", e.Dir) - n.didOpenTopic.Publish(e) + return n.didOpenTopic.Publish(e) } diff --git a/internal/eventbus/discover.go b/internal/eventbus/discover.go index 99b5d68b..cea31a2c 100644 --- a/internal/eventbus/discover.go +++ b/internal/eventbus/discover.go @@ -17,7 +17,7 @@ type DiscoverEvent struct { Files []string } -func (n *EventBus) OnDiscover(identifier string, doneChannel <-chan struct{}) <-chan DiscoverEvent { +func (n *EventBus) OnDiscover(identifier string, doneChannel DoneChannel) <-chan DiscoverEvent { n.logger.Printf("bus: %q subscribed to OnDiscover", identifier) return n.discoverTopic.Subscribe(doneChannel) } diff --git a/internal/eventbus/lockfile_change.go b/internal/eventbus/lockfile_change.go index 5ab1ef97..e3a9b888 100644 --- a/internal/eventbus/lockfile_change.go +++ b/internal/eventbus/lockfile_change.go @@ -19,7 +19,7 @@ type PluginLockChangeEvent struct { ChangeType protocol.FileChangeType } -func (n *EventBus) OnPluginLockChange(identifier string, doneChannel <-chan struct{}) <-chan PluginLockChangeEvent { +func (n *EventBus) OnPluginLockChange(identifier string, doneChannel DoneChannel) <-chan PluginLockChangeEvent { n.logger.Printf("bus: %q subscribed to OnPluginLockChange", identifier) return n.pluginLockChangeTopic.Subscribe(doneChannel) } diff --git a/internal/eventbus/manifest_change.go b/internal/eventbus/manifest_change.go index add9eb53..265207d9 100644 --- a/internal/eventbus/manifest_change.go +++ b/internal/eventbus/manifest_change.go @@ -19,7 +19,7 @@ type ManifestChangeEvent struct { ChangeType protocol.FileChangeType } -func (n *EventBus) OnManifestChange(identifier string, doneChannel <-chan struct{}) <-chan ManifestChangeEvent { +func (n *EventBus) OnManifestChange(identifier string, doneChannel DoneChannel) <-chan ManifestChangeEvent { n.logger.Printf("bus: %q subscribed to OnManifestChange", identifier) return n.manifestChangeTopic.Subscribe(doneChannel) } diff --git a/internal/features/modules/modules_feature.go b/internal/features/modules/modules_feature.go index 72e7ec5c..f555f871 100644 --- a/internal/features/modules/modules_feature.go +++ b/internal/features/modules/modules_feature.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform-ls/internal/features/modules/hooks" "github.com/hashicorp/terraform-ls/internal/features/modules/jobs" "github.com/hashicorp/terraform-ls/internal/features/modules/state" + "github.com/hashicorp/terraform-ls/internal/job" "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" "github.com/hashicorp/terraform-ls/internal/registry" globalState "github.com/hashicorp/terraform-ls/internal/state" @@ -74,13 +75,13 @@ func (f *ModulesFeature) Start(ctx context.Context) { discover := f.eventbus.OnDiscover("feature.modules", nil) - didOpenDone := make(chan struct{}, 10) + didOpenDone := make(chan job.IDs, 10) didOpen := f.eventbus.OnDidOpen("feature.modules", didOpenDone) - didChangeDone := make(chan struct{}, 10) + didChangeDone := make(chan job.IDs, 10) didChange := f.eventbus.OnDidChange("feature.modules", didChangeDone) - didChangeWatchedDone := make(chan struct{}, 10) + didChangeWatchedDone := make(chan job.IDs, 10) didChangeWatched := f.eventbus.OnDidChangeWatched("feature.modules", didChangeWatchedDone) go func() { @@ -91,16 +92,16 @@ func (f *ModulesFeature) Start(ctx context.Context) { f.discover(discover.Path, discover.Files) case didOpen := <-didOpen: // TODO? collect errors - f.didOpen(didOpen.Context, didOpen.Dir, didOpen.LanguageID) - didOpenDone <- struct{}{} + spawnedIds, _ := f.didOpen(didOpen.Context, didOpen.Dir, didOpen.LanguageID) + didOpenDone <- spawnedIds case didChange := <-didChange: // TODO? collect errors - f.didChange(didChange.Context, didChange.Dir) - didChangeDone <- struct{}{} + spawnedIds, _ := f.didChange(didChange.Context, didChange.Dir) + didChangeDone <- spawnedIds case didChangeWatched := <-didChangeWatched: // TODO? collect errors - f.didChangeWatched(didChangeWatched.Context, didChangeWatched.RawPath, didChangeWatched.ChangeType, didChangeWatched.IsDir) - didChangeWatchedDone <- struct{}{} + spawnedIds, _ := f.didChangeWatched(didChangeWatched.Context, didChangeWatched.RawPath, didChangeWatched.ChangeType, didChangeWatched.IsDir) + didChangeWatchedDone <- spawnedIds case <-ctx.Done(): return diff --git a/internal/features/rootmodules/root_modules_feature.go b/internal/features/rootmodules/root_modules_feature.go index c51bcc0a..2c708f9c 100644 --- a/internal/features/rootmodules/root_modules_feature.go +++ b/internal/features/rootmodules/root_modules_feature.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform-ls/internal/eventbus" "github.com/hashicorp/terraform-ls/internal/features/rootmodules/jobs" "github.com/hashicorp/terraform-ls/internal/features/rootmodules/state" + "github.com/hashicorp/terraform-ls/internal/job" globalState "github.com/hashicorp/terraform-ls/internal/state" "github.com/hashicorp/terraform-ls/internal/telemetry" "github.com/hashicorp/terraform-ls/internal/terraform/exec" @@ -70,13 +71,13 @@ func (f *RootModulesFeature) Start(ctx context.Context) { discover := f.eventbus.OnDiscover("feature.rootmodules", nil) - didOpenDone := make(chan struct{}, 10) + didOpenDone := make(chan job.IDs, 10) didOpen := f.eventbus.OnDidOpen("feature.rootmodules", didOpenDone) - manifestChangeDone := make(chan struct{}, 10) + manifestChangeDone := make(chan job.IDs, 10) manifestChange := f.eventbus.OnManifestChange("feature.rootmodules", manifestChangeDone) - pluginLockChangeDone := make(chan struct{}, 10) + pluginLockChangeDone := make(chan job.IDs, 10) pluginLockChange := f.eventbus.OnPluginLockChange("feature.rootmodules", pluginLockChangeDone) go func() { @@ -87,16 +88,16 @@ func (f *RootModulesFeature) Start(ctx context.Context) { f.discover(discover.Path, discover.Files) case didOpen := <-didOpen: // TODO? collect errors - f.didOpen(didOpen.Context, didOpen.Dir) - didOpenDone <- struct{}{} + spawnedIds, _ := f.didOpen(didOpen.Context, didOpen.Dir) + didOpenDone <- spawnedIds case manifestChange := <-manifestChange: // TODO? collect errors - f.manifestChange(manifestChange.Context, manifestChange.Dir, manifestChange.ChangeType) - manifestChangeDone <- struct{}{} + spawnedIds, _ := f.manifestChange(manifestChange.Context, manifestChange.Dir, manifestChange.ChangeType) + manifestChangeDone <- spawnedIds case pluginLockChange := <-pluginLockChange: // TODO? collect errors - f.pluginLockChange(pluginLockChange.Context, pluginLockChange.Dir) - pluginLockChangeDone <- struct{}{} + spawnedIds, _ := f.pluginLockChange(pluginLockChange.Context, pluginLockChange.Dir) + pluginLockChangeDone <- spawnedIds case <-ctx.Done(): return diff --git a/internal/features/stacks/events.go b/internal/features/stacks/events.go index 4d42b306..8af087ab 100644 --- a/internal/features/stacks/events.go +++ b/internal/features/stacks/events.go @@ -196,25 +196,18 @@ func (f *StacksFeature) decodeStack(ctx context.Context, dir document.DirHandle, DependsOn: job.IDs{parseId}, IgnoreState: ignoreState, Defer: func(ctx context.Context, jobErr error) (job.IDs, error) { - deferIds := make(job.IDs, 0) if jobErr != nil { f.logger.Printf("loading module metadata returned error: %s", jobErr) } - componentSourcesId, err := f.stateStore.JobStore.EnqueueJob(ctx, job.Job{ - Dir: dir, - Func: func(ctx context.Context) error { - return jobs.LoadStackComponentSources(ctx, f.store, f.bus, path) - }, - Type: operation.OpTypeLoadStackComponentSources.String(), - IgnoreState: ignoreState, - }) - if err != nil { - return deferIds, err - } - deferIds = append(deferIds, componentSourcesId) + spawnedIds, err := jobs.LoadStackComponentSources(ctx, f.store, f.bus, path) + + // while we now have the job ids in here, depending on the metaId job is not enough + // to await these spawned jobs, so we will need to move all depending jobs to this function + // as well. e.g. LoadStackComponentSources, PreloadEmbeddedSchema (because future ref collection jobs depend on it), etc. + // we might just move all in here for simplicity - return deferIds, nil + return spawnedIds, err }, }) if err != nil { diff --git a/internal/features/stacks/jobs/component_sources.go b/internal/features/stacks/jobs/component_sources.go index 3776b8d7..7b27f6ba 100644 --- a/internal/features/stacks/jobs/component_sources.go +++ b/internal/features/stacks/jobs/component_sources.go @@ -10,16 +10,19 @@ import ( "github.com/hashicorp/terraform-ls/internal/document" "github.com/hashicorp/terraform-ls/internal/eventbus" "github.com/hashicorp/terraform-ls/internal/features/stacks/state" + "github.com/hashicorp/terraform-ls/internal/job" "github.com/hashicorp/terraform-ls/internal/lsp" tfaddr "github.com/hashicorp/terraform-registry-address" "github.com/hashicorp/terraform-schema/module" ) // LoadStackComponentSources will trigger parsing the local terraform modules for a stack in the ModulesFeature -func LoadStackComponentSources(ctx context.Context, stackStore *state.StackStore, bus *eventbus.EventBus, stackPath string) error { +func LoadStackComponentSources(ctx context.Context, stackStore *state.StackStore, bus *eventbus.EventBus, stackPath string) (job.IDs, error) { + ids := make(job.IDs, 0) + record, err := stackStore.StackRecordByPath(stackPath) if err != nil { - return err + return ids, err } // iterate over each component in the stack and find local terraform modules @@ -52,12 +55,14 @@ func LoadStackComponentSources(ctx context.Context, stackStore *state.StackStore // notify the event bus that a new Component with a // local modules has been opened - bus.DidOpen(eventbus.DidOpenEvent{ + spawnedIds := bus.DidOpen(eventbus.DidOpenEvent{ Context: ctx, Dir: dh, LanguageID: lsp.Terraform.String(), }) + + ids = append(ids, spawnedIds...) } - return nil + return ids, nil } diff --git a/internal/features/stacks/stacks_feature.go b/internal/features/stacks/stacks_feature.go index b80b2417..a197c760 100644 --- a/internal/features/stacks/stacks_feature.go +++ b/internal/features/stacks/stacks_feature.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/terraform-ls/internal/features/modules/jobs" stackDecoder "github.com/hashicorp/terraform-ls/internal/features/stacks/decoder" "github.com/hashicorp/terraform-ls/internal/features/stacks/state" + "github.com/hashicorp/terraform-ls/internal/job" "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" globalState "github.com/hashicorp/terraform-ls/internal/state" ) @@ -57,9 +58,9 @@ func (f *StacksFeature) Start(ctx context.Context) { topic := "feature.stacks" - didOpenDone := make(chan struct{}, 10) - didChangeDone := make(chan struct{}, 10) - didChangeWatchedDone := make(chan struct{}, 10) + didOpenDone := make(chan job.IDs, 10) + didChangeDone := make(chan job.IDs, 10) + didChangeWatchedDone := make(chan job.IDs, 10) discover := f.bus.OnDiscover(topic, nil) didOpen := f.bus.OnDidOpen(topic, didOpenDone) @@ -74,16 +75,16 @@ func (f *StacksFeature) Start(ctx context.Context) { f.discover(discover.Path, discover.Files) case didOpen := <-didOpen: // TODO? collect errors - f.didOpen(didOpen.Context, didOpen.Dir, didOpen.LanguageID) - didOpenDone <- struct{}{} + spawnedIds, _ := f.didOpen(didOpen.Context, didOpen.Dir, didOpen.LanguageID) + didOpenDone <- spawnedIds case didChange := <-didChange: // TODO? collect errors - f.didChange(didChange.Context, didChange.Dir) - didChangeDone <- struct{}{} + spawnedIds, _ := f.didChange(didChange.Context, didChange.Dir) + didChangeDone <- spawnedIds case didChangeWatched := <-didChangeWatched: // TODO? collect errors - f.didChangeWatched(didChangeWatched.Context, didChangeWatched.RawPath, didChangeWatched.ChangeType, didChangeWatched.IsDir) - didChangeWatchedDone <- struct{}{} + spawnedIds, _ := f.didChangeWatched(didChangeWatched.Context, didChangeWatched.RawPath, didChangeWatched.ChangeType, didChangeWatched.IsDir) + didChangeWatchedDone <- spawnedIds case <-ctx.Done(): return diff --git a/internal/features/variables/variables_feature.go b/internal/features/variables/variables_feature.go index 090e0c58..070a05f9 100644 --- a/internal/features/variables/variables_feature.go +++ b/internal/features/variables/variables_feature.go @@ -14,6 +14,7 @@ import ( fdecoder "github.com/hashicorp/terraform-ls/internal/features/variables/decoder" "github.com/hashicorp/terraform-ls/internal/features/variables/jobs" "github.com/hashicorp/terraform-ls/internal/features/variables/state" + "github.com/hashicorp/terraform-ls/internal/job" "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" globalState "github.com/hashicorp/terraform-ls/internal/state" ) @@ -62,13 +63,13 @@ func (f *VariablesFeature) Start(ctx context.Context) { discover := f.eventbus.OnDiscover("feature.variables", nil) - didOpenDone := make(chan struct{}, 10) + didOpenDone := make(chan job.IDs, 10) didOpen := f.eventbus.OnDidOpen("feature.variables", didOpenDone) - didChangeDone := make(chan struct{}, 10) + didChangeDone := make(chan job.IDs, 10) didChange := f.eventbus.OnDidChange("feature.variables", didChangeDone) - didChangeWatchedDone := make(chan struct{}, 10) + didChangeWatchedDone := make(chan job.IDs, 10) didChangeWatched := f.eventbus.OnDidChangeWatched("feature.variables", didChangeWatchedDone) go func() { @@ -79,16 +80,16 @@ func (f *VariablesFeature) Start(ctx context.Context) { f.discover(discover.Path, discover.Files) case didOpen := <-didOpen: // TODO? collect errors - f.didOpen(didOpen.Context, didOpen.Dir, didOpen.LanguageID) - didOpenDone <- struct{}{} + spawnedIds, _ := f.didOpen(didOpen.Context, didOpen.Dir, didOpen.LanguageID) + didOpenDone <- spawnedIds case didChange := <-didChange: // TODO? collect errors - f.didChange(didChange.Context, didChange.Dir) - didChangeDone <- struct{}{} + spawnedIds, _ := f.didChange(didChange.Context, didChange.Dir) + didChangeDone <- spawnedIds case didChangeWatched := <-didChangeWatched: // TODO? collect errors - f.didChangeWatched(didChangeWatched.Context, didChangeWatched.RawPath, didChangeWatched.ChangeType, didChangeWatched.IsDir) - didChangeWatchedDone <- struct{}{} + spawnedIds, _ := f.didChangeWatched(didChangeWatched.Context, didChangeWatched.RawPath, didChangeWatched.ChangeType, didChangeWatched.IsDir) + didChangeWatchedDone <- spawnedIds case <-ctx.Done(): return diff --git a/internal/job/job.go b/internal/job/job.go index d96dc0e9..d0cba658 100644 --- a/internal/job/job.go +++ b/internal/job/job.go @@ -29,6 +29,8 @@ type Job struct { // Defer is a function to execute after Func is executed // and before the job is marked as done (StateDone). // This can be used to schedule jobs dependent on the main job. + // Jobs depending on the main job won't automatically depend on + // jobs scheduled by Defer. Defer DeferFunc // DependsOn represents any other job IDs this job depends on. From a74d55a91b1e7372da7d5cf70919c4e15eeaa094 Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Mon, 22 Jul 2024 15:49:48 +0200 Subject: [PATCH 3/3] chore: move function from jobs package to events.go, remove unused code, use DoneChannel type in more places --- internal/eventbus/bus.go | 2 +- internal/eventbus/did_open.go | 2 +- internal/features/stacks/events.go | 57 +++++++++++++++- .../features/stacks/jobs/component_sources.go | 68 ------------------- .../module/operation/op_type_string.go | 7 +- .../terraform/module/operation/operation.go | 1 - 6 files changed, 61 insertions(+), 76 deletions(-) delete mode 100644 internal/features/stacks/jobs/component_sources.go diff --git a/internal/eventbus/bus.go b/internal/eventbus/bus.go index 78c08869..2d2ef901 100644 --- a/internal/eventbus/bus.go +++ b/internal/eventbus/bus.go @@ -74,7 +74,7 @@ func NewTopic[T any]() *Topic[T] { } // Subscribe adds a subscriber to a topic -func (eb *Topic[T]) Subscribe(doneChannel <-chan job.IDs) <-chan T { +func (eb *Topic[T]) Subscribe(doneChannel DoneChannel) <-chan T { channel := make(chan T, ChannelSize) eb.mutex.Lock() defer eb.mutex.Unlock() diff --git a/internal/eventbus/did_open.go b/internal/eventbus/did_open.go index 1c32bf81..7e0a1efc 100644 --- a/internal/eventbus/did_open.go +++ b/internal/eventbus/did_open.go @@ -24,7 +24,7 @@ type DidOpenEvent struct { LanguageID string } -func (n *EventBus) OnDidOpen(identifier string, doneChannel <-chan job.IDs) <-chan DidOpenEvent { +func (n *EventBus) OnDidOpen(identifier string, doneChannel DoneChannel) <-chan DidOpenEvent { n.logger.Printf("bus: %q subscribed to OnDidOpen", identifier) return n.didOpenTopic.Subscribe(doneChannel) } diff --git a/internal/features/stacks/events.go b/internal/features/stacks/events.go index 8af087ab..8b4ef698 100644 --- a/internal/features/stacks/events.go +++ b/internal/features/stacks/events.go @@ -9,14 +9,18 @@ import ( "path/filepath" "github.com/hashicorp/terraform-ls/internal/document" + "github.com/hashicorp/terraform-ls/internal/eventbus" "github.com/hashicorp/terraform-ls/internal/features/stacks/ast" "github.com/hashicorp/terraform-ls/internal/features/stacks/jobs" + "github.com/hashicorp/terraform-ls/internal/features/stacks/state" "github.com/hashicorp/terraform-ls/internal/job" "github.com/hashicorp/terraform-ls/internal/lsp" "github.com/hashicorp/terraform-ls/internal/protocol" "github.com/hashicorp/terraform-ls/internal/schemas" globalAst "github.com/hashicorp/terraform-ls/internal/terraform/ast" "github.com/hashicorp/terraform-ls/internal/terraform/module/operation" + tfaddr "github.com/hashicorp/terraform-registry-address" + "github.com/hashicorp/terraform-schema/module" ) func (f *StacksFeature) discover(path string, files []string) error { @@ -200,7 +204,7 @@ func (f *StacksFeature) decodeStack(ctx context.Context, dir document.DirHandle, f.logger.Printf("loading module metadata returned error: %s", jobErr) } - spawnedIds, err := jobs.LoadStackComponentSources(ctx, f.store, f.bus, path) + spawnedIds, err := loadStackComponentSources(ctx, f.store, f.bus, path) // while we now have the job ids in here, depending on the metaId job is not enough // to await these spawned jobs, so we will need to move all depending jobs to this function @@ -253,3 +257,54 @@ func (f *StacksFeature) removeIndexedStack(rawPath string) { return } } + +// loadStackComponentSources will trigger parsing the local terraform modules for a stack in the ModulesFeature +func loadStackComponentSources(ctx context.Context, stackStore *state.StackStore, bus *eventbus.EventBus, stackPath string) (job.IDs, error) { + ids := make(job.IDs, 0) + + record, err := stackStore.StackRecordByPath(stackPath) + if err != nil { + return ids, err + } + + // iterate over each component in the stack and find local terraform modules + for _, component := range record.Meta.Components { + if component.Source == "" { + // no source recorded, skip + continue + } + + var fullPath string + // detect if component.Source is a local module + switch component.SourceAddr.(type) { + case module.LocalSourceAddr: + fullPath = filepath.Join(stackPath, filepath.FromSlash(component.Source)) + case tfaddr.Module: + continue + case module.RemoteSourceAddr: + continue + default: + // Unknown source address, we can't resolve the path + continue + } + + if fullPath == "" { + // Unknown source address, we can't resolve the path + continue + } + + dh := document.DirHandleFromPath(fullPath) + + // notify the event bus that a new Component with a + // local modules has been opened + spawnedIds := bus.DidOpen(eventbus.DidOpenEvent{ + Context: ctx, + Dir: dh, + LanguageID: lsp.Terraform.String(), + }) + + ids = append(ids, spawnedIds...) + } + + return ids, nil +} diff --git a/internal/features/stacks/jobs/component_sources.go b/internal/features/stacks/jobs/component_sources.go deleted file mode 100644 index 7b27f6ba..00000000 --- a/internal/features/stacks/jobs/component_sources.go +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package jobs - -import ( - "context" - "path/filepath" - - "github.com/hashicorp/terraform-ls/internal/document" - "github.com/hashicorp/terraform-ls/internal/eventbus" - "github.com/hashicorp/terraform-ls/internal/features/stacks/state" - "github.com/hashicorp/terraform-ls/internal/job" - "github.com/hashicorp/terraform-ls/internal/lsp" - tfaddr "github.com/hashicorp/terraform-registry-address" - "github.com/hashicorp/terraform-schema/module" -) - -// LoadStackComponentSources will trigger parsing the local terraform modules for a stack in the ModulesFeature -func LoadStackComponentSources(ctx context.Context, stackStore *state.StackStore, bus *eventbus.EventBus, stackPath string) (job.IDs, error) { - ids := make(job.IDs, 0) - - record, err := stackStore.StackRecordByPath(stackPath) - if err != nil { - return ids, err - } - - // iterate over each component in the stack and find local terraform modules - for _, component := range record.Meta.Components { - if component.Source == "" { - // no source recorded, skip - continue - } - - var fullPath string - // detect if component.Source is a local module - switch component.SourceAddr.(type) { - case module.LocalSourceAddr: - fullPath = filepath.Join(stackPath, filepath.FromSlash(component.Source)) - case tfaddr.Module: - continue - case module.RemoteSourceAddr: - continue - default: - // Unknown source address, we can't resolve the path - continue - } - - if fullPath == "" { - // Unknown source address, we can't resolve the path - continue - } - - dh := document.DirHandleFromPath(fullPath) - - // notify the event bus that a new Component with a - // local modules has been opened - spawnedIds := bus.DidOpen(eventbus.DidOpenEvent{ - Context: ctx, - Dir: dh, - LanguageID: lsp.Terraform.String(), - }) - - ids = append(ids, spawnedIds...) - } - - return ids, nil -} diff --git a/internal/terraform/module/operation/op_type_string.go b/internal/terraform/module/operation/op_type_string.go index 67090f50..705f0a96 100644 --- a/internal/terraform/module/operation/op_type_string.go +++ b/internal/terraform/module/operation/op_type_string.go @@ -29,13 +29,12 @@ func _() { _ = x[OpTypeTerraformValidate-18] _ = x[OpTypeParseStackConfiguration-19] _ = x[OpTypeLoadStackMetadata-20] - _ = x[OpTypeLoadStackComponentSources-21] - _ = x[OpTypeLoadStackRequiredTerraformVersion-22] + _ = x[OpTypeLoadStackRequiredTerraformVersion-21] } -const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeGetInstalledTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeStacksPreloadEmbeddedSchemaOpTypeSchemaModuleValidationOpTypeSchemaVarsValidationOpTypeReferenceValidationOpTypeTerraformValidateOpTypeParseStackConfigurationOpTypeLoadStackMetadataOpTypeLoadStackComponentSourcesOpTypeLoadStackRequiredTerraformVersion" +const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeGetInstalledTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeStacksPreloadEmbeddedSchemaOpTypeSchemaModuleValidationOpTypeSchemaVarsValidationOpTypeReferenceValidationOpTypeTerraformValidateOpTypeParseStackConfigurationOpTypeLoadStackMetadataOpTypeLoadStackRequiredTerraformVersion" -var _OpType_index = [...]uint16{0, 13, 38, 72, 90, 120, 140, 165, 189, 217, 245, 271, 302, 329, 356, 389, 417, 443, 468, 491, 520, 543, 574, 613} +var _OpType_index = [...]uint16{0, 13, 38, 72, 90, 120, 140, 165, 189, 217, 245, 271, 302, 329, 356, 389, 417, 443, 468, 491, 520, 543, 582} func (i OpType) String() string { if i >= OpType(len(_OpType_index)-1) { diff --git a/internal/terraform/module/operation/operation.go b/internal/terraform/module/operation/operation.go index 34cfcaa4..1043cdc9 100644 --- a/internal/terraform/module/operation/operation.go +++ b/internal/terraform/module/operation/operation.go @@ -38,6 +38,5 @@ const ( OpTypeTerraformValidate OpTypeParseStackConfiguration OpTypeLoadStackMetadata - OpTypeLoadStackComponentSources OpTypeLoadStackRequiredTerraformVersion )