Repository navigation
Migliorie milestone: security, resilience, caching, docs & GPL v3 relicensing - #10
Merged
Merged
Conversation
- Remove dead Integrations/MQTT section from development settings (secret treated as compromised; rotation is a manual maintainer operation) - Fix NU1903: bump Microsoft.AspNetCore.OpenApi to 10.0.10 and pin Microsoft.OpenApi 2.7.5 (GHSA-v5pm-xwqc-g5wc) - Replace #if DEBUG/RELEASE config loading with the standard appsettings.{Environment}.json host mechanism; drop empty dev-only block; document Scalar/OpenAPI exposure in production as an explicit choice - Make WeatherFileStorage Enabled/CleanupEnabled explicit in committed settings - CI: fail on high/critical vulnerable packages (transitive included) - README: document secret-handling policy (User Secrets in dev, env vars in prod) Co-Authored-By: Claude Fable 5 <[email protected]>
- WebApplicationFactory-based harness: fake scraper swappable per-test, background jobs removed, clock pinned via FakeTimeProvider - Real app registers TimeProvider.System (call-site migration is ticket 08) - HTTP smoke tests asserting status + JSON payload, incl. empty scrapes - Parser tests call ParseDayPage directly (internal + InternalsVisibleTo) instead of reflection; duplicate HTML-fixture accessor removed Co-Authored-By: Claude Fable 5 <[email protected]>
Extract the Italian description -> WeatherType mapping from Meteo3bScraper
into Meteo3bWeatherTypeMapper, unit-tested directly:
- fix the "pioggere" typo so plural "piogge" phrases map to Rainy
- keep "possibili piogge" combos prioritized over the generic rain rule
(HeavyRain still wins, as before)
- map drizzle ("pioviggine", "pioggia debole") to LightRain
- log a warning with the original text when a description maps to Unknown
- pin documented precedence for "sereno con velature" and "vento forte"
- name the degraded-page reliability constant (DegradedPageReliabilityPerc)
Co-Authored-By: Claude Fable 5 <[email protected]>
Validate scraping and file-storage settings with DataAnnotations and ValidateOnStart so invalid configuration fails fast at startup with a clear message instead of killing the host mid-run or hammering the provider: - Job intervals restricted to [1, 1440]; required fields enforced. - WeatherScrapingSettings implements IValidatableObject to check that the default provider is among the enabled providers (case-insensitive, matching the resolver). - Wire both settings via AddOptions().Bind().ValidateDataAnnotations() .ValidateOnStart() in Infrastructure DI. - Integration factory now seeds EnabledProviders to satisfy the new cross-field rule. Tests: unit tests over the validation rules (out-of-range interval, missing/case-insensitive default provider, default-not-enabled, empty enabled list, happy path) plus startup integration tests proving the host refuses to boot on invalid config and boots as before on valid. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
- WeatherFileStorageJob writes each day file to a temp file in the target directory then atomically renames it over the destination, so a reading agent never observes a truncated JSON document. - Committed OutputPath default and docker-compose volume use the relative weather-data path; README documents the Windows absolute-path option. - End-to-end job tests (write, overwrite, cleanup cutoff) run against a temporary directory via an internal single-cycle seam. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Introduce domain exceptions (ProviderNotFoundException, LocationNotFoundException) and a GlobalExceptionHandler that maps them to ProblemDetails from a single place: unknown provider -> 400 with the list of valid providers, location-not-found -> 404, anything else -> an opaque 500 that never leaks internals. Remove the per-controller try/catch blocks for domain errors. The location-not-found *throw* path for empty forecasts is deferred to ticket 07; this ticket delivers the exception types, the handler and the provider-not-found mapping end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
A location with no data now returns 404 with ProblemDetails instead of an empty 200 cached for a day. - BaseWeatherScraper negative-caches empty scrapes with a short 5-min TTL (real forecasts keep the standard 24h TTL), so a failed scrape is retried within minutes rather than served empty for 24h. - Query handlers raise LocationNotFoundException on an empty forecast via a single shared GetForecastOrNotFoundAsync policy; the global handler maps it to 404. - Meteo3bScraper logs a warning (URL + reason) for each failed day-page fetch and now treats an HTTP timeout as a skipped day instead of aborting the whole scrape. - Tests: empty-scrape -> 404 (integration), negative-cache expiry -> re-scrape and TTL policy (scraper), timeout/HTTP-error skip-and-log, happy path. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Every now/today computation flows through the injected TimeProvider converted to the provider's timezone (Europe/Rome for 3bMeteo, declared per-provider on IWeatherProviderScraper), so a UTC host no longer shifts forecast days around midnight. Week/next-24h responses carry a top-level timezone field; raw DayWeather payloads carry it via Provider.TimeZoneId. Pinned-clock integration tests cover the midnight rollover, the evening slot ending at 00:00, and the 23:30 window. Co-Authored-By: Claude Fable 5 <[email protected]>
Malformed requests now fail fast with 400 ProblemDetails: numberOfDays is bounded to [1, 8] via ForecastLimits.MaxDays and every location route parameter is checked by ValidLocationAttribute against the domain rules in LocationName (letters, spaces, apostrophes, hyphens, max 100 chars). LocationName.Normalize (trim, lowercase, separator runs to a single hyphen) is applied once in BaseWeatherScraper and feeds both the cache key and the provider URL, so spaced and hyphenated spellings share one cache entry and one scrape; Meteo3bScraper dropped its private normalization. The 400 responses are documented in OpenAPI. The integration fake now extends BaseWeatherScraper so tests exercise the real cache pipeline and can count actual scrapes. Co-Authored-By: Claude Fable 5 <[email protected]>
Cache forecasts wrapped with the scrape time so LastUpdatedAt tells the
truth: a response served from cache reports the original scrape time, not
the moment it was served, and JSON files on disk carry the scrape time too.
- New ScrapedForecast envelope ({ ScrapedAt, Days }) is the cached value;
BaseWeatherScraper stamps ScrapedAt (via injected TimeProvider) at scrape
time and a forced refresh re-stamps it.
- Week and next-24h handlers and the file-storage job propagate
scraped.ScrapedAt instead of "now".
- GetWeatherForecast trims days into a new envelope, leaving the cached
forecast intact for later callers.
Co-Authored-By: Claude Fable 5 <[email protected]>
Attach the standard resilience handler to the Meteo3bScraper typed client so one slow or flaky provider page can no longer stall a scraping cycle: retry with backoff, circuit breaker, and a per-attempt timeout that abandons a hanging request. The timeout bound lives on the handler's attempt timeout (configurable via WeatherScraping:HttpTimeoutSeconds, default 15s) rather than HttpClient.Timeout, which would wrap the whole pipeline and preempt retries. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Protect the weather endpoints so a stranger can no longer exhaust memory or use the instance to bombard the provider: - Per-IP fixed-window rate limiter on the weather endpoints (configurable via the new RateLimitingSettings; exceeding the limit returns 429 with a ProblemDetails and Retry-After). Enabled by default with a generous 100/60s. - Opt-in location whitelist via WeatherScraping:AllowUnconfiguredLocations (default true = open, backwards compatible). When disabled, only configured locations are served; others are rejected with 403 before any scrape. The ServableLocationFilter matches on the normalized location, so spaced and hyphenated spellings both match. Integration tests cover the 429 path and the whitelist on/off / normalized paths. Full suite green (126 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Replace the DLR-based CQRS dispatch with compiled generic wrappers cached per concrete request type in a ConcurrentDictionary. No dynamic keyword remains in the dispatch path, a missing handler registration surfaces a clear typed InvalidOperationException, and endpoint behaviour is unchanged (the existing integration harness stays green). No external mediator library. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Decouple the wire contract from the domain model without changing what consumers parse: - WeatherType is now a real enum with a type-level JsonStringEnumConverter, so every value still serializes to its historical PascalCase string. The file-storage job's camelCase enum converter was removed (it would have lowercased the strings now that the property is an enum). - Multi-day and single-day endpoints return ForecastResponse / DayWeatherResponse (freshness + timezone envelope, consistent with the week/next-24h responses); no domain entity remains in a controller response signature. Envelope freshness/timezone come from a shared ForecastEnvelope so both entity-derived endpoints report them identically. - WeatherProvider drops its public setters (ProviderName get-only, TimeZoneId init-only). Home Assistant is unaffected: it consumes only /forecast/week, whose shape and weatherType strings are unchanged. New WeatherTypeSerializationTests snapshot the byte-identical strings. Full suite green (187 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Add a /health endpoint (liveness + a custom check verifying the last scraping cycle succeeded within a configurable window), a Docker HEALTHCHECK against it, and basic System.Diagnostics.Metrics counters (scrape success/failure, duration, Unknown-mapped slots). OpenTelemetry stays out of scope. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Forecast responses now carry a strong ETag derived from the data's scrape timestamp plus a private, no-cache Cache-Control, so a polling agent can revalidate with If-None-Match and get a 304 Not Modified while nothing has been re-scraped. The ETag rotates only on an actual re-scrape. Adds a `today` shortcut endpoint that returns the current day's weather in the provider's timezone (ticket 08), with a payload identical to requesting today's date explicitly. - IFreshnessStamped: shared freshness contract on the four response DTOs, giving the controller a single hook for ETag/Cache-Control. - CacheableOk helper on WeatherController, wired into every forecast GET. - GetTodayWeather query + handler, registered in Application DI. - Integration tests: 304, ETag rotation, timezone-honouring shortcut, and Cache-Control presence. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
- Read day-level reliability from where the API exposes it (DayForecast, not HourSlot) - Skip a forecast day missing its date (or otherwise malformed) with a log instead of failing the whole update - URL-encode the location so names with spaces work, via a shared forecast_week_url helper - Config flow sets a unique id (base-url + normalized location) to reject duplicate entries, and stores the normalized values - Use Home Assistant's shared aiohttp client session instead of a per-request session (coordinator + config flow) - Hourly forecast starts at the current hour: drop only fully-elapsed slots, keeping the in-progress one (handles the 18:00->00:00 wrap) Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Docs: - README: fix broken docs/jobs.md link and "Yu can consume" typo; point Docker instructions at the docker/ directory; refresh the env-var table (drop the obsolete storage interval, add the whitelist/timeout/rate-limit/health knobs). - Add docs/api.md documenting the API contract as implemented: timezone semantics, reliability meaning, LastUpdatedAt freshness, ETag/304, and 400/403/404/429 plus the unknown-location 404 / negative-cache behaviour. - Add docs/scraping.md: User-Agent choice, ToS/robots guidance, recommended minimum scraping interval. - Rewrite docs/jobs.md for the single merged scrape->persist cycle; align docs/docker.md. - Remove the redundant, stale appsettings.Template.json; drop the obsolete WeatherFileStorage__JobIntervalMinutes from docker/.env.template. Repository hygiene: - Add .editorconfig (C# conventions) and Directory.Build.props centralising shared build settings with the latest-recommended analyzer set enabled. - Uniform project files (Infrastructure/Test now consistent); strip csproj BOMs. - Build is warning-clean; documented suppressions (solution-wide CA1848/CA1873/1591, Infrastructure-scoped CA1051/CA1725, test-scoped CA1707/etc.). - Fix analyzer findings in scope: XmlReader for XXE-safe doc loading (CA5372), static wind-separator array (CA1861), redundant field initialisers removed (CA1805). CI: - pipeline.yml runs dotnet format --verify-no-changes and collects/publishes coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Replace the single 24h cache entry per location with two segments that have independent lifetimes: - current day: WeatherScraping:TodayCacheMinutes (default 30 min), because providers refresh today's data frequently; - following days: WeatherScraping:ExtendedCacheHours (default 6 h), because they change slowly; - empty scrape (unknown/unreachable location): configurable negative cache WeatherScraping:NegativeCacheMinutes (default 5 min). A cold request scrapes the whole week once and populates both segments with a single, consistent scrape time. When only the short-lived current-day entry expires, just that one page is re-scraped instead of the whole week; when the following-days segment expires, the whole week is re-scraped. lastUpdatedAt/ETag for a multi-day response reflect the most recent scrape among the days returned. ScrapeAsync now takes a day-offset range; the base scraper splits/merges the segments and seeds them separately from disk. Tests cover per-segment TTLs, current-day-only refresh, following-days expiry and configurable values. Docs (api.md, scraping.md), README, CHANGELOG and the docker env template updated accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Merges the
improvementsline intomaster: the full migliorie milestone (19 tickets, one commit each) plus the follow-up work on caching, Docker assets and documentation.The API surface stays backward compatible except where noted under Breaking changes.
106 files changed, ~6.7k insertions.Security & robustness
appsettings*.json; User Secrets in Development, environment variables in Production (dc13c13)ProblemDetailsfor every failure, no per-controllertry/catch(0378023)locationis validated and normalized, so equivalent spellings share one scrape/cache entry (2db7186)AllowUnconfiguredLocations = falseto restrict the API to configured locations (c2b4f4c)Correctness
TimeProvider&Europe/Romeprovider timezone — "today" and the next-24h window are computed in the provider's timezone, so they stay correct on a UTC host (f3bd670)404, not an empty200, and is negative-cached briefly instead of masked for hours (1a82583)lastUpdatedAtreports when the data was scraped, not when the response was produced (c674301)Architecture & API
dynamic— typed CQRS handler wrappers (1450ebf)WeatherTypeenum — domain entities no longer leak into responses (6ac034d)/healthturns unhealthy when the last successful scrape is older than a configurable window (d3c45aa)todayshortcut — strongETag,Cache-Control: private, no-cacheand304 Not ModifiedonIf-None-Match, so re-polling an unchanged forecast costs almost nothing (750a183)Integrations, tooling & docs
WebApplicationFactory+ fake scraper and fake clock; 15 new test files (0d61e94)Dockerfile, compose assets underdocker/, removed the VS-generated Dockerfile (842e205)docs/api.md,docs/scraping.md, refresheddocs/jobs.md/docs/docker.md,.editorconfigandDirectory.Build.propswith the recommended analyzer set; CI verifies formatting and publishes a coverage report (4c8e2e3)Breaking changes
WeatherFileStorage__JobIntervalMinutesno longer exists: file storage runs as the final step of the scraping cycle, so it is governed byWeatherScraping__JobIntervalMinutes.ReliabilityPercmoved from the hourly slot to the day level (DayWeather/DayForecastEntry), shipped in v1.1.0.Known follow-up (not in this PR)
The MQTT password committed in earlier history (commits
68eacb3,617f9cb) is still retrievable from the repository history. It must be rotated — removing it from the current files does not undo the exposure.Test plan
dotnet test— unit + integration suites (rate limiting, whitelist, health checks, timezone/clock, cache bootstrap, freshness, input validation, API conveniences)Pipelineworkflow: build,dotnet formatcheck, tests with coverage reportdocker compose up -dfromdocker/, thenGET /health,/scalar/v1and a forecast request withIf-None-Matchto confirm the304🤖 Generated with Claude Code