mirror of
https://github.com/zitadel/zitadel.git
synced 2025-08-11 20:47:32 +00:00
fix: correct handling of removed targets (#9824)
# Which Problems Are Solved
In Actions v2, if a target is removed, which is still used in an
execution, the target is still listed when list executions.
# How the Problems Are Solved
Removed targets are now also removed from the executions.
# Additional Changes
To be sure the list executions include a check if the target is still
existing.
# Additional Context
None
Co-authored-by: Livio Spring <livio.a@gmail.com>
(cherry picked from commit a05f7ce3fc
)
This commit is contained in:

committed by
Livio Spring

parent
8bd7414241
commit
bd75b8c9bd
@@ -1,11 +1,13 @@
|
||||
package query
|
||||
|
||||
import (
|
||||
"cmp"
|
||||
"context"
|
||||
"database/sql"
|
||||
_ "embed"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"slices"
|
||||
"time"
|
||||
|
||||
sq "github.com/Masterminds/squirrel"
|
||||
@@ -301,13 +303,15 @@ func executionTargetsUnmarshal(data []byte) ([]*exec.Target, error) {
|
||||
}
|
||||
|
||||
targets := make([]*exec.Target, len(executionTargets))
|
||||
// position starts with 1
|
||||
for _, item := range executionTargets {
|
||||
slices.SortFunc(executionTargets, func(a, b *executionTarget) int {
|
||||
return cmp.Compare(a.Position, b.Position)
|
||||
})
|
||||
for i, item := range executionTargets {
|
||||
if item.Target != "" {
|
||||
targets[item.Position-1] = &exec.Target{Type: domain.ExecutionTargetTypeTarget, Target: item.Target}
|
||||
targets[i] = &exec.Target{Type: domain.ExecutionTargetTypeTarget, Target: item.Target}
|
||||
}
|
||||
if item.Include != "" {
|
||||
targets[item.Position-1] = &exec.Target{Type: domain.ExecutionTargetTypeInclude, Target: item.Include}
|
||||
targets[i] = &exec.Target{Type: domain.ExecutionTargetTypeInclude, Target: item.Include}
|
||||
}
|
||||
}
|
||||
return targets, nil
|
||||
|
@@ -1,11 +1,15 @@
|
||||
SELECT instance_id,
|
||||
execution_id,
|
||||
SELECT et.instance_id,
|
||||
et.execution_id,
|
||||
JSONB_AGG(
|
||||
JSON_OBJECT(
|
||||
'position' : position,
|
||||
'include' : include,
|
||||
'target' : target_id
|
||||
)
|
||||
) as targets
|
||||
FROM projections.executions1_targets
|
||||
GROUP BY instance_id, execution_id
|
||||
'position' : et.position,
|
||||
'include' : et.include,
|
||||
'target' : et.target_id
|
||||
)
|
||||
) as targets
|
||||
FROM projections.executions1_targets AS et
|
||||
INNER JOIN projections.targets2 AS t
|
||||
ON et.instance_id = t.instance_id
|
||||
AND et.target_id IS NOT NULL
|
||||
AND et.target_id = t.id
|
||||
GROUP BY et.instance_id, et.execution_id
|
@@ -22,9 +22,10 @@ var (
|
||||
` COUNT(*) OVER ()` +
|
||||
` FROM projections.executions1` +
|
||||
` JOIN (` +
|
||||
`SELECT instance_id, execution_id, JSONB_AGG( JSON_OBJECT( 'position' : position, 'include' : include, 'target' : target_id ) ) as targets` +
|
||||
` FROM projections.executions1_targets` +
|
||||
` GROUP BY instance_id, execution_id` +
|
||||
`SELECT et.instance_id, et.execution_id, JSONB_AGG( JSON_OBJECT( 'position' : et.position, 'include' : et.include, 'target' : et.target_id ) ) as targets` +
|
||||
` FROM projections.executions1_targets AS et` +
|
||||
` INNER JOIN projections.targets2 AS t ON et.instance_id = t.instance_id AND et.target_id IS NOT NULL AND et.target_id = t.id` +
|
||||
` GROUP BY et.instance_id, et.execution_id` +
|
||||
`)` +
|
||||
` AS execution_targets` +
|
||||
` ON execution_targets.instance_id = projections.executions1.instance_id` +
|
||||
@@ -45,9 +46,10 @@ var (
|
||||
` execution_targets.targets` +
|
||||
` FROM projections.executions1` +
|
||||
` JOIN (` +
|
||||
`SELECT instance_id, execution_id, JSONB_AGG( JSON_OBJECT( 'position' : position, 'include' : include, 'target' : target_id ) ) as targets` +
|
||||
` FROM projections.executions1_targets` +
|
||||
` GROUP BY instance_id, execution_id` +
|
||||
`SELECT et.instance_id, et.execution_id, JSONB_AGG( JSON_OBJECT( 'position' : et.position, 'include' : et.include, 'target' : et.target_id ) ) as targets` +
|
||||
` FROM projections.executions1_targets AS et` +
|
||||
` INNER JOIN projections.targets2 AS t ON et.instance_id = t.instance_id AND et.target_id IS NOT NULL AND et.target_id = t.id` +
|
||||
` GROUP BY et.instance_id, et.execution_id` +
|
||||
`)` +
|
||||
` AS execution_targets` +
|
||||
` ON execution_targets.instance_id = projections.executions1.instance_id` +
|
||||
@@ -179,6 +181,63 @@ func Test_ExecutionPrepares(t *testing.T) {
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "prepareExecutionsQuery multiple result, removed target, position missing",
|
||||
prepare: prepareExecutionsQuery,
|
||||
want: want{
|
||||
sqlExpectations: mockQueries(
|
||||
regexp.QuoteMeta(prepareExecutionsStmt),
|
||||
prepareExecutionsCols,
|
||||
[][]driver.Value{
|
||||
{
|
||||
"ro",
|
||||
"id-1",
|
||||
testNow,
|
||||
testNow,
|
||||
[]byte(`[{"position" : 1, "target" : "target"}, {"position" : 3, "include" : "include"}]`),
|
||||
},
|
||||
{
|
||||
"ro",
|
||||
"id-2",
|
||||
testNow,
|
||||
testNow,
|
||||
[]byte(`[{"position" : 2, "target" : "target"}, {"position" : 1, "include" : "include"}]`),
|
||||
},
|
||||
},
|
||||
),
|
||||
},
|
||||
object: &Executions{
|
||||
SearchResponse: SearchResponse{
|
||||
Count: 2,
|
||||
},
|
||||
Executions: []*Execution{
|
||||
{
|
||||
ObjectDetails: domain.ObjectDetails{
|
||||
ID: "id-1",
|
||||
EventDate: testNow,
|
||||
CreationDate: testNow,
|
||||
ResourceOwner: "ro",
|
||||
},
|
||||
Targets: []*exec.Target{
|
||||
{Type: domain.ExecutionTargetTypeTarget, Target: "target"},
|
||||
{Type: domain.ExecutionTargetTypeInclude, Target: "include"},
|
||||
},
|
||||
},
|
||||
{
|
||||
ObjectDetails: domain.ObjectDetails{
|
||||
ID: "id-2",
|
||||
EventDate: testNow,
|
||||
CreationDate: testNow,
|
||||
ResourceOwner: "ro",
|
||||
},
|
||||
Targets: []*exec.Target{
|
||||
{Type: domain.ExecutionTargetTypeInclude, Target: "include"},
|
||||
{Type: domain.ExecutionTargetTypeTarget, Target: "target"},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "prepareExecutionsQuery sql err",
|
||||
prepare: prepareExecutionsQuery,
|
||||
|
@@ -9,6 +9,7 @@ import (
|
||||
"github.com/zitadel/zitadel/internal/eventstore/handler/v2"
|
||||
exec "github.com/zitadel/zitadel/internal/repository/execution"
|
||||
"github.com/zitadel/zitadel/internal/repository/instance"
|
||||
"github.com/zitadel/zitadel/internal/repository/target"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -78,6 +79,15 @@ func (p *executionProjection) Reducers() []handler.AggregateReducer {
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
Aggregate: target.AggregateType,
|
||||
EventReducers: []handler.EventReducer{
|
||||
{
|
||||
Event: target.RemovedEventType,
|
||||
Reduce: p.reduceTargetRemoved,
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
Aggregate: instance.AggregateType,
|
||||
EventReducers: []handler.EventReducer{
|
||||
@@ -152,6 +162,21 @@ func (p *executionProjection) reduceExecutionSet(event eventstore.Event) (*handl
|
||||
return handler.NewMultiStatement(e, stmts...), nil
|
||||
}
|
||||
|
||||
func (p *executionProjection) reduceTargetRemoved(event eventstore.Event) (*handler.Statement, error) {
|
||||
e, err := assertEvent[*target.RemovedEvent](event)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return handler.NewDeleteStatement(
|
||||
e,
|
||||
[]handler.Condition{
|
||||
handler.NewCond(ExecutionTargetInstanceIDCol, e.Aggregate().InstanceID),
|
||||
handler.NewCond(ExecutionTargetTargetIDCol, e.Aggregate().ID),
|
||||
},
|
||||
handler.WithTableSuffix(ExecutionTargetSuffix),
|
||||
), nil
|
||||
}
|
||||
|
||||
func (p *executionProjection) reduceExecutionRemoved(event eventstore.Event) (*handler.Statement, error) {
|
||||
e, err := assertEvent[*exec.RemovedEvent](event)
|
||||
if err != nil {
|
||||
|
@@ -7,6 +7,7 @@ import (
|
||||
"github.com/zitadel/zitadel/internal/eventstore/handler/v2"
|
||||
exec "github.com/zitadel/zitadel/internal/repository/execution"
|
||||
"github.com/zitadel/zitadel/internal/repository/instance"
|
||||
"github.com/zitadel/zitadel/internal/repository/target"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
)
|
||||
|
||||
@@ -79,6 +80,35 @@ func TestExecutionProjection_reduces(t *testing.T) {
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "reduceTargetRemoved",
|
||||
args: args{
|
||||
event: getEvent(
|
||||
testEvent(
|
||||
target.RemovedEventType,
|
||||
target.AggregateType,
|
||||
[]byte(`{}`),
|
||||
),
|
||||
eventstore.GenericEventMapper[target.RemovedEvent],
|
||||
),
|
||||
},
|
||||
reduce: (&executionProjection{}).reduceTargetRemoved,
|
||||
want: wantReduce{
|
||||
aggregateType: eventstore.AggregateType("target"),
|
||||
sequence: 15,
|
||||
executer: &testExecuter{
|
||||
executions: []execution{
|
||||
{
|
||||
expectedStmt: "DELETE FROM projections.executions1_targets WHERE (instance_id = $1) AND (target_id = $2)",
|
||||
expectedArgs: []interface{}{
|
||||
"instance-id",
|
||||
"agg-id",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "reduceExecutionRemoved",
|
||||
args: args{
|
||||
|
Reference in New Issue
Block a user