Skip to content

Commit

Permalink
refactor: apply review suggestions, better error handling, better naming
Browse files Browse the repository at this point in the history
  • Loading branch information
ansgarm committed Jul 8, 2024
1 parent f29d0f3 commit cb7f0ff
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 35 deletions.
4 changes: 2 additions & 2 deletions internal/features/stacks/decoder/path_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (pr *PathReader) PathContext(path lang.Path) (*decoder.PathContext, error)

func stackPathContext(record *state.StackRecord) (*decoder.PathContext, error) {
// TODO: this should only work for terraform 1.8 and above
version := record.TerraformVersion
version := record.RequiredTerraformVersion
if version == nil {
version = stackschema.LatestAvailableVersion
}
Expand Down Expand Up @@ -75,7 +75,7 @@ func stackPathContext(record *state.StackRecord) (*decoder.PathContext, error) {

func deployPathContext(record *state.StackRecord) (*decoder.PathContext, error) {
// TODO: this should only work for terraform 1.8 and above
version := record.TerraformVersion
version := record.RequiredTerraformVersion
if version == nil {
version = stackschema.LatestAvailableVersion
}
Expand Down
2 changes: 1 addition & 1 deletion internal/features/stacks/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (f *StacksFeature) didOpen(ctx context.Context, dir document.DirHandle, lan
Func: func(ctx context.Context) error {
return jobs.LoadTerraformVersion(ctx, f.fs, f.store, path)
},
Type: operation.OpTypeLoadTerraformVersion.String(),
Type: operation.OpTypeLoadStackRequiredTerraformVersion.String(),
})
if err != nil {
return ids, err
Expand Down
23 changes: 16 additions & 7 deletions internal/features/stacks/jobs/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,36 @@ func LoadTerraformVersion(ctx context.Context, fs ReadOnlyFS, stackStore *state.
}

// Avoid parsing if it is already in progress or already known
if stackRecord.TerraformVersionState != operation.OpStateUnknown && !job.IgnoreState(ctx) {
if stackRecord.RequiredTerraformVersionState != operation.OpStateUnknown && !job.IgnoreState(ctx) {
return job.StateNotChangedErr{Dir: document.DirHandleFromPath(stackPath)}
}

stackStore.SetTerraformVersionState(stackPath, operation.OpStateLoading)
err = stackStore.SetTerraformVersionState(stackPath, operation.OpStateLoading)
if err != nil {
return err
}

// read version file
v, err := fs.ReadFile(filepath.Join(stackPath, ".terraform-version"))
if err != nil {
stackStore.SetTerraformVersionError(stackPath, err)
updateErr := stackStore.SetTerraformVersionError(stackPath, err)
if updateErr != nil {
return updateErr
}

return err
}

// parse version
version, err := version.NewVersion(strings.TrimSpace(string(v)))
if err != nil {
stackStore.SetTerraformVersionError(stackPath, err)
updateErr := stackStore.SetTerraformVersionError(stackPath, err)
if updateErr != nil {
return updateErr
}

return err
}

stackStore.SetTerraformVersion(stackPath, version)

return nil
return stackStore.SetTerraformVersion(stackPath, version)
}
22 changes: 11 additions & 11 deletions internal/features/stacks/jobs/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,16 @@ func TestLoadTerraformVersion(t *testing.T) {
t.Fatal(err)
}

if record.TerraformVersion.String() != "1.9.0" {
t.Fatalf("expected version 1.9.0, got %s", record.TerraformVersion.String())
if record.RequiredTerraformVersion.String() != "1.9.0" {
t.Fatalf("expected version 1.9.0, got %s", record.RequiredTerraformVersion.String())
}

if record.TerraformVersionState != operation.OpStateLoaded {
t.Fatalf("expected state %s, got %s", operation.OpStateLoaded, record.TerraformVersionState)
if record.RequiredTerraformVersionState != operation.OpStateLoaded {
t.Fatalf("expected state %s, got %s", operation.OpStateLoaded, record.RequiredTerraformVersionState)
}

if record.TerraformVersionErr != nil {
t.Fatalf("expected nil error, got %s", record.TerraformVersionErr)
if record.RequiredTerraformVersionErr != nil {
t.Fatalf("expected nil error, got %s", record.RequiredTerraformVersionErr)
}
}

Expand Down Expand Up @@ -116,15 +116,15 @@ func TestLoadTerraformVersion_invalid(t *testing.T) {
t.Fatal(err)
}

if record.TerraformVersion != nil {
t.Fatalf("expected nil version, got %s", record.TerraformVersion.String())
if record.RequiredTerraformVersion != nil {
t.Fatalf("expected nil version, got %s", record.RequiredTerraformVersion.String())
}

if record.TerraformVersionState != operation.OpStateLoaded {
t.Fatalf("expected state %s, got %s", operation.OpStateLoaded, record.TerraformVersionState)
if record.RequiredTerraformVersionState != operation.OpStateLoaded {
t.Fatalf("expected state %s, got %s", operation.OpStateLoaded, record.RequiredTerraformVersionState)
}

if record.TerraformVersionErr == nil {
if record.RequiredTerraformVersionErr == nil {
t.Fatal("expected error in record, got nil")
}
})
Expand Down
6 changes: 3 additions & 3 deletions internal/features/stacks/state/stack_record.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ type StackRecord struct {
Diagnostics ast.SourceDiagnostics
DiagnosticsState globalAst.DiagnosticSourceState

TerraformVersion *version.Version
TerraformVersionErr error
TerraformVersionState operation.OpState
RequiredTerraformVersion *version.Version
RequiredTerraformVersionErr error
RequiredTerraformVersionState operation.OpState
}

func (m *StackRecord) Path() string {
Expand Down
14 changes: 7 additions & 7 deletions internal/features/stacks/state/stack_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,9 @@ func (s *StackStore) setTerraformVersionWithChangeNotification(path string, vers
}
stack := oldStack.Copy()

stack.TerraformVersion = version
stack.TerraformVersionErr = vErr
stack.TerraformVersionState = state
stack.RequiredTerraformVersion = version
stack.RequiredTerraformVersionErr = vErr
stack.RequiredTerraformVersionState = state

err = txn.Insert(s.tableName, stack)
if err != nil {
Expand Down Expand Up @@ -248,7 +248,7 @@ func (s *StackStore) SetTerraformVersionState(path string, state operation.OpSta
return err
}

stack.TerraformVersionState = state
stack.RequiredTerraformVersionState = state
err = txn.Insert(s.tableName, stack)
if err != nil {
return err
Expand Down Expand Up @@ -312,19 +312,19 @@ func (s *StackStore) queueRecordChange(oldRecord, newRecord *StackRecord) error
switch {
// new record added
case oldRecord == nil && newRecord != nil:
if newRecord.TerraformVersion != nil {
if newRecord.RequiredTerraformVersion != nil {
changes.TerraformVersion = true
}
// record removed
case oldRecord != nil && newRecord == nil:
changes.IsRemoval = true

if oldRecord.TerraformVersion != nil {
if oldRecord.RequiredTerraformVersion != nil {
changes.TerraformVersion = true
}
// record changed
default:
if oldRecord.TerraformVersion == nil || !oldRecord.TerraformVersion.Equal(newRecord.TerraformVersion) {
if !oldRecord.RequiredTerraformVersion.Equal(newRecord.RequiredTerraformVersion) {
changes.TerraformVersion = true
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal/terraform/module/operation/op_type_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion internal/terraform/module/operation/operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ const (
OpTypeReferenceValidation
OpTypeTerraformValidate
OpTypeParseStackConfiguration
OpTypeLoadTerraformVersion
OpTypeLoadStackRequiredTerraformVersion
)

0 comments on commit cb7f0ff

Please sign in to comment.