Fix bugs and performance: Oban chain, 0-cent prices, duplicate queries, regex fragility

- Fix Oban self-scheduling chain silently dying (remove unique constraint so after block can schedule next job)
- Fix Natchez and TrueShotAmmo extract_price defaulting to 0 for missing price data
- Fix caliber_matcher redundant String.downcase inside Enum.find (pre-downcase outside loop)
- Fix runner.ex double iteration (Enum.map + Enum.count merged into single Enum.reduce)
- Fix price_stats_for_caliber firing 3 queries instead of 2 (merge all_time + thirty_day via FILTER)
- Fix PSA duplicate Floki.find on same selector (merge extract_title + extract_url)
- Fix PubSub broadcast carrying no caliber IDs (now includes IDs; clients skip irrelevant reloads)
- Fix BulkAmmo ^ anchor skipping non-leading round counts
- Fix TargetSportsUSA fragile exact text match 'Add To Cart' (use case-insensitive contains)
- Fix SgAmmo dead destructure {_, _, _} = row
- Fix text_detector brass/nickel detection order
- Fix Natchez GraphQL string interpolation (add escape_gql_string)
- Fix chart data triple Enum.map merged into single reduce
- Change Oban scraping queue workers from 2 to 1 (only one needed with Process.sleep)
This commit is contained in:
Graham McIntire 2026-06-21 18:14:49 -05:00
parent a52a73ee09
commit 872c94bea1
No known key found for this signature in database
GPG key ID: F4ABF488E6029E59
16 changed files with 132 additions and 102 deletions

View file

@ -30,7 +30,7 @@ config :ammoprices, AmmopricesWeb.Endpoint,
# Configure Oban # Configure Oban
config :ammoprices, Oban, config :ammoprices, Oban,
repo: Ammoprices.Repo, repo: Ammoprices.Repo,
queues: [scraping: 2], queues: [scraping: 1],
plugins: [Oban.Plugins.Lifeline] plugins: [Oban.Plugins.Lifeline]
config :ammoprices, config :ammoprices,

View file

@ -119,13 +119,25 @@ defmodule Ammoprices.Prices do
Based on in-stock snapshots only. Based on in-stock snapshots only.
""" """
def price_stats_for_caliber(caliber_id) do def price_stats_for_caliber(caliber_id) do
thirty_days_ago = DateTime.add(DateTime.utc_now(), -30 * 86_400, :second)
stats = stats =
Repo.one( Repo.one(
from(ps in PriceSnapshot, from(ps in PriceSnapshot,
join: p in assoc(ps, :product), join: p in assoc(ps, :product),
where: p.caliber_id == ^caliber_id, where: p.caliber_id == ^caliber_id,
where: ps.in_stock == true, where: ps.in_stock == true,
select: %{all_time_low: min(ps.price_per_round_cents), all_time_high: max(ps.price_per_round_cents)} select: %{
all_time_low: min(ps.price_per_round_cents),
all_time_high: max(ps.price_per_round_cents),
thirty_day_avg:
fragment(
"round(avg(?) filter (where ? >= ?))::integer",
ps.price_per_round_cents,
ps.recorded_at,
^thirty_days_ago
)
}
) )
) )
@ -142,24 +154,11 @@ defmodule Ammoprices.Prices do
|> select([s], min(s.price_per_round_cents)) |> select([s], min(s.price_per_round_cents))
|> Repo.one() |> Repo.one()
thirty_days_ago = DateTime.add(DateTime.utc_now(), -30 * 86_400, :second)
thirty_day_avg =
Repo.one(
from(ps in PriceSnapshot,
join: p in assoc(ps, :product),
where: p.caliber_id == ^caliber_id,
where: ps.in_stock == true,
where: ps.recorded_at >= ^thirty_days_ago,
select: fragment("round(avg(?))::integer", ps.price_per_round_cents)
)
)
%{ %{
current_min: current_min, current_min: current_min,
all_time_low: stats.all_time_low, all_time_low: stats.all_time_low,
all_time_high: stats.all_time_high, all_time_high: stats.all_time_high,
thirty_day_avg: thirty_day_avg thirty_day_avg: stats.thirty_day_avg
} }
end end

View file

@ -2,20 +2,32 @@ defmodule Ammoprices.Scraping.CaliberMatcher do
@moduledoc false @moduledoc false
def match(title, calibers) do def match(title, calibers) do
downcased = String.downcase(title) downcased_title = String.downcase(title)
prepped = Enum.map(calibers, &prepare_caliber/1)
Enum.find(calibers, fn caliber -> case Enum.find(prepped, fn caliber ->
name_matches?(downcased, caliber) or alias_matches?(downcased, caliber) name_matches?(downcased_title, caliber) or alias_matches?(downcased_title, caliber)
end) end) do
nil -> nil
caliber -> caliber.original
end
end
defp prepare_caliber(caliber) do
%{
original: caliber,
name: String.downcase(caliber.name),
aliases: Enum.map(caliber.aliases, &String.downcase/1)
}
end end
defp name_matches?(downcased_title, caliber) do defp name_matches?(downcased_title, caliber) do
String.contains?(downcased_title, String.downcase(caliber.name)) String.contains?(downcased_title, caliber.name)
end end
defp alias_matches?(downcased_title, caliber) do defp alias_matches?(downcased_title, caliber) do
Enum.any?(caliber.aliases, fn a -> Enum.any?(caliber.aliases, fn a ->
String.contains?(downcased_title, String.downcase(a)) String.contains?(downcased_title, a)
end) end)
end end
end end

View file

@ -127,7 +127,7 @@ defmodule Ammoprices.Scraping.Retailers.BulkAmmo do
end end
defp extract_round_count(title) do defp extract_round_count(title) do
case Regex.run(~r/^(\d[\d,]*)\s+[Rr]ounds?/i, title) do case Regex.run(~r/(\d[\d,]*)\s+[Rr]ounds?/i, title) do
[_, count] -> count |> String.replace(",", "") |> String.to_integer() [_, count] -> count |> String.replace(",", "") |> String.to_integer()
_ -> nil _ -> nil
end end

View file

@ -60,11 +60,13 @@ defmodule Ammoprices.Scraping.Retailers.Natchez do
nil nil
caliber_value -> caliber_value ->
escaped = escape_gql_string(caliber_value)
""" """
{ {
products( products(
search: "#{caliber_value}" search: "#{escaped}"
filter: { caliber: { eq: "#{caliber_value}" } } filter: { caliber: { eq: "#{escaped}" } }
pageSize: 48 pageSize: 48
currentPage: 1 currentPage: 1
) { ) {
@ -80,6 +82,13 @@ defmodule Ammoprices.Scraping.Retailers.Natchez do
end end
end end
defp escape_gql_string(str) do
str
|> String.replace("\\", "\\\\")
|> String.replace("\"", "\\\"")
|> String.replace("\n", "\\n")
end
def parse_graphql_response(%{"data" => %{"products" => %{"items" => items}}}) do def parse_graphql_response(%{"data" => %{"products" => %{"items" => items}}}) do
items items
|> Enum.map(&parse_item/1) |> Enum.map(&parse_item/1)
@ -89,33 +98,39 @@ defmodule Ammoprices.Scraping.Retailers.Natchez do
def parse_graphql_response(_), do: [] def parse_graphql_response(_), do: []
defp parse_item(item) do defp parse_item(item) do
price_cents = extract_price(item) case extract_price(item) do
round_count = parse_integer(item["rounds"]) {:ok, price_cents} ->
ppr = if round_count && round_count > 0, do: round(price_cents / round_count) round_count = parse_integer(item["rounds"])
name = item["name"] ppr = if round_count && round_count > 0, do: round(price_cents / round_count)
name = item["name"]
%{ %{
title: name, title: name,
url: "/#{item["url_key"]}.html", url: "/#{item["url_key"]}.html",
brand: item["brand"], brand: item["brand"],
price_cents: price_cents, price_cents: price_cents,
price_per_round_cents: ppr, price_per_round_cents: ppr,
grain_weight: parse_integer(item["grain"]), grain_weight: parse_integer(item["grain"]),
round_count: round_count, round_count: round_count,
casing: parse_casing(item["case_material"]), casing: parse_casing(item["case_material"]),
subsonic: TextDetector.detect_subsonic(name), subsonic: TextDetector.detect_subsonic(name),
in_stock: item["stock_status"] == "IN_STOCK" in_stock: item["stock_status"] == "IN_STOCK"
} }
:error ->
nil
end
end end
defp extract_price(%{"price_range" => %{"minimum_price" => %{"final_price" => %{"value" => value}}}}) do defp extract_price(%{"price_range" => %{"minimum_price" => %{"final_price" => %{"value" => value}}}}) do
value {:ok,
|> String.to_float() value
|> Kernel.*(100) |> String.to_float()
|> round() |> Kernel.*(100)
|> round()}
end end
defp extract_price(_), do: 0 defp extract_price(_), do: :error
defp parse_integer(nil), do: nil defp parse_integer(nil), do: nil

View file

@ -45,8 +45,7 @@ defmodule Ammoprices.Scraping.Retailers.PalmettoStateArmory do
end end
defp parse_item(item) do defp parse_item(item) do
with {:ok, title} <- extract_title(item), with {:ok, title, url} <- extract_link_info(item),
{:ok, url} <- extract_url(item),
{:ok, price_cents} <- extract_price(item), {:ok, price_cents} <- extract_price(item),
{:ok, ppr_cents} <- extract_unit_price(item) do {:ok, ppr_cents} <- extract_unit_price(item) do
%{ %{
@ -66,18 +65,13 @@ defmodule Ammoprices.Scraping.Retailers.PalmettoStateArmory do
end end
end end
defp extract_title(item) do defp extract_link_info(item) do
case Floki.find(item, ".product-item-link") do case Floki.find(item, ".product-item-link") do
[{_, _, _} = node] -> {:ok, node |> Floki.text() |> String.trim()} [{_, attrs, _} = node] ->
_ -> :error title = node |> Floki.text() |> String.trim()
end
end
defp extract_url(item) do
case Floki.find(item, ".product-item-link") do
[{_, attrs, _}] ->
case List.keyfind(attrs, "href", 0) do case List.keyfind(attrs, "href", 0) do
{"href", href} -> {:ok, href} {"href", href} -> {:ok, title, href}
_ -> :error _ -> :error
end end

View file

@ -161,7 +161,6 @@ defmodule Ammoprices.Scraping.Retailers.SgAmmo do
end end
defp extract_in_stock(row) do defp extract_in_stock(row) do
{_, _attrs, _} = row
classes = [row] |> Floki.attribute("class") |> List.first() || "" classes = [row] |> Floki.attribute("class") |> List.first() || ""
String.contains?(classes, "instock") String.contains?(classes, "instock")
end end

View file

@ -183,8 +183,8 @@ defmodule Ammoprices.Scraping.Retailers.TargetSportsUsa do
defp extract_in_stock(item) do defp extract_in_stock(item) do
case Floki.find(item, ".add-to-cart") do case Floki.find(item, ".add-to-cart") do
[{_, _, _} = node] -> [{_, _, _} = node] ->
text = node |> Floki.text() |> String.trim() text = node |> Floki.text() |> String.trim() |> String.downcase()
text == "Add To Cart" String.contains?(text, "add to cart")
_ -> _ ->
false false

View file

@ -47,27 +47,25 @@ defmodule Ammoprices.Scraping.Retailers.TrueShotAmmo do
title = product["title"] title = product["title"]
first_variant = List.first(product["variants"]) first_variant = List.first(product["variants"])
case first_variant do with variant when not is_nil(variant) <- first_variant,
nil -> price_cents when is_integer(price_cents) <- parse_price(variant["price"]) do
nil round_count = parse_round_count(variant["title"])
ppr = if round_count && round_count > 0, do: round(price_cents / round_count)
variant -> %{
price_cents = parse_price(variant["price"]) title: title,
round_count = parse_round_count(variant["title"]) url: "/products/#{product["handle"]}",
ppr = if round_count && round_count > 0, do: round(price_cents / round_count) brand: product["vendor"],
price_cents: price_cents,
%{ price_per_round_cents: ppr,
title: title, grain_weight: extract_grain_weight(title),
url: "/products/#{product["handle"]}", round_count: round_count,
brand: product["vendor"], casing: TextDetector.detect_casing(title),
price_cents: price_cents, subsonic: TextDetector.detect_subsonic(title),
price_per_round_cents: ppr, in_stock: variant["available"] == true
grain_weight: extract_grain_weight(title), }
round_count: round_count, else
casing: TextDetector.detect_casing(title), _ -> nil
subsonic: TextDetector.detect_subsonic(title),
in_stock: variant["available"] == true
}
end end
end end
@ -78,7 +76,7 @@ defmodule Ammoprices.Scraping.Retailers.TrueShotAmmo do
|> round() |> round()
end end
defp parse_price(_), do: 0 defp parse_price(_), do: nil
defp parse_round_count(title) when is_binary(title) do defp parse_round_count(title) when is_binary(title) do
case Integer.parse(title) do case Integer.parse(title) do

View file

@ -51,9 +51,13 @@ defmodule Ammoprices.Scraping.Runner do
defp process_results(parsed, retailer, caliber) do defp process_results(parsed, retailer, caliber) do
now = DateTime.truncate(DateTime.utc_now(), :second) now = DateTime.truncate(DateTime.utc_now(), :second)
results = Enum.map(parsed, &process_product(&1, retailer, caliber, now)) {_results, successful} =
Enum.reduce(parsed, {[], 0}, fn product_data, {acc, count} ->
successful = Enum.count(results, &(&1 == :ok)) case process_product(product_data, retailer, caliber, now) do
:ok -> {[:ok | acc], count + 1}
:error -> {[:error | acc], count}
end
end)
if successful > 0 do if successful > 0 do
Catalog.mark_unseen_products_out_of_stock(retailer.id, caliber.id, now) Catalog.mark_unseen_products_out_of_stock(retailer.id, caliber.id, now)

View file

@ -2,8 +2,7 @@ defmodule Ammoprices.Scraping.ScrapeJob do
@moduledoc false @moduledoc false
use Oban.Worker, use Oban.Worker,
queue: :scraping, queue: :scraping,
max_attempts: 3, max_attempts: 3
unique: [period: :infinity, states: [:available, :scheduled, :executing, :retryable, :suspended]]
alias Ammoprices.Catalog alias Ammoprices.Catalog
alias Ammoprices.Scraping.Runner alias Ammoprices.Scraping.Runner
@ -26,6 +25,7 @@ defmodule Ammoprices.Scraping.ScrapeJob do
@impl Oban.Worker @impl Oban.Worker
def perform(_job) do def perform(_job) do
calibers = Catalog.list_calibers() calibers = Catalog.list_calibers()
caliber_ids = Enum.map(calibers, & &1.id)
for scraper <- @scrapers, caliber <- calibers do for scraper <- @scrapers, caliber <- calibers do
scrape_delay() scrape_delay()
@ -34,7 +34,7 @@ defmodule Ammoprices.Scraping.ScrapeJob do
prune_stale_products() prune_stale_products()
Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, %{}}) Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, caliber_ids})
:ok :ok
after after

View file

@ -12,8 +12,8 @@ defmodule Ammoprices.Scraping.TextDetector do
cond do cond do
String.contains?(downcased, "steel") -> "steel" String.contains?(downcased, "steel") -> "steel"
String.contains?(downcased, "aluminum") -> "aluminum" String.contains?(downcased, "aluminum") -> "aluminum"
String.contains?(downcased, "nickel") -> "brass"
String.contains?(downcased, "brass") -> "brass" String.contains?(downcased, "brass") -> "brass"
String.contains?(downcased, "nickel") -> "brass"
true -> nil true -> nil
end end
end end

View file

@ -103,15 +103,19 @@ defmodule AmmopricesWeb.CaliberLive.Show do
end end
@impl true @impl true
def handle_info({:prices_updated, _}, socket) do def handle_info({:prices_updated, caliber_ids}, socket) do
socket = if socket.assigns.caliber.id in caliber_ids do
socket socket =
|> load_filter_options() socket
|> load_products() |> load_filter_options()
|> load_chart_data() |> load_products()
|> load_price_stats() |> load_chart_data()
|> load_price_stats()
{:noreply, push_event(socket, "update-chart", %{data: socket.assigns.chart_data})} {:noreply, push_event(socket, "update-chart", %{data: socket.assigns.chart_data})}
else
{:noreply, socket}
end
end end
defp load_filter_options(socket) do defp load_filter_options(socket) do
@ -178,10 +182,15 @@ defmodule AmmopricesWeb.CaliberLive.Show do
days = Map.get(@range_days, range, 30) days = Map.get(@range_days, range, 30)
daily_data = Prices.daily_averages_for_caliber(caliber.id, days: days) daily_data = Prices.daily_averages_for_caliber(caliber.id, days: days)
{labels, avgs, mins} =
Enum.reduce(daily_data, {[], [], []}, fn d, {ls, as, ms} ->
{[Date.to_iso8601(d.date) | ls], [d.avg_ppr | as], [d.min_ppr | ms]}
end)
chart_data = %{ chart_data = %{
labels: Enum.map(daily_data, &Date.to_iso8601(&1.date)), labels: Enum.reverse(labels),
avg: Enum.map(daily_data, & &1.avg_ppr), avg: Enum.reverse(avgs),
min: Enum.map(daily_data, & &1.min_ppr) min: Enum.reverse(mins)
} }
assign(socket, chart_data: chart_data, chart_empty?: daily_data == []) assign(socket, chart_data: chart_data, chart_empty?: daily_data == [])

View file

@ -40,7 +40,7 @@ defmodule AmmopricesWeb.HomeLive do
end end
@impl true @impl true
def handle_info({:prices_updated, _}, socket) do def handle_info({:prices_updated, _caliber_ids}, socket) do
{:noreply, assign_cheapest_map(socket)} {:noreply, assign_cheapest_map(socket)}
end end

View file

@ -228,7 +228,7 @@ defmodule AmmopricesWeb.CaliberLive.ShowTest do
snapshot_fixture(%{product: new_product, price_per_round_cents: 18, in_stock: true, recorded_at: now}) snapshot_fixture(%{product: new_product, price_per_round_cents: 18, in_stock: true, recorded_at: now})
# Broadcast price update # Broadcast price update
Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, %{}}) Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, [caliber.id]})
# The view should re-render with updated data # The view should re-render with updated data
html = render(view) html = render(view)

View file

@ -43,7 +43,7 @@ defmodule AmmopricesWeb.HomeLiveTest do
{:ok, view, _html} = live(conn, "/") {:ok, view, _html} = live(conn, "/")
snapshot_fixture(%{product: product, price_per_round_cents: 17, in_stock: true, recorded_at: now}) snapshot_fixture(%{product: product, price_per_round_cents: 17, in_stock: true, recorded_at: now})
Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, %{}}) Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, [caliber.id]})
assert render(view) =~ "0.17" assert render(view) =~ "0.17"
end end