Skip to content

Commit

Permalink
Performance Improvements/query minimization
Browse files Browse the repository at this point in the history
1. IdeaIdeaLabel: Only create a new join table entry vs. processing lots
We don't end up using the idea in the code that calls
`IdeaLabels.add_idea_label_to_ideas` - we broadcast and load
all the brainstormings and lanes again. So, there's no need
to adjust the `Idea` struct to be fine. This saves us some
queries (I think) but at the very least some code and some
CPU cycles.

2.Do not trigger preloads for brainstorming when unneeded
  • Loading branch information
PragTob committed Dec 23, 2024
1 parent 54845af commit ce34983
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 46 deletions.
4 changes: 4 additions & 0 deletions lib/mindwendel/brainstormings.ex
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ defmodule Mindwendel.Brainstormings do
end
end

def get_bare_brainstorming!(id) do
Repo.get!(Brainstorming, id)
end

@doc """
Gets a single brainstorming with the admin url id
"""
Expand Down
9 changes: 9 additions & 0 deletions lib/mindwendel/brainstormings/idea_idea_label.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,13 @@ defmodule Mindwendel.Brainstormings.IdeaIdeaLabel do
name: :idea_idea_labels_idea_id_idea_label_id_index
)
end

def bare_creation_changeset(idea_idea_label \\ %__MODULE__{}, attrs) do
idea_idea_label
|> cast(attrs, [:idea_id, :idea_label_id])
|> validate_required([:idea_id, :idea_label_id])
|> unique_constraint([:idea_id, :idea_label_id],
name: :idea_idea_labels_idea_id_idea_label_id_index
)
end
end
15 changes: 5 additions & 10 deletions lib/mindwendel/idea_labels.ex
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,13 @@ defmodule Mindwendel.IdeaLabels do
nil
end

# As the broadcast results in a full reload of the ideas, we don't need to actually update
# the idea struct, a new association is enough
def add_idea_label_to_idea(%Idea{} = idea, %IdeaLabel{} = idea_label) do
idea = Repo.preload(idea, :idea_labels)

idea_labels =
(idea.idea_labels ++ [idea_label])
|> Enum.map(&Ecto.Changeset.change/1)

result =
idea
|> Ecto.Changeset.change()
|> Ecto.Changeset.put_assoc(:idea_labels, idea_labels)
|> Repo.update()
%{idea_id: idea.id, idea_label_id: idea_label.id}
|> IdeaIdeaLabel.bare_creation_changeset()
|> Repo.insert()

Lanes.broadcast_lanes_update(idea.brainstorming_id)
result
Expand Down
15 changes: 6 additions & 9 deletions lib/mindwendel/lanes.ex
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,7 @@ defmodule Mindwendel.Lanes do
def get_lanes_for_brainstorming_with_labels_filtered(id) do
{:ok, brainstorming} = Brainstormings.get_brainstorming(id)

filter_label =
if length(brainstorming.filter_labels_ids) > 0,
do: %{filter_labels_ids: brainstorming.filter_labels_ids},
else: %{}
filter_label = %{filter_labels_ids: brainstorming.filter_labels_ids}

get_lanes_for_brainstorming(id, filter_label)
end
Expand All @@ -89,7 +86,7 @@ defmodule Mindwendel.Lanes do
[%Lane{}, ...]
"""
def get_lanes_for_brainstorming(id, filters \\ %{}) do
def get_lanes_for_brainstorming(id, filters \\ %{filter_labels_ids: []}) do
lane_query =
from lane in Lane,
where: lane.brainstorming_id == ^id,
Expand All @@ -105,6 +102,10 @@ defmodule Mindwendel.Lanes do
|> Repo.preload(ideas: {ideas_advanced_query, [:link, :likes, :idea_labels, :files]})
end

defp build_ideas_query_with_filter(%{filter_labels_ids: []}) do
from(idea in Idea)
end

defp build_ideas_query_with_filter(%{filter_labels_ids: filter_labels_ids}) do
distinct_ideas =
from idea in Idea,
Expand All @@ -118,10 +119,6 @@ defmodule Mindwendel.Lanes do
)
end

defp build_ideas_query_with_filter(%{} = _filters) do
from(idea in Idea)
end

@doc """
Creates a lane.
Expand Down
4 changes: 4 additions & 0 deletions lib/mindwendel/repo.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@ defmodule Mindwendel.Repo do
use Ecto.Repo,
otp_app: :mindwendel,
adapter: Ecto.Adapters.Postgres

def count(query) do
aggregate(query, :count)
end
end
9 changes: 2 additions & 7 deletions lib/mindwendel_web/live/idea_live/card_component.ex
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,8 @@ defmodule MindwendelWeb.IdeaLive.CardComponent do
idea = Ideas.get_idea!(idea_id)
idea_label = IdeaLabels.get_idea_label(idea_label_id)

case(IdeaLabels.add_idea_label_to_idea(idea, idea_label)) do
{:ok, _idea} ->
{:noreply, socket}

{:error, _changeset} ->
{:noreply, socket}
end
IdeaLabels.add_idea_label_to_idea(idea, idea_label)
{:noreply, socket}
end

def handle_event(
Expand Down
2 changes: 1 addition & 1 deletion test/mindwendel/brainstormings_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ defmodule Mindwendel.BrainstormingsTest do

like = Factory.insert!(:like, idea: idea)

{:ok, idea} =
{:ok, _idea_idea_label} =
IdeaLabels.add_idea_label_to_idea(idea, Enum.at(brainstorming.labels, 0))

idea = idea |> Repo.preload([:idea_labels])
Expand Down
50 changes: 31 additions & 19 deletions test/mindwendel/idea_labels_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ defmodule Mindwendel.IdeaLabelsTest do

setup do
brainstorming = Factory.insert!(:brainstorming, %{labels: [Factory.build(:idea_label)]})
idea_label = brainstorming.labels |> Enum.at(0)
[idea_label] = brainstorming.labels
lane = Enum.at(brainstorming.lanes, 0)
idea = Factory.insert!(:idea, %{brainstorming: brainstorming, lane: lane})

Expand All @@ -18,43 +18,46 @@ defmodule Mindwendel.IdeaLabelsTest do

describe "#add_idea_label_to_idea" do
test "adds IdeaLabel to Idea", %{idea_label: idea_label, idea: idea} do
{:ok, idea_changed} = IdeaLabels.add_idea_label_to_idea(idea, idea_label)
{:ok, _idea_idea_label} = IdeaLabels.add_idea_label_to_idea(idea, idea_label)

assert idea_changed.idea_labels |> Enum.count() == 1
assert Repo.all(IdeaIdeaLabel) |> Enum.count() == 1
assert [idea_label] == labels_of(idea)
assert Repo.count(IdeaIdeaLabel) == 1
end

test "creates one IdeaIdeaLabel", %{idea_label: idea_label, idea: idea} do
{:ok, _idea_changed} = IdeaLabels.add_idea_label_to_idea(idea, idea_label)
{:ok, _idea_idea_label} = IdeaLabels.add_idea_label_to_idea(idea, idea_label)

assert Repo.all(IdeaIdeaLabel) |> Enum.count() == 1
assert Repo.count(IdeaIdeaLabel) == 1

idea_idea_label = Repo.one(IdeaIdeaLabel)
assert idea_idea_label.idea_label_id == idea_label.id
assert idea_idea_label.idea_id == idea.id
end

test "does not create additional IdeaLabel", %{idea_label: idea_label, idea: idea} do
assert Repo.all(IdeaLabel) |> Enum.count() == 1
assert Repo.count(IdeaLabel) == 1

{:ok, _idea_changed} = IdeaLabels.add_idea_label_to_idea(idea, idea_label)
{:ok, _idea_idea_label} = IdeaLabels.add_idea_label_to_idea(idea, idea_label)

assert Repo.all(IdeaLabel) |> Enum.count() == 1
assert Repo.count(IdeaLabel) == 1
assert Repo.one(IdeaLabel) == idea_label
end

@tag :skip
test "does not add the same IdeaLabel twice to Idea", %{idea_label: idea_label, idea: idea} do
# Calling this method twice does not fail and does not create duplicates
{:ok, idea_after_method_call_1} = IdeaLabels.add_idea_label_to_idea(idea, idea_label)
{:ok, idea_after_method_call_2} = IdeaLabels.add_idea_label_to_idea(idea, idea_label)
{:ok, idea_idea_label_after_method_call_1} =
IdeaLabels.add_idea_label_to_idea(idea, idea_label)

{:ok, idea_idea_label_after_method_call_2} =
IdeaLabels.add_idea_label_to_idea(idea, idea_label)

# There should still be only one IdeaIdeaLabel
assert Repo.all(IdeaIdeaLabel) |> Enum.count() == 1
assert Repo.count(IdeaIdeaLabel) == 1

assert Repo.all(IdeaLabel) |> Enum.count() == 1
assert Repo.count(IdeaLabel) == 1

assert idea_after_method_call_1 == idea_after_method_call_2
assert idea_idea_label_after_method_call_1 == idea_idea_label_after_method_call_2
end

@tag :skip
Expand Down Expand Up @@ -88,16 +91,16 @@ defmodule Mindwendel.IdeaLabelsTest do

@tag :skip
test "does not create idea_labels without brainstorming", %{idea: idea} do
assert Repo.all(IdeaIdeaLabel) |> Enum.count() == 0
assert Repo.count(IdeaIdeaLabel) == 0
idea = Repo.preload(idea, [:idea_labels, :idea_idea_labels])
idea_label = idea.brainstorming.labels |> Enum.at(0)

IdeaLabels.add_idea_label_to_idea(idea, idea_label)

assert Repo.all(IdeaIdeaLabel) |> Enum.count() == 1
assert Repo.count(IdeaIdeaLabel) == 1
assert Repo.one(IdeaIdeaLabel).idea_id == idea.id
assert Repo.one(IdeaIdeaLabel).idea_label_id == idea_label.id
assert Repo.all(IdeaLabel) |> Enum.count() == 5
assert Repo.count(IdeaLabel) == 5

assert Enum.empty?(
Repo.all(
Expand All @@ -110,8 +113,9 @@ defmodule Mindwendel.IdeaLabelsTest do

describe "#delete_idea_label_from_idea" do
setup %{idea_label: idea_label, idea: idea} do
{:ok, idea} = IdeaLabels.add_idea_label_to_idea(idea, idea_label)
assert Repo.all(IdeaIdeaLabel) |> Enum.count() == 1
{:ok, _idea_idea_label} = IdeaLabels.add_idea_label_to_idea(idea, idea_label)
idea = idea |> Repo.reload!() |> Repo.preload(:idea_labels)
assert Repo.count(IdeaIdeaLabel) == 1
%{idea: idea}
end

Expand All @@ -131,4 +135,12 @@ defmodule Mindwendel.IdeaLabelsTest do
assert Enum.empty?(Repo.all(IdeaIdeaLabel))
end
end

defp labels_of(idea_record) do
Repo.all(
from idea_label in IdeaLabel,
join: idea in assoc(idea_label, :ideas),
where: idea.id == ^idea_record.id
)
end
end

0 comments on commit ce34983

Please sign in to comment.