Skip to content

Commit

Permalink
[fix] Check if DefaultConfig is nil before merging (#13988)
Browse files Browse the repository at this point in the history
  • Loading branch information
cedric-cordenier authored Aug 1, 2024
1 parent 1b4cb83 commit 0e8f2de
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 1 deletion.
4 changes: 4 additions & 0 deletions core/services/workflows/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,10 @@ func (e *Engine) configForStep(ctx context.Context, executionID string, step *st
return step.config, nil
}

if capConfig.DefaultConfig == nil {
return step.config, nil
}

// Merge the configs for now; note that this means that a workflow can override
// all of the config set by the capability. This is probably not desirable in
// the long-term, but we don't know much about those use cases so stick to a simpler
Expand Down
53 changes: 52 additions & 1 deletion core/services/workflows/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (t testConfigProvider) ConfigForCapability(ctx context.Context, capabilityI
return t.configForCapability(ctx, capabilityID, donID)
}

return capabilities.CapabilityConfiguration{DefaultConfig: values.EmptyMap()}, nil
return capabilities.CapabilityConfiguration{}, nil
}

// newTestEngine creates a new engine with some test defaults.
Expand Down Expand Up @@ -1063,3 +1063,54 @@ func TestEngine_MergesWorkflowConfigAndCRConfig(t *testing.T) {
assert.Equal(t, m.(map[string]any)["deltaStage"], "1s")
assert.Equal(t, m.(map[string]any)["schedule"], "allAtOnce")
}

func TestEngine_HandlesNilConfigOnchain(t *testing.T) {
ctx := testutils.Context(t)
reg := coreCap.NewRegistry(logger.TestLogger(t))

trigger, _ := mockTrigger(t)

require.NoError(t, reg.Add(ctx, trigger))
require.NoError(t, reg.Add(ctx, mockConsensus()))
writeID := "[email protected]"

gotConfig := values.EmptyMap()
target := newMockCapability(
// Create a remote capability so we don't use the local transmission protocol.
capabilities.MustNewRemoteCapabilityInfo(
writeID,
capabilities.CapabilityTypeTarget,
"a write capability targeting polygon testnet",
&capabilities.DON{ID: 1},
),
func(req capabilities.CapabilityRequest) (capabilities.CapabilityResponse, error) {
gotConfig = req.Config

return capabilities.CapabilityResponse{
Value: req.Inputs,
}, nil
},
)
require.NoError(t, reg.Add(ctx, target))

eng, testHooks := newTestEngine(
t,
reg,
simpleWorkflow,
)
reg.SetLocalRegistry(testConfigProvider{})

servicetest.Run(t, eng)

eid := getExecutionId(t, eng, testHooks)

state, err := eng.executionStates.Get(ctx, eid)
require.NoError(t, err)

assert.Equal(t, state.Status, store.StatusCompleted)

m, err := values.Unwrap(gotConfig)
require.NoError(t, err)
// The write target config contains three keys
assert.Len(t, m.(map[string]any), 3)
}

0 comments on commit 0e8f2de

Please sign in to comment.