remove Child.ref (#564)

lib/membrane/bin.ex

@@ -274,7 +274,6 @@ defmodule Membrane.Bin do
   Options:
     - `:bring_spec?` - if true (default) imports and aliases `Membrane.ChildrenSpec`
     - `:bring_pad?` - if true (default) requires and aliases `Membrane.Pad`
-    - `:bring_child?` - if true (default) requires and aliases `Membrane.Child`
   """
   defmacro __using__(options) do
     bring_spec =
@@ -292,21 +291,13 @@ defmodule Membrane.Bin do
         end
       end
 
-    bring_child =
-      if Keyword.get(options, :bring_child?, true) do
-        quote do
-          require Membrane.Child, as: Child
-        end
-      end
-
     quote location: :keep do
       alias unquote(__MODULE__)
       @behaviour unquote(__MODULE__)
       @before_compile unquote(__MODULE__)
 
       unquote(bring_spec)
       unquote(bring_pad)
-      unquote(bring_child)
 
       import unquote(__MODULE__),
         only: [def_input_pad: 2, def_output_pad: 2, def_options: 1, def_clock: 0, def_clock: 1]

lib/membrane/bin/action.ex

@@ -44,12 +44,12 @@ defmodule Membrane.Bin.Action do
   Action that stops, unlinks and removes specified child/children from the bin.
 
   A child ref, list of children refs, children group id or a list of children group ids can be specified
-  as an argument. In case you need to refer to a single child from a children group, use `Membrane.Child.ref/2`.
+  as an argument.
   """
   @type remove_children ::
           {:remove_children,
-           Child.ref()
-           | [Child.ref()]
+           Child.name()
+           | [Child.name()]
            | Membrane.Child.group()
            | [Membrane.Child.group()]}
 

lib/membrane/child.ex

@@ -9,87 +9,8 @@ defmodule Membrane.Child do
 
   @type name :: Element.name() | Bin.name()
   @type group() :: any()
-  @type ref :: {Membrane.Child, group(), name()} | name()
 
   @type options :: Element.options() | Bin.options()
 
   defguard is_child_name?(arg) when is_element_name?(arg) or is_bin_name?(arg)
-
-  @type child_ref_options :: [group: group()]
-
-  @doc """
-  Returns a reference to a child.
-  """
-  defmacro ref(name) do
-    case __CALLER__.context do
-      :match -> ref_in_a_match(name)
-      _not_match -> ref_outside_a_match(name)
-    end
-  end
-
-  defmacro ref(name, options) do
-    case __CALLER__.context do
-      :match ->
-        Macro.expand(options, __CALLER__)
-        |> Access.fetch(:group)
-        |> case do
-          {:ok, group_ast} -> ref_in_a_match(name, group_ast)
-          :error -> raise "Improper options. The options must be in form of [group: group]."
-        end
-
-      _not_match ->
-        ref_outside_a_match(name, options)
-    end
-  end
-
-  defp ref_in_a_match(name) do
-    quote do
-      unquote(name)
-    end
-  end
-
-  defp ref_in_a_match(name, group) do
-    quote do
-      {
-        unquote(__MODULE__),
-        unquote(group),
-        unquote(name)
-      }
-    end
-  end
-
-  defp ref_outside_a_match(name) do
-    quote generated: true do
-      case unquote(name) do
-        {unquote(__MODULE__), _group, _child} ->
-          raise "Improper name: #{inspect(unquote(name))}. The name cannot match the reserved internal Membrane's pattern."
-
-        name ->
-          name
-      end
-    end
-  end
-
-  defp ref_outside_a_match(name, options) do
-    quote generated: true do
-      cond do
-        match?({unquote(__MODULE__), _group, _child}, unquote(name)) ->
-          raise "Improper name: #{inspect(unquote(name))}. The name cannot match the reserved internal Membrane's pattern."
-
-        not match?([group: _group], unquote(options)) ->
-          raise "Improper options: #{inspect(unquote(options))}. The options must be in form of [group: group]."
-
-        true ->
-          [group: group] = unquote(options)
-          {unquote(__MODULE__), group, unquote(name)}
-      end
-    end
-  end
-
-  @doc """
-  Returns a name of a child from its reference.
-  """
-  @spec name_by_ref(ref()) :: name()
-  def name_by_ref(ref(name, group: _group)), do: name
-  def name_by_ref(ref(name)), do: name
 end

lib/membrane/children_spec.ex

@@ -169,13 +169,6 @@ defmodule Membrane.ChildrenSpec do
   The children spawned within `links1` specification will be put inside `:first_children_group`, whereas the
   children spawned within `links2` specification will be put inside `second_children_group`.
 
-  In order to refer to a child which resides inside the children group, you need to use the `Membrane.Child.ref/2` function,
-  as in the example below:
-   ```
-    spec1 = {child(:source, Source), group: :first_group)
-    spec2 = get_child(Membrane.Child.ref(:source, group: :first_group)) |> child(:sink, Sink)
-   ```
-
   Later on, the children from a given group can be referred with their `group`, as in the example below:
   ```
     actions = [remove_children: :first_children_group]
@@ -262,8 +255,7 @@ defmodule Membrane.ChildrenSpec do
     @typep child_options_map :: %{get_if_exists: boolean}
 
     @type child_spec ::
-            {{:child_name, Child.name()} | {:child_ref, Child.name()},
-             Membrane.ChildrenSpec.child_definition(), child_options_map()}
+            {Child.name(), Membrane.ChildrenSpec.child_definition(), child_options_map()}
 
     @type status :: :from_pad | :to_pad | :done
 
@@ -348,21 +340,21 @@ defmodule Membrane.ChildrenSpec do
   See the _Children's specification_ section of the moduledoc for more information.
   """
   @spec get_child(builder(), Child.name()) :: builder()
-  def get_child(%Builder{} = builder, child_ref) do
-    do_get_child(builder, child_ref)
+  def get_child(%Builder{} = builder, child_name) do
+    do_get_child(builder, child_name)
   end
 
-  defp do_get_child(child_ref) do
-    %Builder{link_starting_child: {:child_ref, child_ref}}
+  defp do_get_child(child_name) do
+    %Builder{link_starting_child: child_name}
   end
 
-  defp do_get_child(builder, child_ref) do
+  defp do_get_child(builder, child_name) do
     if builder.status == :to_pad do
       builder
     else
       via_in(builder, :input)
     end
-    |> Builder.finish_link({:child_ref, child_ref})
+    |> Builder.finish_link(child_name)
   end
 
   @doc """
@@ -445,15 +437,13 @@ defmodule Membrane.ChildrenSpec do
   defp do_child(child_name, child_definition, opts) do
     ensure_is_child_definition!(child_definition)
     {:ok, opts} = Bunch.Config.parse(opts, @default_child_options)
-    child_name = {:child_name, child_name}
     child_spec = {child_name, child_definition, opts}
     %Builder{children: [child_spec], link_starting_child: child_name}
   end
 
   defp do_child(%Builder{} = builder, child_name, child_definition, opts) do
     ensure_is_child_definition!(child_definition)
     {:ok, opts} = Bunch.Config.parse(opts, @default_child_options)
-    child_name = {:child_name, child_name}
     child_spec = {child_name, child_definition, opts}
 
     if builder.status == :to_pad do

lib/membrane/core/bin.ex

@@ -262,9 +262,9 @@ defmodule Membrane.Core.Bin do
   end
 
   @impl GenServer
-  def handle_call(Message.new(:get_child_pid, child_ref), _from, state) do
+  def handle_call(Message.new(:get_child_pid, child_name), _from, state) do
     reply =
-      with %State{children: %{^child_ref => %{pid: child_pid}}} <- state do
+      with %State{children: %{^child_name => %{pid: child_pid}}} <- state do
         {:ok, child_pid}
       else
         _other -> {:error, :child_not_found}

lib/membrane/core/element.ex

@@ -151,7 +151,7 @@ defmodule Membrane.Core.Element do
   end
 
   @impl GenServer
-  def handle_call(Message.new(:get_child_pid, _child_ref), _from, state) do
+  def handle_call(Message.new(:get_child_pid, _child_name), _from, state) do
     {:reply, {:error, :element_cannot_have_children}, state}
   end
 

lib/membrane/core/parent/child_life_controller.ex

@@ -3,7 +3,7 @@ defmodule Membrane.Core.Parent.ChildLifeController do
   use Bunch
 
   alias __MODULE__.{CrashGroupUtils, LinkUtils, StartupUtils}
-  alias Membrane.ChildrenSpec
+  alias Membrane.{Child, ChildrenSpec}
   alias Membrane.Core.{Bin, CallbackHandler, Component, Parent, Pipeline}
 
   alias Membrane.Core.Parent.{
@@ -18,7 +18,6 @@ defmodule Membrane.Core.Parent.ChildLifeController do
   alias Membrane.Pad
   alias Membrane.ParentError
 
-  require Membrane.Child, as: Child
   require Membrane.Core.Component
   require Membrane.Core.Message, as: Message
   require Membrane.Logger
@@ -209,8 +208,6 @@ defmodule Membrane.Core.Parent.ChildLifeController do
     {:ok, options} =
       Bunch.Config.parse(options_keywords_list, @children_spec_options_fields_specs)
 
-    this_level_specs = equip_spec_with_children_refs(this_level_specs, options)
-
     options = Map.merge(defaults, options)
 
     options_to_pass_to_nested =
@@ -230,51 +227,10 @@ defmodule Membrane.Core.Parent.ChildLifeController do
   defp make_canonical(spec, defaults) do
     spec = Bunch.listify(spec)
     {:ok, options} = Bunch.Config.parse([], @children_spec_options_fields_specs)
-    spec = equip_spec_with_children_refs(spec, options)
     options = Map.merge(defaults, options)
     [{spec, options}]
   end
 
-  defp equip_spec_with_children_refs(specification_builders, options) do
-    Enum.map(specification_builders, fn specification_builder ->
-      children =
-        Enum.map(specification_builder.children, fn {child_name, child_definition, child_options} ->
-          child_ref = get_child_ref(child_name, options)
-          {child_ref, child_definition, child_options}
-        end)
-
-      links = Enum.map(specification_builder.links, &equip_link_with_child_ref(&1, options))
-      %{specification_builder | children: children, links: links}
-    end)
-  end
-
-  defp equip_link_with_child_ref(link, options) do
-    Bunch.Access.put_in(
-      link,
-      [:from],
-      get_child_ref(link.from, options)
-    )
-    |> Bunch.Access.put_in(
-      [:to],
-      get_child_ref(link.to, options)
-    )
-  end
-
-  defp get_child_ref(child_name_or_ref, options) do
-    case child_name_or_ref do
-      # child name created with child(...)
-      {:child_name, child_name} when is_map_key(options, :group) ->
-        Child.ref(child_name, group: options.group)
-
-      {:child_name, child_name} ->
-        Child.ref(child_name)
-
-      # child name created with get_child(...), bin_input() and bin_output()
-      {:child_ref, ref} ->
-        ref
-    end
-  end
-
   defp setup_children(
          {children_definitions_with_given_options, options},
          spec_ref,
@@ -494,7 +450,7 @@ defmodule Membrane.Core.Parent.ChildLifeController do
   end
 
   @spec handle_remove_children(
-          Child.ref() | [Child.ref()] | Child.group() | [Child.group()],
+          Child.name() | [Child.name()] | Child.group() | [Child.group()],
           Parent.state()
         ) :: Parent.state()
   def handle_remove_children(children_or_children_groups, state) do
@@ -504,8 +460,8 @@ defmodule Membrane.Core.Parent.ChildLifeController do
 
     refs =
       state.children
-      |> Enum.filter(fn {child_ref, child_entry} ->
-        child_ref in children_or_children_groups or
+      |> Enum.filter(fn {child_name, child_entry} ->
+        child_name in children_or_children_groups or
           child_entry.group in children_or_children_groups
       end)
       |> Enum.map(fn {ref, _child_entry} -> ref end)
@@ -549,9 +505,9 @@ defmodule Membrane.Core.Parent.ChildLifeController do
       [] ->
         :ok
 
-      children_refs ->
+      children_names ->
         raise Membrane.ParentError, """
-        Trying to remove children #{Enum.map_join(children_refs, ", ", &inspect/1)}, while such children or children groups do not exist.
+        Trying to remove children #{Enum.map_join(children_names, ", ", &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)}
         """

lib/membrane/core/parent/child_life_controller/startup_utils.ex

@@ -123,24 +123,17 @@ defmodule Membrane.Core.Parent.ChildLifeController.StartupUtils do
         ) :: :ok
   def check_if_children_names_and_children_groups_ids_are_unique(children_definitions, state) do
     state_children_groups =
-      Enum.map(state.children, fn {_child_ref, child_entry} -> child_entry.group end)
+      Enum.map(state.children, fn {_child_name, child_entry} -> child_entry.group end)
 
     new_children_groups =
       Enum.map(children_definitions, fn {_children, options} -> options.group end)
 
-    state_children_names =
-      Map.keys(state.children)
-      |> Enum.map(fn child_ref ->
-        case child_ref do
-          {Membrane.Child, _group, child_name} -> child_name
-          child_name -> child_name
-        end
-      end)
+    state_children_names = Map.keys(state.children)
 
     new_children_names =
-      Enum.flat_map(children_definitions, fn {children, _options} ->
-        get_children_names(children)
-      end)
+      children_definitions
+      |> Enum.flat_map(fn {children, _options} -> children end)
+      |> Enum.map(fn {child_name, _child_module, _options} -> child_name end)
 
     duplicated = Enum.filter(new_children_groups, &(&1 in state_children_names))
 
@@ -174,15 +167,6 @@ defmodule Membrane.Core.Parent.ChildLifeController.StartupUtils do
     :ok
   end
 
-  defp get_children_names(children) do
-    Enum.map(children, fn {child_ref, _child_module, _options} ->
-      case child_ref do
-        {Membrane.Child, _group, child_name} -> child_name
-        child_name -> child_name
-      end
-    end)
-  end
-
   defp start_child(child, node, parent_clock, syncs, log_metadata, supervisor, group) do
     %ChildEntry{name: name, module: module, options: options} = child
 

lib/membrane/core/pipeline.ex

@@ -136,9 +136,9 @@ defmodule Membrane.Core.Pipeline do
   end
 
   @impl GenServer
-  def handle_call(Message.new(:get_child_pid, child_ref), _from, state) do
+  def handle_call(Message.new(:get_child_pid, child_name), _from, state) do
     reply =
-      with %State{children: %{^child_ref => %{pid: child_pid}}} <- state do
+      with %State{children: %{^child_name => %{pid: child_pid}}} <- state do
         {:ok, child_pid}
       else
         _other -> {:error, :child_not_found}