mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-10-05 09:35:31 -03:00
fix(update): read model-page prices from a host that answers
End-to-end verification against the live site found the price capture broken for a whole class of users: the civitai page hosts are not interchangeable, and the user's civitai_host preference was silently fatal. With civitai_host=civitai.red the update DB held zero prices even with tracking enabled. - civitai.red refuses non-browser HTTP clients outright (Cloudflare challenge, 403 for any User-Agent, aiohttp and httpx alike), while civitai.com and civitai.green answer normally for anonymously visible models and 404 for mature ones. An earlier manual check with curl passed on TLS fingerprint luck, which is why this was missed. - get_model_prices now tries the configured host first, then the others, and takes the first parseable payload. The host that worked is remembered, and a host that refuses outright is parked for 15 minutes so a library full of mature models does not pay three requests each; a 404 is model-specific and does not park the host. Links keep using the configured host, which is where the user's own browser has clearance. - Mature models still have no price source anywhere, so that is now stated instead of silent: price_check_attempted_at separates "tried and unreadable" from "never looked", gated versions show a muted "Price unavailable" badge, and the alerts panel reports unavailableCount. - Failures are logged at warning level, once per host per TTL, with the per-host reason, instead of only at debug level. - The recorded alternatives (internal tRPC with the user's API key, or an extension-assisted fetch from the user's browser) and the strengthened upstream ask for a public price field are documented in the plan.
This commit is contained in:
@@ -283,3 +283,96 @@ describe('ModelVersionsTab download button visibility', () => {
|
||||
expect(openFileSelectionForVersion).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('ModelVersionsTab price badges', () => {
|
||||
let getModelApiClient;
|
||||
let fetchModelUpdateVersions;
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.resetModules();
|
||||
document.body.innerHTML = `
|
||||
<div id="model-versions-modal">
|
||||
<div id="versions-tab">
|
||||
<div class="model-versions-tab"></div>
|
||||
</div>
|
||||
</div>
|
||||
`;
|
||||
({ getModelApiClient } = await import(API_FACTORY_MODULE));
|
||||
fetchModelUpdateVersions = vi.fn();
|
||||
getModelApiClient.mockReturnValue({
|
||||
fetchModelUpdateVersions,
|
||||
fetchModelRoots: vi.fn(),
|
||||
setModelUpdateIgnore: vi.fn(),
|
||||
setVersionUpdateIgnore: vi.fn(),
|
||||
deleteModel: vi.fn(),
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = '';
|
||||
});
|
||||
|
||||
function rowFor(versionId) {
|
||||
return document.querySelector(`.model-version-row[data-version-id="${versionId}"]`);
|
||||
}
|
||||
|
||||
it('shows the price for a gated version whose price was captured', async () => {
|
||||
fetchModelUpdateVersions.mockResolvedValue(buildRecord([
|
||||
{
|
||||
versionId: 20,
|
||||
name: 'Alpha',
|
||||
isInLibrary: false,
|
||||
shouldIgnore: false,
|
||||
isPaid: true,
|
||||
paidAccess: { permanent: true, endsAt: null },
|
||||
priceBuzz: 5000,
|
||||
listPriceBuzz: 5000,
|
||||
priceCheckedAt: 1791039694.5,
|
||||
priceAttemptedAt: 1791039694.5,
|
||||
},
|
||||
]));
|
||||
|
||||
await renderVersions();
|
||||
|
||||
expect(rowFor(20).textContent).toContain('5,000 Buzz');
|
||||
});
|
||||
|
||||
it('marks a gated version with no readable price as unavailable', async () => {
|
||||
// The mature-model case: we looked, no host would serve the page.
|
||||
fetchModelUpdateVersions.mockResolvedValue(buildRecord([
|
||||
{
|
||||
versionId: 21,
|
||||
name: 'Beta',
|
||||
isInLibrary: false,
|
||||
shouldIgnore: false,
|
||||
isPaid: true,
|
||||
paidAccess: { permanent: true, endsAt: null },
|
||||
priceBuzz: null,
|
||||
priceAttemptedAt: 1791039694.5,
|
||||
},
|
||||
]));
|
||||
|
||||
await renderVersions();
|
||||
|
||||
expect(rowFor(21).textContent).toContain('Price unavailable');
|
||||
});
|
||||
|
||||
it('stays quiet when the price was never looked up', async () => {
|
||||
fetchModelUpdateVersions.mockResolvedValue(buildRecord([
|
||||
{
|
||||
versionId: 22,
|
||||
name: 'Gamma',
|
||||
isInLibrary: false,
|
||||
shouldIgnore: false,
|
||||
isPaid: true,
|
||||
paidAccess: { permanent: true, endsAt: null },
|
||||
priceBuzz: null,
|
||||
priceAttemptedAt: null,
|
||||
},
|
||||
]));
|
||||
|
||||
await renderVersions();
|
||||
|
||||
expect(rowFor(22).textContent).not.toContain('Price unavailable');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -203,6 +203,21 @@ describe('UpdateService price alerts panel', () => {
|
||||
expect(document.querySelectorAll('#priceAlertsList .price-alert-item')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('reports how many gated versions have no readable price', async () => {
|
||||
global.fetch = vi.fn().mockResolvedValue(
|
||||
createFetchResponse(
|
||||
alertPayload([BELOW_THRESHOLD_ALERT], { unavailableCount: 3 })
|
||||
)
|
||||
);
|
||||
|
||||
await service.loadPriceAlerts({ force: true });
|
||||
|
||||
expect(service.priceAlertsUnavailableCount).toBe(3);
|
||||
const note = document.getElementById('priceAlertsStale');
|
||||
expect(note.classList.contains('hidden')).toBe(false);
|
||||
expect(note.textContent).toContain('3 paid version(s) have no readable price');
|
||||
});
|
||||
|
||||
it('keeps the last known list when the request fails', async () => {
|
||||
global.fetch = vi
|
||||
.fn()
|
||||
|
||||
@@ -2950,9 +2950,10 @@ def _price_alerts_adapter(update_service, scanners=None):
|
||||
|
||||
|
||||
class _FakeUpdateService:
|
||||
def __init__(self, alerts):
|
||||
def __init__(self, alerts, *, unavailable_count=0):
|
||||
self.alerts = alerts
|
||||
self.calls = []
|
||||
self.unavailable_count = unavailable_count
|
||||
|
||||
async def get_price_alerts(self, model_type=None, *, threshold_buzz=None, limit=200):
|
||||
self.calls.append((model_type, threshold_buzz, limit))
|
||||
@@ -2961,6 +2962,9 @@ class _FakeUpdateService:
|
||||
def newest_price_checked_at(self):
|
||||
return 1791039694.5
|
||||
|
||||
def count_unavailable_prices(self, model_type=None):
|
||||
return self.unavailable_count
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_price_alerts_handler_returns_the_global_list():
|
||||
@@ -2997,6 +3001,7 @@ async def test_price_alerts_handler_returns_the_global_list():
|
||||
assert payload["enabled"] is True
|
||||
assert payload["thresholdBuzz"] == 300
|
||||
assert payload["newestCheckedAt"] == 1791039694.5
|
||||
assert payload["unavailableCount"] == 0
|
||||
assert payload["alerts"][0]["civitaiUrl"] == (
|
||||
"https://civitai.com/models/2981320?modelVersionId=3379626"
|
||||
)
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import copy
|
||||
import pathlib
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
@@ -921,3 +922,104 @@ async def test_get_model_prices_propagates_rate_limit(downloader):
|
||||
|
||||
with pytest.raises(RateLimitError):
|
||||
await client.get_model_prices(7)
|
||||
|
||||
|
||||
# --- Model page host fallback -------------------------------------------------
|
||||
|
||||
_PAGE_FIXTURE = (
|
||||
pathlib.Path(__file__).resolve().parents[1]
|
||||
/ "utils"
|
||||
/ "fixtures"
|
||||
/ "civitai_model_page_paid.html"
|
||||
)
|
||||
|
||||
|
||||
def _page_html() -> str:
|
||||
return _PAGE_FIXTURE.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
async def test_get_model_prices_falls_back_when_the_preferred_host_refuses(
|
||||
downloader, monkeypatch
|
||||
):
|
||||
"""civitai.red refuses non-browser clients (Cloudflare); the price must still
|
||||
be readable from a host that answers. This is the user-visible bug: with
|
||||
civitai_host=civitai.red every fetch used to fail."""
|
||||
|
||||
client = await CivitaiClient.get_instance()
|
||||
monkeypatch.setattr(client, "_page_host", lambda: "civitai.red")
|
||||
seen = []
|
||||
|
||||
async def fake_make_request(method, url, use_auth=True, **kwargs):
|
||||
seen.append(url)
|
||||
if "civitai.red" in url:
|
||||
return False, "Access forbidden"
|
||||
return True, _page_html()
|
||||
|
||||
downloader.make_request = fake_make_request
|
||||
|
||||
result = await client.get_model_prices(4242)
|
||||
|
||||
assert result is not None
|
||||
assert result[1001]["price_buzz"] == 5000
|
||||
assert seen[0].startswith("https://civitai.red/")
|
||||
assert any("civitai.com" in url for url in seen)
|
||||
# The working host is remembered and the refusing one is parked.
|
||||
assert client._page_host_preference == "civitai.com"
|
||||
assert "civitai.red" in client._page_host_blocked
|
||||
|
||||
|
||||
async def test_get_model_prices_reuses_the_working_host(downloader, monkeypatch):
|
||||
client = await CivitaiClient.get_instance()
|
||||
monkeypatch.setattr(client, "_page_host", lambda: "civitai.red")
|
||||
calls = []
|
||||
|
||||
async def fake_make_request(method, url, use_auth=True, **kwargs):
|
||||
calls.append(url)
|
||||
if "civitai.red" in url:
|
||||
return False, "Access forbidden"
|
||||
return True, _page_html()
|
||||
|
||||
downloader.make_request = fake_make_request
|
||||
|
||||
await client.get_model_prices(4242)
|
||||
calls.clear()
|
||||
await client.get_model_prices(4242)
|
||||
|
||||
# Second time around the parked host is not retried.
|
||||
assert calls and all("civitai.red" not in url for url in calls)
|
||||
|
||||
|
||||
async def test_get_model_prices_returns_none_when_no_host_can_serve(downloader, monkeypatch):
|
||||
"""Mature models: 404 on civitai.com/green, challenge on civitai.red."""
|
||||
|
||||
client = await CivitaiClient.get_instance()
|
||||
monkeypatch.setattr(client, "_page_host", lambda: "civitai.com")
|
||||
|
||||
async def fake_make_request(method, url, use_auth=True, **kwargs):
|
||||
if "civitai.red" in url:
|
||||
return False, "Access forbidden"
|
||||
return False, "Resource not found"
|
||||
|
||||
downloader.make_request = fake_make_request
|
||||
|
||||
assert await client.get_model_prices(2981320) is None
|
||||
|
||||
|
||||
async def test_get_model_prices_does_not_park_a_host_on_404(downloader, monkeypatch):
|
||||
"""A 404 is model-specific (mature content hidden anonymously), not a reason
|
||||
to stop using the host for other models."""
|
||||
|
||||
client = await CivitaiClient.get_instance()
|
||||
monkeypatch.setattr(client, "_page_host", lambda: "civitai.com")
|
||||
|
||||
async def fake_make_request(method, url, use_auth=True, **kwargs):
|
||||
if "civitai.com" in url:
|
||||
return False, "Resource not found"
|
||||
return False, "Access forbidden"
|
||||
|
||||
downloader.make_request = fake_make_request
|
||||
|
||||
await client.get_model_prices(1)
|
||||
|
||||
assert "civitai.com" not in client._page_host_blocked
|
||||
assert "civitai.red" in client._page_host_blocked
|
||||
|
||||
@@ -1884,3 +1884,74 @@ async def test_newest_price_checked_at_reports_the_latest_fetch(tmp_path):
|
||||
|
||||
assert newest is not None
|
||||
assert newest > 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_failed_price_attempt_is_recorded_as_unavailable(tmp_path):
|
||||
"""A gated version we tried to price and could not must be distinguishable
|
||||
from one we never looked at — that is the honest "unavailable" state for
|
||||
mature models, whose pages no host will serve anonymously."""
|
||||
|
||||
service = _price_service(tmp_path, price_tracking_enabled=True)
|
||||
scanner = DummyScanner(LOCAL_RAW_DATA)
|
||||
failing = PriceProvider(GATED_RESPONSE, prices=None)
|
||||
|
||||
await service.refresh_for_model_type("lora", scanner, failing)
|
||||
record = await service.get_record("lora", 1)
|
||||
version = next(v for v in record.versions if v.version_id == 12)
|
||||
|
||||
assert failing.price_calls == 1
|
||||
assert version.price_buzz is None
|
||||
assert version.price_checked_at is None
|
||||
assert version.price_check_attempted_at is not None
|
||||
assert service.count_unavailable_prices("lora") == 1
|
||||
assert service.count_unavailable_prices("checkpoint") == 0
|
||||
# Nothing to alert on, and no price alert state.
|
||||
assert await service.get_price_alerts("lora") == []
|
||||
assert version.price_alert_state is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_successful_price_attempt_sets_both_markers(tmp_path):
|
||||
service = _price_service(tmp_path, price_tracking_enabled=True)
|
||||
scanner = DummyScanner(LOCAL_RAW_DATA)
|
||||
|
||||
await service.refresh_for_model_type(
|
||||
"lora", scanner, PriceProvider(GATED_RESPONSE, prices=PRICE_PAYLOAD)
|
||||
)
|
||||
record = await service.get_record("lora", 1)
|
||||
version = next(v for v in record.versions if v.version_id == 12)
|
||||
|
||||
assert version.price_buzz == 250
|
||||
assert version.price_checked_at is not None
|
||||
assert version.price_check_attempted_at is not None
|
||||
assert service.count_unavailable_prices("lora") == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_price_attempt_is_recorded_while_tracking_is_off(tmp_path):
|
||||
service = _price_service(tmp_path) # tracking off
|
||||
scanner = DummyScanner(LOCAL_RAW_DATA)
|
||||
|
||||
await service.refresh_for_model_type("lora", scanner, DummyProvider(GATED_RESPONSE))
|
||||
record = await service.get_record("lora", 1)
|
||||
|
||||
assert record.versions[0].price_check_attempted_at is None
|
||||
assert service.count_unavailable_prices("lora") == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unavailable_marker_clears_when_the_version_becomes_free(tmp_path):
|
||||
service = _price_service(tmp_path, price_tracking_enabled=True)
|
||||
scanner = DummyScanner(LOCAL_RAW_DATA)
|
||||
|
||||
await service.refresh_for_model_type(
|
||||
"lora", scanner, PriceProvider(GATED_RESPONSE, prices=None)
|
||||
)
|
||||
assert service.count_unavailable_prices("lora") == 1
|
||||
|
||||
await service.refresh_for_model_type("lora", scanner, DummyProvider(FREE_RESPONSE))
|
||||
record = await service.get_record("lora", 1)
|
||||
|
||||
assert record.versions[0].price_check_attempted_at is None
|
||||
assert service.count_unavailable_prices("lora") == 0
|
||||
|
||||
Reference in New Issue
Block a user