Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow rollup on deregistered aggregates #1551

Merged
merged 3 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions lib/trento/domain/cluster/cluster.ex
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,16 @@ defmodule Trento.Domain.Cluster do
|> maybe_update_cluster(command)
end

def execute(
%Cluster{cluster_id: cluster_id} = snapshot,
%RollUpCluster{}
) do
%ClusterRollUpRequested{
cluster_id: cluster_id,
snapshot: snapshot
}
end

def execute(%Cluster{deregistered_at: deregistered_at}, _) when not is_nil(deregistered_at),
do: {:error, :cluster_not_registered}

Expand Down Expand Up @@ -311,16 +321,6 @@ defmodule Trento.Domain.Cluster do
|> Multi.execute(&maybe_emit_cluster_health_changed_event/1)
end

def execute(
%Cluster{cluster_id: cluster_id} = snapshot,
%RollUpCluster{}
) do
%ClusterRollUpRequested{
cluster_id: cluster_id,
snapshot: snapshot
}
end

def execute(
%Cluster{cluster_id: cluster_id} = cluster,
%DeregisterClusterHost{
Expand Down
20 changes: 10 additions & 10 deletions lib/trento/domain/host/host.ex
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,16 @@ defmodule Trento.Domain.Host do
]
end

def execute(
%Host{host_id: host_id} = snapshot,
%RollUpHost{}
) do
%HostRollUpRequested{
host_id: host_id,
snapshot: snapshot
}
end

def execute(
%Host{deregistered_at: deregistered_at},
_
Expand Down Expand Up @@ -315,16 +325,6 @@ defmodule Trento.Domain.Host do
}
end

def execute(
%Host{host_id: host_id} = snapshot,
%RollUpHost{}
) do
%HostRollUpRequested{
host_id: host_id,
snapshot: snapshot
}
end

def execute(
%Host{host_id: host_id},
%RequestHostDeregistration{requested_at: requested_at}
Expand Down
16 changes: 8 additions & 8 deletions lib/trento/domain/sap_system/sap_system.ex
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,6 @@ defmodule Trento.Domain.SapSystem do
|> Multi.execute(&maybe_emit_sap_system_tombstoned_event/1)
end

def execute(
%SapSystem{deregistered_at: deregistered_at},
_
)
when not is_nil(deregistered_at) do
{:error, :sap_system_not_registered}
end

def execute(
%SapSystem{sap_system_id: sap_system_id} = snapshot,
%RollUpSapSystem{}
Expand All @@ -295,6 +287,14 @@ defmodule Trento.Domain.SapSystem do
}
end

def execute(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this does nothing after all 😆
We don't have any command afterward.
Anyway, leave it, maybe we need it in the future

%SapSystem{deregistered_at: deregistered_at},
_
)
when not is_nil(deregistered_at) do
{:error, :sap_system_not_registered}
end

def apply(
%SapSystem{sap_system_id: nil},
%DatabaseRegistered{
Expand Down
10 changes: 9 additions & 1 deletion test/trento/domain/cluster/cluster_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,6 @@ defmodule Trento.ClusterTest do
commands_to_reject = [
%CompleteChecksExecution{},
%DeregisterClusterHost{},
%RollUpCluster{},
%SelectChecks{},
%RegisterClusterHost{}
]
Expand All @@ -923,6 +922,15 @@ defmodule Trento.ClusterTest do
assert match?({:error, :cluster_not_registered}, aggregate_run(initial_events, command)),
"Command #{inspect(command)} should be rejected by the aggregate"
end

commands_to_accept = [
%RollUpCluster{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we could include the RegisterClusterHost command as well, for completeness

]

for command <- commands_to_accept do
assert match?({:ok, _, _}, aggregate_run(initial_events, command)),
"Command #{inspect(command)} should be accepted by a deregistered cluster"
end
end

test "should emit the HostRemovedFromCluster event after a DeregisterClusterHost command and remove the host from the cluster aggregate state" do
Expand Down
10 changes: 9 additions & 1 deletion test/trento/domain/host/host_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,6 @@ defmodule Trento.HostTest do
commands_to_reject = [
%DeregisterHost{},
%RequestHostDeregistration{},
%RollUpHost{},
%UpdateHeartbeat{},
%UpdateProvider{},
%UpdateSlesSubscriptions{}
Expand All @@ -753,6 +752,15 @@ defmodule Trento.HostTest do
for command <- commands_to_reject do
assert_error(initial_events, command, {:error, :host_not_registered})
end

commands_to_accept = [
%RollUpHost{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same. We could include the RegisterHost

]

for command <- commands_to_accept do
assert match?({:ok, _, _}, aggregate_run(initial_events, command)),
"Command #{inspect(command)} should be accepted by a deregistered host"
end
end

test "should emit the HostDeregistered and HostTombstoned events" do
Expand Down
15 changes: 2 additions & 13 deletions test/trento/domain/sap_system/sap_system_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1794,25 +1794,14 @@ defmodule Trento.SapSystemTest do
build(:register_database_instance_command),
build(:register_application_instance_command),
build(:deregister_database_instance_command, sap_system_id: sap_system_id),
build(:deregister_application_instance_command, sap_system_id: sap_system_id)
build(:deregister_application_instance_command, sap_system_id: sap_system_id),
build(:rollup_sap_system_command)
]

for command <- commands_to_accept do
assert match?({:ok, _, _}, aggregate_run(initial_events, command)),
"Command #{inspect(command)} should be accepted by a deregistered SAP system"
end

commands_to_reject = [
build(:rollup_sap_system_command)
]

for command <- commands_to_reject do
assert match?(
{:error, :sap_system_not_registered},
aggregate_run(initial_events, command)
),
"Command #{inspect(command)} should be rejected by a deregistered SAP system"
end
end

test "should deregister a Database and SAP system when the Primary database instance is removed" do
Expand Down