Skip to content

Commit

Permalink
Refactor diamond detection code
Browse files Browse the repository at this point in the history
  • Loading branch information
FelonEkonom committed Nov 26, 2024
1 parent a3c6bdd commit 9855e8a
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 48 deletions.
26 changes: 15 additions & 11 deletions lib/membrane/core/element/diamond_detection_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ defmodule Membrane.Core.Element.DiamondDetectionController do
require Membrane.Pad, as: Pad

alias __MODULE__.{DiamondLogger, PathInGraph}
alias __MODULE__.PathInGraph.Vertex
alias Membrane.Core.Element.State
alias Membrane.Element.PadData

@component_path_suffix "__membrane_component_path_64_byte_suffix________________________"
@component_path_prefix "__membrane_component_path_64_byte_prefix________________________"

@spec start_diamond_detection(State.t()) :: :ok
def start_diamond_detection(state) do
Expand Down Expand Up @@ -61,11 +62,14 @@ defmodule Membrane.Core.Element.DiamondDetectionController do
state

true ->
old_diamond_detection_path =
state.diamond_detection_ref_to_path[diamond_detection_ref]
|> remove_component_path_prefix()

:ok =
DiamondLogger.log_diamond(
diamond_detecton_path |> cut_suffixes_in_path(),
state.diamond_detection_ref_to_path[diamond_detection_ref] |> cut_suffixes_in_path()
)
diamond_detecton_path
|> remove_component_path_prefix()
|> DiamondLogger.log_diamond(old_diamond_detection_path)

state
end
Expand Down Expand Up @@ -186,18 +190,18 @@ defmodule Membrane.Core.Element.DiamondDetectionController do
end

defp get_component_path() do
# adding @component_path_suffix to component path will cause that component path will
# adding @component_path_prefix to component path will cause that component path will
# always have more than 64 bytes, so it won't be copied during sending a message
(Membrane.ComponentPath.get() ++ [@component_path_suffix])
[@component_path_prefix | Membrane.ComponentPath.get()]
|> Enum.join()
end

defp have_common_prefix?(path_a, path_b), do: List.last(path_a) == List.last(path_b)

defp cut_suffixes_in_path(path_in_graph) do
defp remove_component_path_prefix(path_in_graph) do
path_in_graph
|> Enum.map(
&%{&1 | component_path: String.replace(&1.component_path, @component_path_suffix, "")}
)
|> Enum.map(fn %Vertex{component_path: @component_path_prefix <> component_path} = vertex ->
%{vertex | component_path: component_path}
end)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ defmodule Membrane.Core.Element.DiamondDetectionController.DiamondLogger do
alias Membrane.Core.Element.DiamondDetectionController.PathInGraph
alias Membrane.Core.Element.DiamondDetectionController.PathInGraph.Vertex

# logging a diamond is moved to the separate module due to testing

@spec log_diamond(PathInGraph.t(), PathInGraph.t()) :: :ok
def log_diamond(path_a, path_b) do
Membrane.Logger.debug("""
Expand Down

This file was deleted.

3 changes: 2 additions & 1 deletion test/membrane/integration/diamond_detection_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ defmodule Membrane.Integration.DiamondDetectionTest do
def handle_demand(_pad, _demand, _unit, _ctx, state), do: {[], state}
end

@tag :xd
test "logging a diamond" do
with_mock DiamondLogger, log_diamond: fn _path_a, _path_b -> :ok end do
# a -> b -> c and a -> c
Expand Down Expand Up @@ -112,7 +113,7 @@ defmodule Membrane.Integration.DiamondDetectionTest do
Process.sleep(1500)

assert [_old_entry | new_entries] = call_history(DiamondLogger)
assert new_entries != []
assert length(new_entries) in [1, 2]

new_entries
|> Enum.flat_map(fn {_element_pid, {DiamondLogger, :log_diamond, [path_a, path_b]}, :ok} ->
Expand Down

0 comments on commit 9855e8a

Please sign in to comment.