From 5f5b1454ed37ab485569f01f5bab5012c6c5e252 Mon Sep 17 00:00:00 2001 From: Elin Olsson Date: Fri, 15 Nov 2024 17:00:35 +0100 Subject: [PATCH] Fix broken query + add test for metrics filtering --- lib/nerves_hub/devices.ex | 32 +++++------------ lib/nerves_hub_web/live/devices/index.ex | 18 +++++----- .../live/devices/index_test.exs | 35 +++++++++++++++++++ 3 files changed, 52 insertions(+), 33 deletions(-) diff --git a/lib/nerves_hub/devices.ex b/lib/nerves_hub/devices.ex index 56ca3c9ff..127505820 100644 --- a/lib/nerves_hub/devices.ex +++ b/lib/nerves_hub/devices.ex @@ -161,36 +161,20 @@ defmodule NervesHub.Devices do def filter_on_metric(query, key, value, operator) do query - |> where( - [d], - d.id in subquery(metrics_query(key, value, operator)) - ) - |> preload_metric(key) + |> join(:inner, [d], m in DeviceMetric, on: d.id == m.device_id) + |> where([_, m], m.inserted_at == subquery(latest_metric_for_key(key))) + |> where([d, m], m.key == ^key) + |> gt_or_lt(value, operator) end - def metrics_query(key, value, operator) do + defp latest_metric_for_key(key) do DeviceMetric - |> from - |> select([d], d.device_id) + |> select([dm], max(dm.inserted_at)) |> where([dm], dm.key == ^key) - |> gt_or_lt(value, operator) - |> order_by(desc: :inserted_at) - |> distinct(:device_id) end - defp gt_or_lt(query, value, :gt), do: where(query, [dm], dm.value > ^value) - defp gt_or_lt(query, value, :lt), do: where(query, [dm], dm.value < ^value) - - defp preload_metric(query, key) do - preload_query = - DeviceMetric - |> distinct(:device_id) - |> where(key: ^key) - |> order_by([:device_id, desc: :inserted_at]) - - query - |> preload(device_metrics: ^preload_query) - end + defp gt_or_lt(query, value, :gt), do: where(query, [_, dm], dm.value > ^value) + defp gt_or_lt(query, value, :lt), do: where(query, [_, dm], dm.value < ^value) defp filtering(query, filters) do Enum.reduce(filters, query, fn {key, value}, query -> diff --git a/lib/nerves_hub_web/live/devices/index.ex b/lib/nerves_hub_web/live/devices/index.ex index fdae1fabd..695c8c965 100644 --- a/lib/nerves_hub_web/live/devices/index.ex +++ b/lib/nerves_hub_web/live/devices/index.ex @@ -92,8 +92,10 @@ defmodule NervesHubWeb.Live.Devices.Index do def handle_params(unsigned_params, _uri, socket) do filters = Map.merge(@default_filters, filter_changes(unsigned_params)) + # Until UI selectors for metrics are implemented, + # params can be added to the URL and be merged with filters. + # Example: ?metric=cpu_temp&metric_operator=gt&metric_value=37.0 |> filter_on_metrics_if_provided(unsigned_params) - |> dbg() pagination_opts = Map.merge(socket.assigns.paginate_opts, pagination_changes(unsigned_params)) @@ -108,11 +110,11 @@ defmodule NervesHubWeb.Live.Devices.Index do |> noreply() end - def filter_on_metrics_if_provided(filters, %{ - "metric" => metric_type, - "metric_value" => metric_value, - "metric_operator" => operator - }) do + defp filter_on_metrics_if_provided(filters, %{ + "metric" => metric_type, + "metric_value" => metric_value, + "metric_operator" => operator + }) do Map.put(filters, :metrics, %{ key: metric_type, value: String.to_float(metric_value), @@ -120,9 +122,7 @@ defmodule NervesHubWeb.Live.Devices.Index do }) end - def filter_on_metrics_if_provided(filters, _) do - filters - end + defp filter_on_metrics_if_provided(filters, _), do: filters defp self_path(socket, extra) do params = Enum.into(stringify_keys(extra), socket.assigns.params) diff --git a/test/nerves_hub_web/live/devices/index_test.exs b/test/nerves_hub_web/live/devices/index_test.exs index 29d77cd34..74a5c54ea 100644 --- a/test/nerves_hub_web/live/devices/index_test.exs +++ b/test/nerves_hub_web/live/devices/index_test.exs @@ -1,6 +1,7 @@ defmodule NervesHubWeb.Live.Devices.IndexTest do use NervesHubWeb.ConnCase.Browser, async: false + alias NervesHub.Devices alias NervesHub.Fixtures alias NervesHubWeb.Endpoint @@ -85,6 +86,40 @@ defmodule NervesHubWeb.Live.Devices.IndexTest do device2.identifier end + test "filters devices by metrics", %{conn: conn, fixture: fixture} do + %{device: device, firmware: firmware, org: org, product: product} = fixture + + device2 = Fixtures.device_fixture(org, product, firmware, %{}) + {:ok, _view, html} = live(conn, device_index_path(fixture)) + assert html =~ device.identifier + assert html =~ device2.identifier + + # Add metrics for device2, sleep between to secure order. + Devices.Metrics.save_metric(%{device_id: device2.id, key: "cpu_temp", value: 36}) + :timer.sleep(100) + Devices.Metrics.save_metric(%{device_id: device2.id, key: "cpu_temp", value: 42}) + :timer.sleep(100) + Devices.Metrics.save_metric(%{device_id: device2.id, key: "load_1min", value: 3}) + + greater_than_filter = + device_index_path(fixture) <> "?metric=cpu_temp&metric_operator=gt&metric_value=37.0" + + {:ok, _view, html} = live(conn, greater_than_filter) + + # Show only show device2, which has a value greater than 37 on most recent cpu_temp metric. + assert html =~ device2.identifier + refute html =~ device.identifier + + less_than_filter = + device_index_path(fixture) <> "?metric=cpu_temp&metric_operator=lt&metric_value=37.0" + + {:ok, _view, html} = live(conn, less_than_filter) + + # Should not show any device since the query is for values less than 37 + refute html =~ device2.identifier + refute html =~ device.identifier + end + test "filters devices by several tags", %{conn: conn, fixture: fixture} do %{device: _device, firmware: firmware, org: org, product: product} = fixture