Skip to content

Commit

Permalink
fix: reduce use of length/1 whenever not needed
Browse files Browse the repository at this point in the history
  • Loading branch information
hauleth committed Oct 25, 2023
1 parent 27cbb71 commit d30cacd
Show file tree
Hide file tree
Showing 24 changed files with 127 additions and 66 deletions.
2 changes: 1 addition & 1 deletion lib/logflare/account_email.ex
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ defmodule Logflare.AccountEmail do

diff = diff_schema(schema_to_list(formatted_new), schema_to_list(formatted_old))

if diff == [] do
if Enum.empty?(diff) do
# Something generates BOOL and something else generates BOOLEAN which causes this
Logger.error("Schema update email send with no new fields.",
source_id: source.token,
Expand Down
2 changes: 1 addition & 1 deletion lib/logflare/auth.ex
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ defmodule Logflare.Auth do
defp check_scopes(token_scopes, required) do
cond do
"private" in token_scopes -> :ok
required == [] -> :ok
Enum.empty?(required) -> :ok
Enum.any?(token_scopes, fn scope -> scope in required end) -> :ok
true -> {:error, :unauthorized}
end
Expand Down
2 changes: 1 addition & 1 deletion lib/logflare/endpoints/cache.ex
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ defmodule Logflare.Endpoints.Cache do
refresh(proactive_querying_ms(state))
{:noreply, %{state | query_tasks: tasks}}

tasks == [] ->
Enum.empty?(tasks) ->
{:stop, :normal, state}

true ->
Expand Down
10 changes: 5 additions & 5 deletions lib/logflare/logs/log_events.ex
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ defmodule Logflare.Logs.LogEvents do
le = LogEvent.make_from_db(row, %{source: source})
{:ok, le}

%{rows: rows} when length(rows) >= 1 ->
%{rows: rows} ->
row = Enum.find(rows, &(&1.id == id))
le = LogEvent.make_from_db(row, %{source: source})
{:ok, le}
Expand Down Expand Up @@ -150,14 +150,14 @@ defmodule Logflare.Logs.LogEvents do

defp process(result) do
case result do
{:ok, %{rows: rows}} when length(rows) > 1 ->
{:error, "Multiple rows returned, expected one"}
{:ok, %{rows: []}} ->
nil

{:ok, %{rows: [row]}} ->
row

{:ok, %{rows: []}} ->
nil
{:ok, %{rows: _rows}} ->
{:error, "Multiple rows returned, expected one"}

{:error, error} ->
{:error, error}
Expand Down
3 changes: 2 additions & 1 deletion lib/logflare/logs/logs.ex
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ defmodule Logflare.Logs do
)
when is_binary(drop_lql_string) do
cond do
length(filters) >= 1 && SourceRouting.route_with_lql_rules?(le, %Rule{lql_filters: filters}) ->
not Enum.empty?(filters) and
SourceRouting.route_with_lql_rules?(le, %Rule{lql_filters: filters}) ->
Map.put(le, :drop, true)

true ->
Expand Down
3 changes: 2 additions & 1 deletion lib/logflare/logs/lql/lql_encoder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ defmodule Logflare.Lql.Encoder do
|> Enum.map_join(" ", &to_fragment/1)
|> String.replace("chart:timestamp", "")

{"m.level", filter_rules} when length(filter_rules) >= 2 ->
# `filter_rules` has at least 2 entries
{"m.level", [_, _ | _] = filter_rules} ->
{min_level, max_level} =
Enum.min_max_by(filter_rules, &Parser.Helpers.get_level_order(&1.value))

Expand Down
10 changes: 6 additions & 4 deletions lib/logflare/logs/lql/lql_parser.ex
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,16 @@ defmodule Logflare.Lql.Parser do

defp maybe_cast_value(c, {:list, type}), do: maybe_cast_value(c, type)

defp maybe_cast_value(%{values: values, value: nil} = c, type) when length(values) >= 1 do
defp maybe_cast_value(%{values: values, value: nil} = c, type) when values != [] do
%{
c
| values:
values
|> Enum.map(&%{value: &1, path: c.path})
|> Enum.map(&maybe_cast_value(&1, type))
|> Enum.map(& &1.value)
|> Enum.map(fn data ->
%{value: data, path: c.path}
|> maybe_cast_value(type)
|> Map.fetch!(:value)
end)
}
end

Expand Down
2 changes: 1 addition & 1 deletion lib/logflare/logs/search/logs_search_operations.ex
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ defmodule Logflare.Logs.SearchOperations do
so.tailing? and not Enum.empty?(so.lql_ts_filters) ->
Utils.halt(so, @timestamp_filter_with_tailing)

length(so.chart_rules) > 1 ->
Logflare.Utils.List.at_least?(so.chart_rules, 2) ->
Utils.halt(so, "Only one chart rule can be used in a query")

match?([_], so.chart_rules) and
Expand Down
2 changes: 1 addition & 1 deletion lib/logflare/logs/search/logs_search_operations_helpers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ defmodule Logflare.Logs.SearchOperations.Helpers do
%{min: min, max: max, message: message}
end

def get_min_max_filter_timestamps(ts_filters, _chart_period) when length(ts_filters) > 1 do
def get_min_max_filter_timestamps(ts_filters, _chart_period) when is_list(ts_filters) do
{min, max} =
ts_filters
|> Enum.map(& &1.value)
Expand Down
6 changes: 4 additions & 2 deletions lib/logflare/logs/search/lql_validator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ defmodule Logflare.Lql.Validator do
@moduledoc false
import Logflare.Logs.SearchOperations.Helpers

alias Logflare.Utils.List, as: ListH

@timestamp_filter_with_tailing "Timestamp filters can't be used if live tail search is active"
@default_max_n_chart_ticks 250

Expand All @@ -20,10 +22,10 @@ defmodule Logflare.Lql.Validator do
tailing? and not Enum.empty?(lql_ts_filters) ->
@timestamp_filter_with_tailing

length(chart_rules) > 1 ->
ListH.at_least?(chart_rules, 2) ->
"Only one chart rule can be used in a LQL query"

match?([_], chart_rules) and
ListH.exactly?(chart_rules, 1) and
hd(chart_rules).value_type not in ~w[integer float]a and
hd(chart_rules).path != "timestamp" ->
chart_rule = hd(chart_rules)
Expand Down
2 changes: 1 addition & 1 deletion lib/logflare/logs/source_parsers/syslog_parser.ex
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ defmodule Logflare.Logs.SyslogParser do
defp merge_syslog_sd(tokens) when is_list(tokens) do
{sd_element_values, new_tokens} = Keyword.pop_values(tokens, :sd_element)

if length(sd_element_values) > 0 do
if not Enum.empty?(sd_element_values) do
sd =
sd_element_values
|> Enum.map(fn sd_element ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ defmodule Logflare.Logs.Validators.EqDeepFieldTypes do
is_list_of_enums(merged) ->
deep_merge_enums(merged)

merged == [] ->
Enum.empty?(merged) ->
{:list, :empty}

is_homogenous_list(merged) ->
Expand Down
10 changes: 5 additions & 5 deletions lib/logflare/single_tenant.ex
Original file line number Diff line number Diff line change
Expand Up @@ -261,13 +261,13 @@ defmodule Logflare.SingleTenant do
seed_plan = if default_plan, do: :ok

seed_sources =
if default_user do
if Sources.list_sources_by_user(default_user) |> length() > 0, do: :ok
if default_user && not Enum.empty?(Sources.list_sources_by_user(default_user)) do
:ok
end

seed_endpoints =
if default_user do
if Endpoints.list_endpoints_by(user_id: default_user.id) |> length() > 0, do: :ok
if default_user && not Enum.empty?(Endpoints.list_endpoints_by(user_id: default_user.id)) do
:ok
end

source_schemas_updated =
Expand Down Expand Up @@ -310,7 +310,7 @@ defmodule Logflare.SingleTenant do
state.field_count > 3
end

Enum.all?(checks) and length(sources) > 0
Enum.all?(checks) and not Enum.empty?(sources)
else
false
end
Expand Down
24 changes: 13 additions & 11 deletions lib/logflare/source/rate_counter_server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -140,18 +140,12 @@ defmodule Logflare.Source.RateCounterServer do
# TODO: optimize by not recalculating total sum and average
new_queue = LQueue.push(bucket.queue, state.last_rate)

average =
stats =
new_queue
|> Enum.to_list()
|> average()
|> round()
|> stats()

sum =
new_queue
|> Enum.to_list()
|> Enum.sum()

{length, %{bucket | queue: new_queue, average: average, sum: sum}}
{length, %{bucket | queue: new_queue, average: stats.avg, sum: stats.sum}}
end
end)
end
Expand Down Expand Up @@ -274,8 +268,16 @@ defmodule Logflare.Source.RateCounterServer do
Sources.Counters.get_inserts(source_id)
end

def average(xs) when is_list(xs) do
Enum.sum(xs) / length(xs)
def stats(xs) when is_list(xs) do
{total, count} =
Enum.reduce(xs, {0, 0}, fn v, {total, count} ->
{total + v, count + 1}
end)

%{
avg: total / count,
sum: total
}
end

defp init_counters(source_id, bigquery_project_id) when is_atom(source_id) do
Expand Down
11 changes: 3 additions & 8 deletions lib/logflare/sources.ex
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ defmodule Logflare.Sources do
rules =
for rule <- rules do
case rule do
%Rule{lql_filters: lql_filters} when length(lql_filters) >= 1 ->
%Rule{lql_filters: lql_filters} when lql_filters != [] ->
rule

%Rule{regex_struct: rs} when not is_nil(rs) ->
Expand Down Expand Up @@ -341,15 +341,10 @@ defmodule Logflare.Sources do
%{source | metrics: new_metrics}
end

def valid_source_token_param?(string) when is_binary(string) do
case String.length(string) === 36 && Ecto.UUID.cast(string) do
{:ok, _} -> true
_ -> false
end
def valid_source_token_param?(string) do
match?({:ok, _}, Ecto.UUID.dump(string))
end

def valid_source_token_param?(_), do: false

def delete_slack_hook_url(source) do
source
|> Source.changeset(%{slack_hook_url: nil})
Expand Down
6 changes: 3 additions & 3 deletions lib/logflare/sources/rules.ex
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ defmodule Logflare.Rules do
%Rule{regex_struct: rs}, _ when is_nil(rs) ->
{:cont, false}

%Rule{regex_struct: rs, lql_filters: lf}, _ when is_map(rs) and length(lf) >= 1 ->
{:cont, false}

%Rule{regex_struct: rs, lql_filters: []}, _ when is_map(rs) ->
{:halt, true}

%Rule{regex_struct: rs}, _ when is_map(rs) ->
{:cont, false}
end)
end

Expand Down
4 changes: 2 additions & 2 deletions lib/logflare/sources/source_routing.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ defmodule Logflare.Logs.SourceRouting do
rule.regex_struct || if(rule.regex != nil, do: Regex.compile!(rule.regex), else: nil)

cond do
length(rule.lql_filters) >= 1 and route_with_lql_rules?(le, rule) ->
not Enum.empty?(rule.lql_filters) and route_with_lql_rules?(le, rule) ->
do_route(le, rule)

regex_struct != nil and Regex.match?(regex_struct, body["event_message"]) ->
Expand Down Expand Up @@ -53,7 +53,7 @@ defmodule Logflare.Logs.SourceRouting do

@spec route_with_lql_rules?(LE.t(), Rule.t()) :: boolean()
def route_with_lql_rules?(%LE{body: le_body}, %Rule{lql_filters: lql_filters})
when length(lql_filters) >= 1 do
when lql_filters != [] do
lql_rules_match? =
Enum.reduce_while(lql_filters, true, fn lql_filter, _acc ->
%Lql.FilterRule{path: path, value: value, operator: operator, modifiers: mds} = lql_filter
Expand Down
54 changes: 54 additions & 0 deletions lib/logflare/utils/list.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
defmodule Logflare.Utils.List do
@doc """
Check if `list`'s lenght is *exactly* `n`
It does the same check as `length(list) == n`, but it will do at most `n`
steps through the list, while `length/1` will always step through whole `list`.
This can have negative performance impact if `lenght(list)` is much larger
than `n`.
## Example
```elixir
iex> #{__MODULE__}.exactly?([1, 2, 3], 2)
false
iex> #{__MODULE__}.exactly?([1, 2, 3], 3)
true
iex> #{__MODULE__}.exactly?([1, 2, 3], 4)
false
```
"""
@spec exactly?(list :: list(), n :: non_neg_integer()) :: boolean()
def exactly?([], 0), do: true
def exactly?([], _), do: false
def exactly?([_ | _], 0), do: false

def exactly?([_ | rest], n) when is_integer(n) and n > 0,
do: exactly?(rest, n - 1)

@doc """
Check if `list`'s length is *at least* of `n`
It does the same check as `length(list) >= n`, but it will do at most `n`
steps through the list, while `length/1` will always step through whole `list`.
This can have negative performance impact if `lenght(list)` is much larger
than `n`.
## Example
```elixir
iex> #{__MODULE__}.at_least?([1, 2, 3], 2)
true
iex> #{__MODULE__}.at_least?([1, 2, 3], 3)
true
iex> #{__MODULE__}.at_least?([1, 2, 3], 4)
false
```
"""
@spec at_least?(list :: list(), n :: non_neg_integer()) :: boolean()
def at_least?([], n) when n > 0, do: false
def at_least?(list, 0) when is_list(list), do: true

def at_least?([_ | rest], n) when is_integer(n) and n > 0,
do: at_least?(rest, n - 1)
end
20 changes: 12 additions & 8 deletions lib/logflare_web/live/endpoints/actions/show.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,25 @@
curl "<%= url(~p"/api/endpoints/query/#{@show_endpoint.token}") %>" \
-H 'X-API-KEY: YOUR-ACCESS-TOKEN' \
-H 'Content-Type: application/json; charset=utf-8'
<span :if={length(@declared_params) > 0}>\</span>
<span :if={length(@declared_params) > 0}>
-G <%= Enum.map(@declared_params, fn p -> "-d \"#{p}=VALUE\"" end) |> Enum.join(" ") %>
</span>
<%= if not Enum.empty?(@declared_params) do %>
<span>\</span>
<span>
-G <%= Enum.map_join(@declared_params, " ", fn p -> "-d \"#{p}=VALUE\"" end) %>
</span>
<% end %>
</code>

<code :if={@show_endpoint.enable_auth} class="tw-whitespace-pre-wrap">
# By name
curl "<%= url(~p"/api/endpoints/query/#{@show_endpoint.name}") %>" \
-H 'X-API-KEY: YOUR-ACCESS-TOKEN' \
-H 'Content-Type: application/json; charset=utf-8'
<span :if={length(@declared_params) > 0}>\</span>
<span :if={length(@declared_params) > 0}>
-G <%= Enum.map(@declared_params, fn p -> "-d \"#{p}=VALUE\"" end) |> Enum.join(" ") %>
</span>
<%= if not Enum.empty?(@declared_params) do %>
<span>\</span>
<span>
-G <%= Enum.map_join(@declared_params, " ", fn p -> "-d \"#{p}=VALUE\"" end) %>
</span>
<% end %>
</code>
</div>
</section>
2 changes: 1 addition & 1 deletion lib/logflare_web/live/search_live/logs_search_lv.ex
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ defmodule LogflareWeb.Source.SearchLV do
defp warning_message(assigns, search_op) do
tailing? = assigns.tailing?
querystring = assigns.querystring
log_events_empty? = search_op.events.rows == []
log_events_empty? = Enum.empty?(search_op.events.rows)

cond do
log_events_empty? and not tailing? ->
Expand Down
5 changes: 2 additions & 3 deletions lib/logflare_web/live/source_backends_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ defmodule LogflareWeb.SourceBackendsLive do
<div class="my-4">
<%= if !@show_create_form do %>
<button class="btn btn-primary" phx-click="toggle-create-form">Add a backend</button>
<% end %>
<%= if @show_create_form do %>
<% else %>
<.form
:let={f}
for={%{}}
Expand Down Expand Up @@ -60,7 +59,7 @@ defmodule LogflareWeb.SourceBackendsLive do
<%= submit("Add", class: "btn btn-primary") %>
</.form>
<% end %>
<div :if={@source_backends != []}>
<div :if={not Enum.empty?(@source_backends)}>
Backends:
<ul>
<li :for={sb <- @source_backends}>
Expand Down
Loading

0 comments on commit d30cacd

Please sign in to comment.