fix(portal): update directory sync job w/ new hard delete (#10267)

Why:

* During the refactor to move to hard delete data in the portal there
were a couple places of inconsistency in the directory sync job where
deletion was concerned.
This commit is contained in:
Brian Manifold
2025-08-30 18:19:59 -07:00
committed by GitHub
parent fed53ee26f
commit 5797511ebd
7 changed files with 45 additions and 35 deletions

View File

@@ -15,7 +15,7 @@ defmodule Domain.Actors.Group.Sync do
with {:ok, groups} <- all_provider_groups(provider),
{:ok, {upsert, delete}} <- plan_groups_update(groups, provider_identifiers),
:ok <- deletion_circuit_breaker(groups, delete, provider),
{:ok, _num_deleted} <- delete_groups(provider, delete),
{:ok, deleted_count} <- delete_groups(provider, delete),
{:ok, upserted} <- upsert_groups(provider, attrs_by_provider_identifier, upsert) do
group_ids_by_provider_identifier =
for group <- groups ++ upserted,
@@ -28,7 +28,7 @@ defmodule Domain.Actors.Group.Sync do
%{
groups: groups,
plan: {upsert, delete},
deleted: delete,
deleted_count: deleted_count,
upserted: upserted,
group_ids_by_provider_identifier: group_ids_by_provider_identifier
}}

View File

@@ -23,13 +23,13 @@ defmodule Domain.Actors.Membership.Sync do
with {:ok, memberships} <- all_provider_memberships(provider),
{:ok, {insert, delete}} <- plan_memberships_update(tuples, memberships),
deleted_stats = delete_memberships(delete),
{:ok, deleted_count} = delete_memberships(delete),
{:ok, inserted} <- insert_memberships(provider, insert) do
{:ok,
%{
plan: {insert, delete},
inserted: inserted,
deleted_stats: deleted_stats
deleted_count: deleted_count
}}
end
end
@@ -63,8 +63,11 @@ defmodule Domain.Actors.Membership.Sync do
end
defp delete_memberships(provider_identifiers_to_delete) do
Membership.Query.by_group_id_and_actor_id({:in, provider_identifiers_to_delete})
|> Repo.delete_all()
{count, nil} =
Membership.Query.by_group_id_and_actor_id({:in, provider_identifiers_to_delete})
|> Repo.delete_all()
{:ok, count}
end
defp insert_memberships(provider, provider_identifiers_to_insert) do

View File

@@ -285,17 +285,17 @@ defmodule Domain.Auth.Adapter.OpenIDConnect.DirectorySync do
plan: {identities_insert_ids, identities_update_ids, identities_delete_ids},
inserted: identities_inserted,
updated: identities_updated,
deleted: identities_deleted
deleted_count: identities_deleted_count
},
%{
plan: {groups_upsert_ids, groups_delete_ids},
upserted: groups_upserted,
deleted: groups_deleted
deleted_count: groups_deleted_count
},
%{
plan: {memberships_insert_tuples, memberships_delete_tuples},
inserted: memberships_inserted,
deleted_stats: {deleted_memberships_count, _}
deleted_count: memberships_deleted_count
}
) do
time_taken = time_taken(start_time, finish_time)
@@ -308,17 +308,17 @@ defmodule Domain.Auth.Adapter.OpenIDConnect.DirectorySync do
plan_identities_delete: length(identities_delete_ids),
identities_inserted: length(identities_inserted),
identities_and_actors_updated: length(identities_updated),
identities_deleted: length(identities_deleted),
identities_deleted: identities_deleted_count,
# Groups
plan_groups_upsert: length(groups_upsert_ids),
plan_groups_delete: length(groups_delete_ids),
groups_upserted: length(groups_upserted),
groups_deleted: length(groups_deleted),
groups_deleted: groups_deleted_count,
# Memberships
plan_memberships_insert: length(memberships_insert_tuples),
plan_memberships_delete: length(memberships_delete_tuples),
memberships_inserted: length(memberships_inserted),
memberships_deleted: deleted_memberships_count
memberships_deleted: memberships_deleted_count
)
end

View File

@@ -15,7 +15,7 @@ defmodule Domain.Auth.Identity.Sync do
{:ok, {insert, update, delete}} <-
plan_identities_update(identities, provider_identifiers),
:ok <- deletion_circuit_breaker(identities, delete, provider),
{:ok, deleted} <- delete_identities(provider, delete),
{:ok, deleted_count} <- delete_identities(provider, delete),
{:ok, inserted} <-
insert_identities(provider, attrs_by_provider_identifier, insert),
{:ok, updated} <-
@@ -32,7 +32,7 @@ defmodule Domain.Auth.Identity.Sync do
%{
identities: identities,
plan: {insert, update, delete},
deleted: deleted,
deleted_count: deleted_count,
inserted: inserted,
updated: updated,
actor_ids_by_provider_identifier: actor_ids_by_provider_identifier

View File

@@ -0,0 +1,9 @@
defmodule Domain.Repo.Migrations.AddFlowsTokenIdIndex do
use Ecto.Migration
@disable_ddl_transaction true
def change do
create_if_not_exists(index(:flows, [:token_id]))
end
end

View File

@@ -711,7 +711,7 @@ defmodule Domain.ActorsTest do
assert {:ok,
%{
plan: {upsert, []},
deleted: [],
deleted_count: 0,
upserted: [_group1, _group2],
group_ids_by_provider_identifier: group_ids_by_provider_identifier
}} = sync_provider_groups(provider, attrs_list)
@@ -760,7 +760,7 @@ defmodule Domain.ActorsTest do
assert {:ok,
%{
plan: {upsert, []},
deleted: [],
deleted_count: 0,
upserted: [_group1, _group2],
group_ids_by_provider_identifier: group_ids_by_provider_identifier
}} = sync_provider_groups(provider, attrs_list)
@@ -832,21 +832,19 @@ defmodule Domain.ActorsTest do
%{"name" => "Group:Finance", "provider_identifier" => "G:GROUP_ID4"}
]
deleted_group_ids = [group1.provider_identifier, group2.provider_identifier]
assert {:ok,
%{
groups: [_group1, _group2, _group3, _group4, _group5],
plan: {_upsert, delete},
deleted: [deleted_group1, deleted_group2],
deleted_count: 2,
upserted: [_upserted_group3, _upserted_group4, _upserted_group5],
group_ids_by_provider_identifier: group_ids_by_provider_identifier
}} = sync_provider_groups(provider, attrs_list)
assert Enum.all?(["G:GROUP_ID1", "OU:OU_ID1"], &(&1 in delete))
assert deleted_group1 in deleted_group_ids
assert deleted_group2 in deleted_group_ids
assert Repo.aggregate(Actors.Group, :count) == 3
refute Repo.get(Domain.Actors.Group, group1.id)
refute Repo.get(Domain.Actors.Group, group2.id)
assert Map.keys(group_ids_by_provider_identifier) |> length() == 3
end
@@ -924,7 +922,7 @@ defmodule Domain.ActorsTest do
%{
groups: [],
plan: {[], []},
deleted: [],
deleted_count: 0,
upserted: [],
group_ids_by_provider_identifier: %{}
}}
@@ -1063,7 +1061,7 @@ defmodule Domain.ActorsTest do
assert {:ok,
%{
plan: {insert, []},
deleted_stats: {0, nil},
deleted_count: 0,
inserted: [_membership1, _membership2]
}} =
sync_provider_memberships(
@@ -1122,7 +1120,7 @@ defmodule Domain.ActorsTest do
assert {:ok,
%{
plan: {[], []},
deleted_stats: {0, nil},
deleted_count: 0,
inserted: []
}} =
sync_provider_memberships(
@@ -1171,7 +1169,7 @@ defmodule Domain.ActorsTest do
assert {:ok,
%{
plan: {[], delete},
deleted_stats: {2, nil},
deleted_count: 2,
inserted: []
}} =
sync_provider_memberships(
@@ -1225,7 +1223,7 @@ defmodule Domain.ActorsTest do
%{
plan: {[], delete},
inserted: [],
deleted_stats: {1, nil}
deleted_count: 1
}} =
sync_provider_memberships(
actor_ids_by_provider_identifier,
@@ -1265,7 +1263,7 @@ defmodule Domain.ActorsTest do
assert {:ok,
%{
plan: {[], []},
deleted_stats: {0, nil},
deleted_count: 0,
inserted: []
}} =
sync_provider_memberships(
@@ -1311,7 +1309,7 @@ defmodule Domain.ActorsTest do
assert {:ok,
%{
plan: {[], delete},
deleted_stats: {2, nil},
deleted_count: 2,
inserted: []
}} =
sync_provider_memberships(
@@ -1360,7 +1358,7 @@ defmodule Domain.ActorsTest do
assert {:ok,
%{
plan: {[], delete},
deleted_stats: {2, nil},
deleted_count: 2,
inserted: []
}} =
sync_provider_memberships(

View File

@@ -1655,7 +1655,7 @@ defmodule Domain.AuthTest do
plan: {insert, [], []},
inserted: [_actor1, _actor2],
updated: [],
deleted: 0,
deleted_count: 0,
actor_ids_by_provider_identifier: actor_ids_by_provider_identifier
}} = sync_provider_identities(provider, attrs_list)
@@ -1717,7 +1717,7 @@ defmodule Domain.AuthTest do
%{
identities: [_identity1, _identity2],
plan: {[], update, []},
deleted: 0,
deleted_count: 0,
updated: [_updated_identity1, _updated_identity2],
inserted: [],
actor_ids_by_provider_identifier: actor_ids_by_provider_identifier
@@ -1767,7 +1767,7 @@ defmodule Domain.AuthTest do
%{
identities: [fetched_identity],
plan: {[], ["USER_ID1"], []},
deleted: 0,
deleted_count: 0,
inserted: [],
actor_ids_by_provider_identifier: actor_ids_by_provider_identifier
}} = sync_provider_identities(provider, attrs_list)
@@ -1802,7 +1802,7 @@ defmodule Domain.AuthTest do
%{
identities: [fetched_identity],
plan: {[], [], []},
deleted: 0,
deleted_count: 0,
inserted: [],
actor_ids_by_provider_identifier: %{}
}} = sync_provider_identities(provider, attrs_list)
@@ -1949,7 +1949,7 @@ defmodule Domain.AuthTest do
%{
identities: [],
plan: {[], [], []},
deleted: 0,
deleted_count: 0,
updated: [],
inserted: [],
actor_ids_by_provider_identifier: %{}
@@ -2014,7 +2014,7 @@ defmodule Domain.AuthTest do
%{
identities: [_identity1, _identity2],
plan: {[], update, []},
deleted: 0,
deleted_count: 0,
inserted: [],
actor_ids_by_provider_identifier: actor_ids_by_provider_identifier
}} = sync_provider_identities(provider, attrs_list)