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.

Sign up for GitHub

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

Jump to bottom

Raise on attempting to remove a not existing child #553

Merged
merged 7 commits into from
May 15, 2023
Merged

Raise on attempting to remove a not existing child #553

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
39281a4
Raise on attempting to remove a not existing child
FelonEkonom May 10, 2023
0b541e4
Fix bug in operating on list
FelonEkonom May 12, 2023
2137940
Move checking if removed child exists to the private function
FelonEkonom May 12, 2023
9804b49
Refctor error log in child life controller
FelonEkonom May 15, 2023
c951c4a
Merge branch 'master' into ensure-removed-child-exists
FelonEkonom May 15, 2023
036dfd3
Merge remote-tracking branch 'origin/master' into ensure-removed-chil…
FelonEkonom May 15, 2023
eb34480
Merge remote-tracking branch 'origin/master' into ensure-removed-chil…
FelonEkonom May 15, 2023
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
23 changes: 23 additions & 0 deletions lib/membrane/core/parent/child_life_controller.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,8 @@ defmodule Membrane.Core.Parent.ChildLifeController do
def handle_remove_children(children_or_children_groups, state) do
children_or_children_groups = Bunch.listify(children_or_children_groups)

:ok = ensure_removed_children_exist!(children_or_children_groups, state)

refs =
state.children
|> Enum.filter(fn {child_ref, child_entry} ->
Expand Down Expand Up @@ -534,6 +536,27 @@ defmodule Membrane.Core.Parent.ChildLifeController do
Parent.ChildrenModel.update_children!(state, refs, &%{&1 | terminating?: true})
end

defp ensure_removed_children_exist!(removed_children_or_groups, state) do
children_groups =
MapSet.new(state.children, fn {_ref, data} -> data.group end)
|> MapSet.delete(nil)

Enum.find(removed_children_or_groups, fn name ->
not Map.has_key?(state.children, name) and not MapSet.member?(children_groups, name)
end)
|> case do
nil ->
:ok

child_ref ->
raise Membrane.ParentError, """
Trying to remove child #{inspect(child_ref)}, while such a child or children group does not exist.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Enum.find(removed_children_or_groups, fn name ->
not Map.has_key?(state.children, name) and not MapSet.member?(children_groups, name)
end)
|> case do
nil ->
:ok
child_ref ->
raise Membrane.ParentError, """
Trying to remove child #{inspect(child_ref)}, while such a child or children group does not exist.
removed_children_or_groups
|> Enum.reject(fn name ->
Map.has_key?(state.children, name) or MapSet.member?(children_groups, name)
end)
|> case do
[] ->
:ok
children_refs ->
raise Membrane.ParentError, """
Trying to remove children #{Enum.map_join(", ", &inspect/1)}, while such children or children groups do not exist.

Existing children are: #{Map.keys(state.children) |> inspect(pretty: true)}
Existing children groups are: #{MapSet.to_list(children_groups) |> inspect(pretty: true)}
"""
end
end

@spec handle_remove_link(Child.name(), Pad.ref(), Parent.state()) ::
Parent.state()
def handle_remove_link(child_name, pad_ref, state) do
Expand Down