From 06fb1be79e1e46eff50a4354f117c9171bfc556c Mon Sep 17 00:00:00 2001 From: prbatero <42007693+prbatero@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:11:43 -0400 Subject: [PATCH 1/7] docs(perf): define app-wide loading targets Document the startup, publishing, map-loading, and route performance contract. Keep deployment gated on trusted Function ingress and authenticated Dev1 measurements. --- .../0005-session-bootstrap-and-revocation.md | 72 ++++++++++++++ spec/features/perf-app-wide-loading/README.md | 80 +++++++++++++++ .../perf-app-wide-loading/data-model.md | 45 +++++++++ spec/features/perf-app-wide-loading/design.md | 98 +++++++++++++++++++ .../perf-app-wide-loading/impact-analysis.md | 42 ++++++++ spec/features/perf-app-wide-loading/plan.md | 41 ++++++++ .../features/perf-app-wide-loading/results.md | 86 ++++++++++++++++ .../features/perf-app-wide-loading/rollout.md | 43 ++++++++ .../perf-app-wide-loading/test-plan.md | 61 ++++++++++++ .../perf-app-wide-loading/user-stories.md | 59 +++++++++++ 10 files changed, 627 insertions(+) create mode 100644 spec/architecture/decisions/0005-session-bootstrap-and-revocation.md create mode 100644 spec/features/perf-app-wide-loading/README.md create mode 100644 spec/features/perf-app-wide-loading/data-model.md create mode 100644 spec/features/perf-app-wide-loading/design.md create mode 100644 spec/features/perf-app-wide-loading/impact-analysis.md create mode 100644 spec/features/perf-app-wide-loading/plan.md create mode 100644 spec/features/perf-app-wide-loading/results.md create mode 100644 spec/features/perf-app-wide-loading/rollout.md create mode 100644 spec/features/perf-app-wide-loading/test-plan.md create mode 100644 spec/features/perf-app-wide-loading/user-stories.md diff --git a/spec/architecture/decisions/0005-session-bootstrap-and-revocation.md b/spec/architecture/decisions/0005-session-bootstrap-and-revocation.md new file mode 100644 index 00000000..fe81316c --- /dev/null +++ b/spec/architecture/decisions/0005-session-bootstrap-and-revocation.md @@ -0,0 +1,72 @@ +# ADR-0005: Session Bootstrap and Revocation + +**Status:** proposed +**Date:** 2026-09-02 +**Deciders:** prbatero + +## Contents + +- [Context](#context) +- [Options](#options) +- [Decision](#decision) +- [Consequences](#consequences) + +## Context + +The UI currently serializes SWA authentication, ACL loading, an Azure +management-plane user listing, an unconditional ACL rewrite, and publishing +provider discovery before rendering a route. Live dev1 telemetry shows this +shared path consumes about two seconds at the median and can exceed three +seconds. + +## Options + +### Keep Management-Plane Reconciliation on Every Login + +- Preserves immediate comparison with SWA user assignments. +- Adds latency, management-plane availability, and an unnecessary write to + every application load. + +### Use a Read-Only Session Bootstrap + +- Uses the trusted SWA principal and HASTE ACL in one API request. +- Removes stable-session writes and management-plane calls. +- Requires explicit out-of-band reconciliation for external revocation. + +## Decision + +Use a read-only `GetSessionBootstrap` endpoint for normal startup. The endpoint +accepts no identity input, decodes the SWA principal, loads current ACL state, +intersects trusted principal roles with ACL roles, and returns user settings +plus publishing capabilities. + +Stable active users are never written during bootstrap. Inactive, pending, or +deleted users receive a roleless blocked session and are never auto-reactivated. +Sensitive routes continue to authorize independently. Management-plane +reconciliation remains an explicit administrative workflow; no automated +revocation SLA is claimed by this change. + +Explicit reconciliation binds the SWA user object ID onto legacy email-only +ACL records. Once bound, runtime matching never falls back to email for that +record. + +Deployment is blocked until the Function runtime endpoint is restricted to the +trusted SWA/APIM path or independently validates a signed identity. The ingress +change was explicitly deferred during implementation review. + +### Components Affected + +| Component | Change | +|---|---| +| `hastegeo` | Plain-data session resolution logic | +| `hastefuncapi` | Thin bootstrap HTTP wrapper | +| React UI | Replace serial startup calls with one bootstrap call | + +## Consequences + +- Stable startup becomes one read-only request. +- Management-plane outages no longer block every page load. +- External SWA assignment changes reach the HASTE ACL only after explicit + administrative reconciliation. +- ACL state remains the deny-first runtime authority. +- No Azure resource, local-development service, or persistent schema changes. \ No newline at end of file diff --git a/spec/features/perf-app-wide-loading/README.md b/spec/features/perf-app-wide-loading/README.md new file mode 100644 index 00000000..f33e2e31 --- /dev/null +++ b/spec/features/perf-app-wide-loading/README.md @@ -0,0 +1,80 @@ +# App-Wide Loading Performance + +**Status:** in-progress +**Author:** prbatero +**Date:** 2026-09-02 +**Priority:** P1 + +## Contents + +- [Summary](#summary) +- [Measured Baseline](#measured-baseline) +- [Success Criteria](#success-criteria) +- [Components](#components) +- [Documents](#documents) + +## Summary + +Bring every HASTE route to useful content in about two seconds, with a hard +target band of one to three seconds. This work removes shared startup waits, +optimizes published-dataset reads and polling, overlaps map and route loading, +and adds deterministic route-level performance coverage. + +## Measured Baseline + +Application Insights for dev1 after deployment `1.0.40rc3` showed: + +| Operation | p50 | p95 | Finding | +|---|---:|---:|---| +| Global `GetUserById` | 1.17 s | 1.87 s | Serial startup dependency | +| Global `PutUser` | 0.87 s | 1.01 s | Unconditional startup write | +| `GetDashboardData` | 19 ms | 81 ms | Endpoint is already fast | +| `GetPublishedDatasets` | 0.98 s | 1.89 s | Leaves little UI budget | +| `GetProjectDetails` | 0.12 s | 2.27 s | Existing optimization remains | + +Cold Azure Maps assets added about 1.82 seconds before route code and data. +Non-map lazy-route JavaScript added at most 63 KiB gzip, so API and asset +waterfalls dominate bundle transfer. + +## Success Criteria + +- [ ] Stable authenticated startup uses one API request, no management-plane + user lookup, and no ACL write. +- [ ] Non-map routes reach useful content within 2 seconds at p50 and 3 seconds + at p95 in the dev1 browser matrix. +- [ ] Map routes show useful shell/progress within 2 seconds and usable map + controls within 3 seconds at p95 on a warm CDN cache. +- [ ] `GetPublishedDatasets` warm p95 is below 750 ms and conditional polls + return `304` when unchanged. +- [ ] Hidden tabs and in-flight requests do not start another poll. +- [ ] Every navigable route has deterministic cold/warm and direct/in-app + timing coverage. + +## Components + +| Component | Impact | +|---|---| +| `hastelib/src/hastegeo/core/` | Session bootstrap and bounded caches | +| `api/hastefuncapi/` | Thin bootstrap and conditional-list routes | +| `ui/src/` | Startup, route loading, polling, and progressive readiness | +| `spec/features/perf-app-wide-loading/` | Performance contract and results | + +No new Azure resources, dependencies, queues, or persistent schemas are added. + +## Documents + +| Document | Purpose | +|---|---| +| [design.md](design.md) | API, cache, route, and security design | +| [plan.md](plan.md) | Ordered implementation slices | +| [impact-analysis.md](impact-analysis.md) | Risk and rollback analysis | +| [user-stories.md](user-stories.md) | Acceptance criteria and agent mapping | +| [data-model.md](data-model.md) | Response and cache data shapes | +| [test-plan.md](test-plan.md) | Regression and performance matrix | +| [rollout.md](rollout.md) | Dev1 validation and rollback | +| [results.md](results.md) | Live baseline and validation status | + +## Related Specs + +- [Project layer-loading performance](../perf-layer-loading/README.md) +- [Session bootstrap and revocation ADR](../../architecture/decisions/0005-session-bootstrap-and-revocation.md) \ No newline at end of file diff --git a/spec/features/perf-app-wide-loading/data-model.md b/spec/features/perf-app-wide-loading/data-model.md new file mode 100644 index 00000000..ce77ff48 --- /dev/null +++ b/spec/features/perf-app-wide-loading/data-model.md @@ -0,0 +1,45 @@ +# Data Model: App-Wide Loading Performance + +## Contents + +- [Persistent Data](#persistent-data) +- [Bootstrap Response](#bootstrap-response) +- [Cache Keys](#cache-keys) +- [Migration](#migration) + +## Persistent Data + +No persistent schema, container, filesystem, queue, or Batch change is +introduced. Existing user ACL and published-dataset records remain compatible. + +## Bootstrap Response + +```json +{ + "user": { + "userId": "string", + "identityId": "string", + "userRoles": ["string"], + "settings": {}, + "status": "string" + }, + "publishing": { + "publishingEnabled": true, + "providers": [] + } +} +``` + +## Cache Keys + +| Data | Key | TTL | Invalidation | +|---|---|---:|---| +| Published dataset page | Caller plus normalized page, size, project, target, status, search, sort | <=5 s | Publishing mutations | +| Browser ETag | Same normalized query | Response lifetime | New `200` or mutation | + +Authorization state is never stored in these caches. + +## Migration + +No forward or backward migration is required. Rolling back discards +process-local caches and restores the legacy UI startup sequence. \ No newline at end of file diff --git a/spec/features/perf-app-wide-loading/design.md b/spec/features/perf-app-wide-loading/design.md new file mode 100644 index 00000000..800cf24f --- /dev/null +++ b/spec/features/perf-app-wide-loading/design.md @@ -0,0 +1,98 @@ +# Technical Design: App-Wide Loading Performance + +## Contents + +- [Architecture](#architecture) +- [Session Bootstrap](#session-bootstrap) +- [Published Datasets](#published-datasets) +- [Route Loading](#route-loading) +- [Security](#security) +- [Deferred Work](#deferred-work) + +## Architecture + +```text +SWA principal -> GetSessionBootstrap -> ACL processor -> bootstrap response +React route -> cached/conditional API reads -> route content +Map route -> route import || Maps CSS/control -> drawing || swipe -> map +``` + +Business logic lives under `hastegeo`; `function_app.py` remains a thin HTTP +boundary. Existing endpoints remain compatible during rollout. + +## Session Bootstrap + +### `GET /api/GetSessionBootstrap` + +The request accepts no identity parameters. It decodes the trusted SWA client +principal and returns: + +```json +{ + "user": { + "userId": "user@example.com", + "identityId": "entra-object-id", + "userRoles": ["contributors"], + "settings": {}, + "status": "Active" + }, + "publishing": { + "publishingEnabled": true, + "providers": [] + } +} +``` + +A stable active session performs one ACL read and zero writes. It does not list +SWA users through the Azure management plane. Existing inactive, pending, or +deleted users receive a blocked session with no roles so the UI can retain its +account-status page. The bootstrap response is not an authorization token; +sensitive routes retain their own checks. + +Pending invitations remain blocked until an administrator runs the explicit +user reconciliation workflow. Startup never writes the ACL or calls the Azure +management plane. + +## Published Datasets + +`GetPublishedDatasets` uses the existing bounded repository read behind a +process-local TTL/single-flight cache keyed by the normalized authenticated +query. The route emits an ETag and supports `If-None-Match`. Cache TTL is at +most five seconds; mutations invalidate the cache. + +The UI stores ETags by query, sends conditional requests, and polls only when a +visible page contains active work and no request is in flight. Polling never +overlaps and preserves the current query. + +## Route Loading + +Route module import begins at the same time as map asset loading. Map control +CSS and drawing CSS load in parallel with map-control JavaScript; drawing and +swipe JavaScript load in parallel only after map control is available. + +The application shell remains visible under Suspense. Data routes render a +stable loading state instead of an empty fragment. Help images use native lazy +loading and videos use `preload="none"`. + +Independent Home, create/edit, and validation requests run concurrently while +preserving required versus optional failure behavior. + +## Security + +- Identity comes only from the decoded SWA principal; no user ID is accepted + from query or body. +- ACL status and deletion state are checked on every bootstrap request. +- Client roles are intersected with ACL roles; role disagreement cannot grant + access. +- Stable sessions do not write user state or call the management plane. +- Caches store data representations, not authorization decisions. +- Development fallback remains restricted to `DEVELOPMENT_MODE`. + +## Deferred Work + +- Moving Blob container/access-policy initialization into deployment requires + separate SAS and provisioning coverage. +- A materialized publishing index is deferred unless cached p95 remains above + 1.5 seconds or the 1,000-record bound becomes material. +- Distributed caching, push updates, Function capacity changes, and new + dependencies are out of scope. \ No newline at end of file diff --git a/spec/features/perf-app-wide-loading/impact-analysis.md b/spec/features/perf-app-wide-loading/impact-analysis.md new file mode 100644 index 00000000..c54cbf97 --- /dev/null +++ b/spec/features/perf-app-wide-loading/impact-analysis.md @@ -0,0 +1,42 @@ +# Impact Analysis: App-Wide Loading Performance + +## Contents + +- [Scope](#scope) +- [Risks](#risks) +- [Security](#security) +- [Rollback](#rollback) + +## Scope + +| Component | Change | Severity | +|---|---|---| +| `hastegeo` | Session and representation cache logic | high | +| `hastefuncapi` | Backward-compatible bootstrap and ETag behavior | high | +| React UI | Startup, route readiness, polling, media | medium | +| Azure Functions/SWA | Existing deployments only; no new resource | low | + +## Risks + +| Risk | Impact | Mitigation | +|---|---|---| +| Cached authorization grants stale access | high | Never cache authorization decisions; load ACL per bootstrap | +| Concurrent ACL writes lose updates | high | Keep bootstrap read-only; retain explicit admin reconciliation and avoid startup writes | +| Publishing cache returns stale status | medium | TTL at most 5 seconds plus mutation invalidation | +| Parallel loading changes error order | medium | Preserve required/optional request semantics in tests | +| Map assets race their prerequisites | medium | Load control before drawing/swipe and cover failures | +| Browser budget varies by network | medium | Record cold/warm desktop/mobile profiles and API timing | + +## Security + +The bootstrap accepts no caller-controlled identity. It uses the decoded SWA +principal and current ACL state, and does not weaken authorization on any +sensitive endpoint. No secrets, CORS changes, public storage, or new roles are +introduced. + +## Rollback + +The change is fully reversible. Existing `GetUserById`, `PutUser`, and +`GetPublishingProviders` endpoints remain available, and the UI can revert to +the prior startup path. Caches are process-local and contain no durable state. +No data migration or Blob cleanup is required. \ No newline at end of file diff --git a/spec/features/perf-app-wide-loading/plan.md b/spec/features/perf-app-wide-loading/plan.md new file mode 100644 index 00000000..bd217eef --- /dev/null +++ b/spec/features/perf-app-wide-loading/plan.md @@ -0,0 +1,41 @@ +# Execution Plan: App-Wide Loading Performance + +## Contents + +- [Slices](#slices) +- [Exit Gates](#exit-gates) +- [Agent Summary](#agent-summary) + +## Slices + +| Slice | Task | Agent | Dependencies | Story | Status | +|---|---|---|---|---|---| +| 1 | Concurrent map/module loading, visible fallbacks, lazy help media | `ui` | Existing PR #189 | US-003 | implemented | +| 2 | Session bootstrap processor and thin API route | `backend-dev` | ADR-0005 | US-001 | implemented | +| 3 | UI bootstrap and independent request fan-out | `ui` | Slice 2 | US-001, US-003 | implemented | +| 4 | Published-dataset TTL/ETag cache and safe polling | `backend-dev`, `ui` | Slice 2 | US-002 | implemented | +| 5 | All-route deterministic performance matrix | `backend-dev`, `ui` | Slices 1-4 | US-004 | in-progress | + +Each slice is reviewable and testable independently. No infrastructure or +dependency changes are planned. + +## Exit Gates + +- [x] Stable startup: one API call and zero user writes. +- [x] Feature-specific core, API, and UI regression tests pass. +- [x] Full `hastelib`, API, queue, and UI suites pass. +- [x] UI lint for changed files and production build pass. +- [ ] Dev1 route matrix records cold/warm direct and in-app timings. +- [ ] Function runtime ingress is restricted to trusted SWA/APIM traffic. +- [ ] No route exceeds the three-second p95 acceptance limit without a + documented data-volume exception. + +## Agent Summary + +| Agent | Responsibility | +|---|---| +| `backend-dev` | Core session/cache logic and API wrappers | +| `backend-validation` | Core/API regression and contract validation | +| `ui` | Route, bootstrap, polling, and loading-state implementation | +| `ui-validation` | Browser route matrix and UI regressions | +| `orchestrator` | Track slice and spec status | \ No newline at end of file diff --git a/spec/features/perf-app-wide-loading/results.md b/spec/features/perf-app-wide-loading/results.md new file mode 100644 index 00000000..be511811 --- /dev/null +++ b/spec/features/perf-app-wide-loading/results.md @@ -0,0 +1,86 @@ +# App-Wide Performance Results + +## Contents + +- [Baseline](#baseline) +- [Implemented Changes](#implemented-changes) +- [Expected Impact](#expected-impact) +- [Local Verification](#local-verification) +- [Open Validation](#open-validation) + +## Baseline + +Application Insights for dev1 release `1.0.40rc3` supplied the server-side +baseline. Post-deployment request samples showed: + +| Endpoint | Samples | p50 | p95 | Maximum | +|---|---:|---:|---:|---:| +| `GetDashboardData` | 32 | 19 ms | 81 ms | 2.18 s | +| `GetModelCatalog` | 27 | 20 ms | 2.16 s | 2.61 s | +| `GetPublishedDatasets` | 6 | 0.98 s | 1.89 s | 1.89 s | +| `GetUserById` | 19 | 1.17 s | 1.87 s | 1.87 s | +| `PutUser` | 19 | 0.87 s | 1.01 s | 1.01 s | +| `GetProjectDetails` | 1,797 | 0.12 s | 2.27 s | 3.16 s | + +The legacy startup chain serialized `GetUserById`, `PutUser`, and +`GetPublishingProviders`. Cold Azure Maps asset loading took about 1.82 seconds +from the measurement host. Route JavaScript was not the dominant cost: lazy +route dependencies ranged from about 0.3 to 65 KiB gzip after the entry bundle. + +`GetModelArtifact` is a separate data-volume path. Over seven days, 24 +successful transfers had a 0.39-second median, 91.5-second p95, and 254-second +maximum. The Interactive Labeler downloads complete PMTiles and feature-sidecar +artifacts, so full map readiness cannot have a universal three-second limit. + +## Implemented Changes + +- One read-only `GetSessionBootstrap` call replaces stable-session user lookup, + user write, and provider discovery. +- Principal roles are intersected with active ACL roles; stable SWA object IDs + are bound during explicit admin reconciliation. +- Published dataset pages use a five-second bounded single-flight cache, + ETags, conditional requests, mutation invalidation, and non-overlapping + visible-tab polling. +- Route imports overlap Azure Maps loading. Independent Maps assets load in two + concurrent phases with retryable failures. +- Create/Edit Image Layer no longer loads Maps until the catalog drawer opens. +- Home, layer-form, validation, and Interactive Labeler requests overlap where + dependencies allow. +- Interactive Labeler PMTiles and sidecar transfers start concurrently and are + both required for readiness. +- Help images decode lazily and videos use `preload="none"`. +- Required route failures render retry actions instead of blank content. +- Route benchmarks require route-owned readiness markers, enforce p95 limits, + fail on browser/API errors, and omit authentication and fixture details. + +## Expected Impact + +The changes remove roughly two seconds of median server work from stable direct +startup and reduce cold map asset critical path from a serial sum to two +parallel phases. Published-list warm reads should become representation-cache +hits after authorization and return `304` when unchanged. + +These are expected effects, not post-deployment measurements. + +## Local Verification + +The final local regression pass completed with 601 core tests, 72 HTTP API +tests, 6 queue-trigger tests, and 148 UI tests passing. The production UI build +transformed 2,419 modules in 431 ms. + +Black, isort, and Flake8 passed for the nine feature-owned Python files. ESLint +passed for 38 changed UI files, both benchmark scripts passed Node syntax +checks, `git diff --check` passed, and the configured `detect-secrets` hook +reported no candidates. The Python suites emitted only existing Pydantic v2 +deprecation warnings. + +## Open Validation + +- Deploy only after trusted Function ingress is enforced. +- Run `tools/route_matrix.cjs` with an authenticated storage state outside the + repository and representative project/layer/model fixtures. +- Record desktop/mobile cold-direct, warm-direct, cold in-app, and warm in-app + results. +- Re-query Application Insights for bootstrap and published-list p50/p95. +- Treat Interactive Labeler shell/progress as the three-second route gate; + report complete artifact/map readiness against artifact byte size separately. \ No newline at end of file diff --git a/spec/features/perf-app-wide-loading/rollout.md b/spec/features/perf-app-wide-loading/rollout.md new file mode 100644 index 00000000..ef5ecf32 --- /dev/null +++ b/spec/features/perf-app-wide-loading/rollout.md @@ -0,0 +1,43 @@ +# Rollout Plan: App-Wide Loading Performance + +## Contents + +- [Strategy](#strategy) +- [Dev1 Validation](#dev1-validation) +- [Monitoring](#monitoring) +- [Rollback](#rollback) + +## Strategy + +Use phased deployment to the existing dev1 Function App and Static Web App. +Do not deploy `GetSessionBootstrap` until the public Function runtime endpoint +is restricted to the trusted SWA/APIM ingress path. That infrastructure change +was deferred and is not part of this branch. + +## Dev1 Validation + +1. Complete and validate the Function ingress prerequisite. +2. Deploy API and UI from the same tested commit. +3. Run the authenticated Playwright matrix across every route. +4. Compare Application Insights endpoint p50/p95 and request counts with the + `1.0.40rc3` baseline. +5. Hold for one normal usage cycle before wider deployment. + +Rollback if authentication failures rise, any route exceeds five seconds p95, +or publishing status freshness exceeds ten seconds. + +## Monitoring + +| Signal | Baseline | Gate | +|---|---:|---:| +| Bootstrap p95 | Legacy chain about 3 s | <1 s | +| Published datasets p95 | 1.89 s post-deploy | <0.75 s warm | +| Project details p95 | 2.27 s post-deploy | <=3 s | +| API failures | 0 for evaluated endpoints | No increase | +| Route content-ready p95 | Not previously measured | <=3 s | + +## Rollback + +Redeploy the previous API/UI commit together. Existing endpoints and data +remain compatible, and process-local caches disappear on restart. No persistent +data repair is required. \ No newline at end of file diff --git a/spec/features/perf-app-wide-loading/test-plan.md b/spec/features/perf-app-wide-loading/test-plan.md new file mode 100644 index 00000000..e8910455 --- /dev/null +++ b/spec/features/perf-app-wide-loading/test-plan.md @@ -0,0 +1,61 @@ +# Test Plan: App-Wide Loading Performance + +## Contents + +- [Test Strategy](#test-strategy) +- [Regression Matrix](#regression-matrix) +- [Performance Matrix](#performance-matrix) +- [Sign-Off](#sign-off) + +## Test Strategy + +| Level | Scope | Tool | Target | +|---|---|---|---| +| Unit | Session, cache, route helpers | `unittest`, Node test runner | Branch coverage for state transitions | +| API | Bootstrap and conditional list routes | Azure Functions test harness | Exact status/body/header contracts | +| UI | Startup, loading, ETag, polling | Node tests | Deterministic promise and timer control | +| Browser | Every route | Playwright | Cold/warm direct and in-app timings | + +## Regression Matrix + +| ID | Scenario | Expected | +|---|---|---| +| BOOT-01 | Stable active principal | One ACL read; no write or management call | +| BOOT-02 | Deleted/inactive principal | Roleless status response; no reactivation | +| BOOT-03 | Role mismatch | Least-privilege role intersection | +| PUB-01 | Concurrent identical list requests | One repository read per process | +| PUB-02 | Matching ETag | Empty `304` response | +| PUB-03 | Mutation then list | Cache invalidated | +| POLL-01 | Hidden tab | No poll | +| POLL-02 | Request in flight | No overlapping poll | +| MAP-01 | Cold map route | Module and map loading overlap | +| MAP-02 | Asset failure then retry | Loader resets and retries safely | +| HELP-01 | Help route | Images lazy; videos do not preload | + +## Performance Matrix + +For each route, record direct cold, direct warm, in-app cold, and in-app warm +on desktop and mobile profiles. Capture shell-ready, content-ready, API time, +map-ready, request count, transferred bytes, and failures. + +| Route class | p50 goal | p95 limit | +|---|---:|---:| +| Non-map data route | 2 s | 3 s | +| Static/help/admin route | 1 s | 2 s | +| Map route shell | 2 s | 3 s | +| Map controls, warm CDN | 2 s | 3 s | + +The Interactive Labeler shell and progress surface use the three-second route +gate. Complete readiness is reported separately by PMTiles/sidecar byte size; +the measured artifact proxy p95 exceeds the universal route budget. + +Synthetic fixtures must contain projects, models, labels, validation records, +published datasets, and active/terminal jobs. Tests do not call partner APIs. + +## Sign-Off + +- [x] Focused tests pass after each slice. +- [x] Full backend, API, queue, and UI tests pass. +- [x] Changed-file lint and production build pass. +- [ ] CI security checks pass. +- [ ] Dev1 route matrix is recorded with no unexplained p95 over 3 seconds. \ No newline at end of file diff --git a/spec/features/perf-app-wide-loading/user-stories.md b/spec/features/perf-app-wide-loading/user-stories.md new file mode 100644 index 00000000..5b46b729 --- /dev/null +++ b/spec/features/perf-app-wide-loading/user-stories.md @@ -0,0 +1,59 @@ +# User Stories: App-Wide Loading Performance + +## Contents + +- [Stories](#stories) +- [Agent Assignment Map](#agent-assignment-map) +- [Out of Scope](#out-of-scope) + +## Stories + +### US-001: Fast Stable Session Startup + +**As a** HASTE user, **I want** the application shell and my route to load +without redundant identity writes, **so that** every direct navigation starts +quickly. + +**Acceptance criteria:** A stable active user causes one bootstrap API request, +zero management-plane calls, and zero ACL writes. Inactive, pending, or deleted +users receive no application roles and cannot reach protected routes. + +### US-002: Responsive Published Dataset Tracking + +**As a** contributor, **I want** published datasets to load and refresh without +repeated full reads, **so that** I can track work without page stalls. + +**Acceptance criteria:** Same-query requests coalesce, unchanged conditional +requests return `304`, and polling stops while hidden or in flight. + +### US-003: Progressive Route Readiness + +**As a** disaster analyst, **I want** each route to show useful progress while +its data or maps load, **so that** navigation never appears frozen. + +**Acceptance criteria:** Route and map assets overlap, the application shell +remains visible, independent requests overlap, and help media loads on demand. + +### US-004: Enforced Route Performance Budget + +**As a** maintainer, **I want** deterministic route timings, **so that** future +changes cannot silently regress the one-to-three-second target. + +**Acceptance criteria:** Every route has cold/warm direct and in-app timing, +request counts, asset bytes, and content-ready evidence. + +## Agent Assignment Map + +| Story | Implementing Agent(s) | Validating Agent(s) | +|---|---|---| +| US-001 | `backend-dev`, `ui` | `backend-validation`, `ui-validation` | +| US-002 | `backend-dev`, `ui` | `backend-validation`, `ui-validation` | +| US-003 | `ui` | `ui-validation` | +| US-004 | `backend-dev`, `ui` | `backend-validation`, `ui-validation` | + +## Out of Scope + +- New Azure services or Function capacity changes. +- Distributed cache or push-notification transport. +- Persistent publishing index until bounded cached reads are remeasured. +- Blob policy provisioning changes without separate SAS regression coverage. \ No newline at end of file From 39c3d0630c66ce5268989c7456e4ee756d91b51b Mon Sep 17 00:00:00 2001 From: prbatero <42007693+prbatero@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:11:50 -0400 Subject: [PATCH 2/7] feat(api): optimize session and publishing reads Replace stable startup reconciliation with a read-only ACL bootstrap, enforce strong principal binding, and restrict legacy user reads. Add bounded single-flight Published Datasets caching with ETags and mutation invalidation. --- api/hastefuncapi/README.md | 16 + api/hastefuncapi/function_app.py | 343 +++++++++++++----- .../tests/test_publishing_routes.py | 223 +++++++++++- .../tests/test_session_bootstrap_route.py | 194 ++++++++++ .../tests/test_user_route_security.py | 211 +++++++++++ docs/api/hastefuncapi.md | 9 + hastelib/src/hastegeo/core/models/session.py | 23 ++ .../src/hastegeo/core/processors/session.py | 202 +++++++++++ .../src/hastegeo/core/utils/async_cache.py | 11 + .../tests/core/processors/test_session.py | 279 ++++++++++++++ hastelib/tests/core/utils/test_async_cache.py | 39 ++ 11 files changed, 1450 insertions(+), 100 deletions(-) create mode 100644 api/hastefuncapi/tests/test_session_bootstrap_route.py create mode 100644 api/hastefuncapi/tests/test_user_route_security.py create mode 100644 hastelib/src/hastegeo/core/models/session.py create mode 100644 hastelib/src/hastegeo/core/processors/session.py create mode 100644 hastelib/tests/core/processors/test_session.py diff --git a/api/hastefuncapi/README.md b/api/hastefuncapi/README.md index cfd5bac2..705735ab 100644 --- a/api/hastefuncapi/README.md +++ b/api/hastefuncapi/README.md @@ -89,6 +89,7 @@ These endpoints use `FUNCTION`-level auth regardless of development mode (intend | Method | Route | Description | |--------|-------|-------------| +| GET | `GetSessionBootstrap` | Trusted current-user, role, settings, and publishing capabilities for one-call application startup. Accepts no caller identity parameters. | | GET | `GetUsers` | All users. Requires `administrators` role. | | GET | `GetUserById` | Single user by `userId`. | | PUT | `PutUser` | Create or update a user. Handles invitations, reinvitations, role assignment, and reactivation. | @@ -96,6 +97,21 @@ These endpoints use `FUNCTION`-level auth regardless of development mode (intend | GET | `GetAdminSettings` | All admin settings. Requires `administrators` role. | | PUT | `PutAdminSettings` | Update admin settings. Requires `administrators` role. | +#### Session Bootstrap + +`GetSessionBootstrap` resolves identity from the SWA client-principal header, +loads current HASTE ACL state, and returns user and publishing configuration in +one response. Stable active sessions are read-only; inactive, pending, and +deleted accounts receive no application roles. + +### Published Datasets + +`GetPublishedDatasets` returns `ETag`, `Cache-Control`, and `X-Haste-Cache` +headers. Send `If-None-Match` to receive an empty `304` for an unchanged fresh +representation. The bounded process-local cache expires within five seconds, +deduplicates concurrent identical reads, and is invalidated after publishing +mutations. + ### Utilities | Method | Route | Description | diff --git a/api/hastefuncapi/function_app.py b/api/hastefuncapi/function_app.py index 39fa3bdc..054cb80a 100644 --- a/api/hastefuncapi/function_app.py +++ b/api/hastefuncapi/function_app.py @@ -54,6 +54,14 @@ PublishingSizeLimitError, PublishingStateConflictError, ) +from hastegeo.core.processors.session import ( + SessionAccessError, + SessionBootstrapProcessor, + bind_swa_object_id, + effective_application_roles, + find_principal_user, + index_unique_aad_users, +) from hastegeo.core.processors.stats import StatsPreProcessor from hastegeo.core.processors.train import TrainPreprocessor from hastegeo.core.processors.uploader import FileUploader @@ -122,6 +130,16 @@ ttl_seconds=_PROJECT_DETAILS_CACHE_SECONDS, max_entries=_PROJECT_DETAILS_CACHE_ENTRIES, ) +_PUBLISHED_DATASETS_CACHE_SECONDS = configured_cache_value( + "HASTE_PUBLISHED_DATASETS_CACHE_SECONDS", 5, 0, 5 +) +_PUBLISHED_DATASETS_CACHE_ENTRIES = configured_cache_value( + "HASTE_PUBLISHED_DATASETS_CACHE_ENTRIES", 128, 1, 512 +) +_published_datasets_cache = AsyncTTLCache( + ttl_seconds=_PUBLISHED_DATASETS_CACHE_SECONDS, + max_entries=_PUBLISHED_DATASETS_CACHE_ENTRIES, +) # Development mode check - when running locally with Docker/Azurite # Set DEVELOPMENT_MODE=true to disable function key authentication @@ -226,32 +244,19 @@ def _decode_client_principal(req: func.HttpRequest) -> dict | None: return None -def _require_roles( +async def _require_roles( req: func.HttpRequest, allowed_roles: set[str] ) -> func.HttpResponse | None: """Enforce identity and role checks for privileged operations.""" if DEVELOPMENT_MODE: return None - principal = _decode_client_principal(req) - if principal is None: - return func.HttpResponse( - "Forbidden. Missing caller identity.", status_code=403 - ) - - user_id = principal.get("userId") or principal.get("userDetails") - if not user_id: - return func.HttpResponse( - "Forbidden. Missing caller identity.", status_code=403 - ) - - raw_roles = principal.get("userRoles") - roles = ( - {role.lower().strip() for role in raw_roles if isinstance(role, str)} - if isinstance(raw_roles, list) - else set() - ) - if not roles.intersection({role.lower() for role in allowed_roles}): + caller, auth_error = await _get_active_publishing_caller(req) + if auth_error: + return auth_error + if not caller["roles"].intersection( + {role.lower() for role in allowed_roles} + ): return func.HttpResponse( "Forbidden. Administrator role required.", status_code=403 ) @@ -260,7 +265,7 @@ def _require_roles( async def _get_active_publishing_caller( - req: func.HttpRequest, + req: func.HttpRequest, raw_users: list[dict] | None = None ) -> tuple[dict | None, func.HttpResponse | None]: """Return the trusted active HASTE caller used by publishing routes.""" principal = _decode_client_principal(req) @@ -282,6 +287,7 @@ async def _get_active_publishing_caller( ) return { "id": str(caller_id).lower(), + "user_id": str(principal.get("userDetails") or caller_id).lower(), "roles": roles, "name": principal.get("userDetails"), }, None @@ -298,47 +304,51 @@ async def _get_active_publishing_caller( "UNAUTHENTICATED", "Authentication is required.", 401 ) - try: - raw_users = await asyncio.to_thread( - MetadataProcessor( - data_type=config.get_metadata_types().USERS.value - ).load, - "acl", - ) - except FileNotFoundError: + if raw_users is None: + try: + raw_users = await asyncio.to_thread( + MetadataProcessor( + data_type=config.get_metadata_types().USERS.value + ).load, + "acl", + ) + except FileNotFoundError: + return None, _publishing_error_response( + "FORBIDDEN", "An active HASTE user is required.", 403 + ) + + active_user = find_principal_user( + raw_users, + str(principal_id or ""), + str(user_details or ""), + ) + if ( + active_user is None + or active_user.status != config.get_user_statuses().ACTIVE.value + or active_user.deleted + ): return None, _publishing_error_response( "FORBIDDEN", "An active HASTE user is required.", 403 ) - users = [User(**user) for user in raw_users] - active_user = next( - ( - user - for user in users - if ( - user.userId in {principal_id, user_details} - or user.objectId == principal_id - ) - and user.status == config.get_user_statuses().ACTIVE.value - and not user.deleted - ), - None, + roles = effective_application_roles( + principal.get("userRoles"), active_user.userRoles ) - if active_user is None: + if not roles: return None, _publishing_error_response( - "FORBIDDEN", "An active HASTE user is required.", 403 + "FORBIDDEN", "No active HASTE role is assigned.", 403 ) - - roles = { - role.lower().strip() - for role in principal.get("userRoles", []) - if isinstance(role, str) - } caller_id = principal_id or user_details # Persist the email/login as the publisher identifier, never the display # name (privacy: display names are resolved from Entra at read time). return { "id": str(caller_id).lower(), + "user_id": str( + active_user.userId + or active_user.email + or user_details + or caller_id + ).lower(), "roles": roles, "name": (active_user.email or user_details), }, None @@ -418,6 +428,44 @@ def _publishing_mutation_authorized(caller: dict) -> bool: ) +@app.route( + route="GetSessionBootstrap", + auth_level=AUTH_LEVEL, + methods=["GET"], +) +async def GetSessionBootstrap(req: func.HttpRequest) -> func.HttpResponse: + principal = _decode_client_principal(req) + if principal is None and DEVELOPMENT_MODE: + principal = { + "userId": "development@local", + "userDetails": "development@local", + "userRoles": ["authenticated", "administrators"], + } + if principal is None: + return _publishing_error_response( + "UNAUTHENTICATED", "Authentication is required.", 401 + ) + + try: + result = await asyncio.to_thread( + SessionBootstrapProcessor( + config=config, + development_mode=DEVELOPMENT_MODE, + ).load, + principal, + ) + return _publishing_json_response(result.model_dump(mode="json")) + except SessionAccessError as error: + return _publishing_error_response("FORBIDDEN", str(error), 403) + except Exception as error: + logger.error( + f"GetSessionBootstrap failed: {error}\n{traceback.format_exc()}" + ) + return _publishing_error_response( + "INTERNAL_ERROR", "Session bootstrap failed.", 500 + ) + + def _publishing_processor() -> PublishingProcessor: return PublishingProcessor(config=config) @@ -1617,7 +1665,7 @@ async def GetAdminSettings(req: func.HttpRequest) -> func.HttpResponse: logger.info( "GetAdminSettings HTTP trigger function processed a request. To get Config data from MetadataProcessor." ) - auth_error = _require_roles(req, {"administrators"}) + auth_error = await _require_roles(req, {"administrators"}) if auth_error: return auth_error try: @@ -1650,7 +1698,7 @@ async def PutAdminSettings(req: func.HttpRequest) -> func.HttpResponse: logger.info( "PutAdminSettings HTTP trigger function processed a request. To save Config data to MetadataProcessor." ) - auth_error = _require_roles(req, {"administrators"}) + auth_error = await _require_roles(req, {"administrators"}) if auth_error: return auth_error try: @@ -1687,7 +1735,7 @@ async def GetUsers(req: func.HttpRequest) -> func.HttpResponse: from hastegeo.core.utils.user import UserManager logger.info("GetUsers HTTP trigger function processed a request.") - auth_error = _require_roles(req, {"administrators"}) + auth_error = await _require_roles(req, {"administrators"}) if auth_error: return auth_error # Define state transition rules @@ -1745,15 +1793,23 @@ async def GetUsers(req: func.HttpRequest) -> func.HttpResponse: User(**user).dict() for user in users ] # To ensure defaults are applied to legacy entries app_users = await asyncio.to_thread(UserManager().list_users) - app_users_dict = { - user.display_name: {"provider": user.provider, "roles": user.roles} - for user in app_users - } + app_users_dict = index_unique_aad_users( + [ + { + "login": getattr(user, "user_details", None) + or getattr(user, "display_name", None), + "provider": getattr(user, "provider", None), + "roles": getattr(user, "roles", None) or "", + "objectId": getattr(user, "user_id", None) + or getattr(user, "id", None), + } + for user in app_users + ] + ) for user in users: - # user = User(**user).dict() - app_user = app_users_dict.get(user["userId"]) + app_user = app_users_dict.get(user["userId"].casefold()) # Determine transition parameters - app_user_exists = app_user is not None + app_user_exists = bind_swa_object_id(user, app_user) roles_match = ( sorted(filter_roles(user["userRoles"])) == sorted(filter_roles(app_user["roles"].split(","))) @@ -1798,8 +1854,6 @@ async def GetUsers(req: func.HttpRequest) -> func.HttpResponse: @app.route(route="PutUser", auth_level=AUTH_LEVEL, methods=["PUT"]) async def PutUser(req: func.HttpRequest) -> func.HttpResponse: - from hastegeo.core.utils.user import InvitationManager - logger.info("PutUser HTTP trigger function processed a request.") try: req_body = req.get_json() @@ -1813,6 +1867,9 @@ async def PutUser(req: func.HttpRequest) -> func.HttpResponse: is_admin = DEVELOPMENT_MODE if not DEVELOPMENT_MODE: principal = _decode_client_principal(req) + caller, auth_error = await _get_active_publishing_caller(req) + if auth_error: + return auth_error caller_email = ( (principal or {}).get("userDetails") or (principal or {}).get("userId") @@ -1822,15 +1879,13 @@ async def PutUser(req: func.HttpRequest) -> func.HttpResponse: return func.HttpResponse( "Forbidden. Missing caller identity.", status_code=403 ) - raw_roles = (principal or {}).get("userRoles") - caller_roles = ( - {r.lower().strip() for r in raw_roles if isinstance(r, str)} - if isinstance(raw_roles, list) - else set() - ) - is_admin = "administrators" in caller_roles - target_email = (input.email or input.userId or "").lower() - is_self = bool(target_email) and caller_email == target_email + is_admin = "administrators" in caller["roles"] + target_identifiers = { + value.lower() for value in (input.userId, input.email) if value + } + is_self = bool(target_identifiers) and target_identifiers == { + caller_email + } if not is_admin and not (action == "update" and is_self): return func.HttpResponse( "Forbidden. Administrator role required.", @@ -1853,6 +1908,8 @@ async def PutUser(req: func.HttpRequest) -> func.HttpResponse: async def send_invitation( email: str, roles: list[str], delete_existing: bool = False ) -> None: + from hastegeo.core.utils.user import InvitationManager + invites = await asyncio.to_thread( InvitationManager( email, roles, delete_existing=delete_existing @@ -1876,6 +1933,25 @@ def roles_changed( ) user_exists = user_index is not None + if not is_admin: + if not user_exists: + return func.HttpResponse( + "Forbidden. Existing active user required.", + status_code=403, + ) + existing_self = users[user_index] + active_status = config.get_user_statuses().ACTIVE.value + if ( + existing_self.deleted + or existing_self.status != active_status + or (existing_self.userId or "").lower() != caller_email + or (existing_self.email or "").lower() != caller_email + ): + return func.HttpResponse( + "Forbidden. Existing active user required.", + status_code=403, + ) + if not user_exists: # Create new user await send_invitation(input.email, input.userRoles) @@ -1995,7 +2071,7 @@ async def DeleteUser(req: func.HttpRequest) -> func.HttpResponse: from hastegeo.core.utils.user import UserManager logger.info("DeleteUser HTTP trigger function processed a request.") - auth_error = _require_roles(req, {"administrators"}) + auth_error = await _require_roles(req, {"administrators"}) if auth_error: return auth_error try: @@ -2046,20 +2122,33 @@ async def DeleteUser(req: func.HttpRequest) -> func.HttpResponse: @app.route(route="GetUserById", auth_level=AUTH_LEVEL, methods=["GET"]) async def GetUserById(req: func.HttpRequest) -> func.HttpResponse: - from hastegeo.core.utils.user import UserManager - logger.info("GetUser HTTP trigger function processed a request.") try: - user_id = req.params.get("userId") - users = await asyncio.to_thread( + user_id = _require_email_param(req, "userId") + raw_users = await asyncio.to_thread( MetadataProcessor( data_type=config.get_metadata_types().USERS.value ).load, "acl", ) - users = [User(**user) for user in users] + if not DEVELOPMENT_MODE: + caller, auth_error = await _get_active_publishing_caller( + req, raw_users=raw_users + ) + if auth_error: + return auth_error + is_self = caller["user_id"] == user_id.casefold() + if not is_self and "administrators" not in caller["roles"]: + return func.HttpResponse("Forbidden.", status_code=403) + + users = [User(**user) for user in raw_users] existing_user = next( - (user for user in users if user.userId == user_id), None + ( + user + for user in users + if user.userId and user.userId.casefold() == user_id.casefold() + ), + None, ) # In development mode, auto-create user if not found @@ -2099,6 +2188,8 @@ async def GetUserById(req: func.HttpRequest) -> func.HttpResponse: json.dumps(existing_user.dict()), status_code=200 ) + from hastegeo.core.utils.user import UserManager + app_user = await asyncio.to_thread( UserManager().find_user_by_email, user_id ) @@ -2174,6 +2265,8 @@ async def GetUserById(req: func.HttpRequest) -> func.HttpResponse: json.dumps(existing_user.dict()), status_code=200 ) + except ValueError as e: + return _bad_request(str(e)) except FileNotFoundError as e: logger.error(f"User not found: {e}\n{traceback.format_exc()}") return func.HttpResponse("User not found.", status_code=404) @@ -4656,28 +4749,75 @@ async def GetPublishedDatasets(req: func.HttpRequest) -> func.HttpResponse: if req.params.get("status") else None ) - records, total_count = await asyncio.to_thread( - PublishingRepository(config=config).list_page, - page=page, - page_size=page_size, - project_id=project_id, - target=target, - status=status, - search=search, - sort_key=req.params.get("sortKey", "publishedDate"), - sort_direction=req.params.get("sortDirection", "desc"), - ) - return _publishing_json_response( - { - "publishedDatasets": [ - record.model_dump(mode="json") for record in records - ], - "pagination": { - "page": page, - "pageSize": page_size, - "totalCount": total_count, - }, + sort_key = req.params.get("sortKey", "publishedDate") + sort_direction = req.params.get("sortDirection", "desc") + + async def load_response() -> dict: + records, total_count = await asyncio.to_thread( + PublishingRepository(config=config).list_page, + page=page, + page_size=page_size, + project_id=project_id, + target=target, + status=status, + search=search, + sort_key=sort_key, + sort_direction=sort_direction, + ) + payload = json.dumps( + { + "publishedDatasets": [ + record.model_dump(mode="json") for record in records + ], + "pagination": { + "page": page, + "pageSize": page_size, + "totalCount": total_count, + }, + } + ) + return { + "payload": payload, + "etag": '"' + + hashlib.sha256(payload.encode()).hexdigest()[:32] + + '"', } + + cache_key = ( + str(caller["id"]).lower(), + page, + page_size, + project_id or "", + target.value if target else "", + status.value if status else "", + search.lower(), + sort_key, + sort_direction, + ) + ( + cached_response, + cache_hit, + ) = await _published_datasets_cache.get_or_create( + cache_key, + load_response, + refresh=_cache_refresh_requested(req.headers.get("Cache-Control")), + ) + headers = { + "Cache-Control": ( + f"private, max-age={_PUBLISHED_DATASETS_CACHE_SECONDS}" + ), + "ETag": cached_response["etag"], + "X-Haste-Cache": "HIT" if cache_hit else "MISS", + } + if _etag_matches( + req.headers.get("If-None-Match"), cached_response["etag"] + ): + return func.HttpResponse(status_code=304, headers=headers) + return func.HttpResponse( + cached_response["payload"], + status_code=200, + mimetype="application/json", + headers=headers, ) except Exception as error: return _publishing_exception_response(error) @@ -4759,6 +4899,7 @@ async def PutPublishDatasetQueueMessage( prepared, assessment_summary, ) + await _published_datasets_cache.invalidate() return _publishing_json_response( {"publishedDataset": record.model_dump(mode="json")}, 202 ) @@ -4790,6 +4931,7 @@ async def PutRetryPublishedDatasetQueueMessage( caller["id"], "administrators" in caller["roles"], ) + await _published_datasets_cache.invalidate() return _publishing_json_response( {"publishedDataset": record.model_dump(mode="json")}, 202 ) @@ -4830,6 +4972,7 @@ async def PutUpdatePublishedDataset( "administrators" in caller["roles"], fields, ) + await _published_datasets_cache.invalidate() return _publishing_json_response( {"publishedDataset": record.model_dump(mode="json")}, 200 ) @@ -4856,6 +4999,7 @@ async def DeletePublishedDataset(req: func.HttpRequest) -> func.HttpResponse: caller["id"], "administrators" in caller["roles"], ) + await _published_datasets_cache.invalidate() return _publishing_json_response( {"publishedDataset": record.model_dump(mode="json")}, 202 ) @@ -4888,6 +5032,7 @@ async def ForceRemovePublishedDataset( caller["id"], "administrators" in caller["roles"], ) + await _published_datasets_cache.invalidate() return _publishing_json_response( {"publishedDataset": record.model_dump(mode="json")}, 200 ) diff --git a/api/hastefuncapi/tests/test_publishing_routes.py b/api/hastefuncapi/tests/test_publishing_routes.py index 0abe3fb6..ba8fe124 100644 --- a/api/hastefuncapi/tests/test_publishing_routes.py +++ b/api/hastefuncapi/tests/test_publishing_routes.py @@ -1,3 +1,4 @@ +import asyncio import base64 import io import json @@ -8,6 +9,7 @@ from unittest.mock import AsyncMock, Mock, patch import azure.functions as func +from hastegeo.core.utils.async_cache import AsyncTTLCache os.environ.setdefault("DEVELOPMENT_MODE", "true") os.environ.setdefault("PUBLISHING_ENABLED", "true") @@ -79,6 +81,19 @@ def make_dataset(status: str = "PENDING") -> PublishedDataset: class TestPublishingRoutes(unittest.IsolatedAsyncioTestCase): + async def asyncSetUp(self) -> None: + self.catalog_cache = AsyncTTLCache(ttl_seconds=5, max_entries=16) + self.cache_patcher = patch.object( + function_app, + "_published_datasets_cache", + self.catalog_cache, + ) + self.cache_patcher.start() + + async def asyncTearDown(self) -> None: + await self.catalog_cache.clear() + self.cache_patcher.stop() + async def test_inference_launch_rejects_client_runtime_state(self) -> None: response = await function_app.PutRunInferenceQueueMessage( make_request( @@ -129,6 +144,7 @@ async def test_trusted_principal_maps_to_active_haste_user(self) -> None: { "userId": "publisher@example.com", "objectId": "OBJECT-ID", + "userRoles": ["contributors"], "status": function_app.config.get_user_statuses().ACTIVE.value, "deleted": False, } @@ -144,7 +160,41 @@ async def test_trusted_principal_maps_to_active_haste_user(self) -> None: self.assertIsNone(error) self.assertEqual(caller["id"], "object-id") - self.assertEqual(caller["roles"], {"authenticated", "contributors"}) + self.assertEqual(caller["roles"], {"contributors"}) + + async def test_publishing_roles_use_principal_acl_intersection( + self, + ) -> None: + principal = { + "userId": "OBJECT-ID", + "userDetails": "publisher@example.com", + "userRoles": ["authenticated", "administrators"], + } + encoded = base64.b64encode( + json.dumps(principal).encode("utf-8") + ).decode("ascii") + metadata = Mock() + metadata.load.return_value = [ + { + "userId": "publisher@example.com", + "objectId": "OBJECT-ID", + "userRoles": ["contributors"], + "status": function_app.config.get_user_statuses().ACTIVE.value, + "deleted": False, + } + ] + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, "MetadataProcessor", return_value=metadata + ): + caller, error = await function_app._get_active_publishing_caller( + make_request(headers={"x-ms-client-principal": encoded}) + ) + + self.assertIsNone(caller) + self.assertEqual(error.status_code, 403) + self.assertEqual(response_json(error)["error"]["code"], "FORBIDDEN") async def test_invalid_principal_header_is_unauthenticated(self) -> None: with patch.object(function_app, "DEVELOPMENT_MODE", False): @@ -282,6 +332,177 @@ async def test_catalog_returns_bounded_pagination_metadata(self) -> None: sort_direction="asc", ) + async def test_catalog_reuses_same_query_after_authorization(self) -> None: + caller = {"id": "viewer", "roles": {"authenticated"}} + authorize = AsyncMock(return_value=(caller, None)) + repository = Mock() + repository.list_page.return_value = ([make_dataset("PUBLISHED")], 1) + with patch.object( + function_app, + "_get_active_publishing_caller", + new=authorize, + ), patch.object( + function_app, "PublishingRepository", return_value=repository + ): + first = await function_app.GetPublishedDatasets(make_request()) + second = await function_app.GetPublishedDatasets(make_request()) + + self.assertEqual(first.headers["X-Haste-Cache"], "MISS") + self.assertEqual(second.headers["X-Haste-Cache"], "HIT") + self.assertEqual(authorize.await_count, 2) + repository.list_page.assert_called_once() + + async def test_catalog_concurrent_requests_share_one_read(self) -> None: + caller = {"id": "viewer", "roles": {"authenticated"}} + repository = Mock() + repository.list_page.return_value = ([make_dataset("PUBLISHED")], 1) + started = asyncio.Event() + release = asyncio.Event() + thread_calls = 0 + + async def fake_to_thread(function, *args, **kwargs): + nonlocal thread_calls + thread_calls += 1 + started.set() + await release.wait() + return function(*args, **kwargs) + + with patch.object( + function_app, + "_get_active_publishing_caller", + new=AsyncMock(return_value=(caller, None)), + ), patch.object( + function_app, "PublishingRepository", return_value=repository + ), patch.object( + function_app.asyncio, + "to_thread", + new=fake_to_thread, + ): + first = asyncio.create_task( + function_app.GetPublishedDatasets(make_request()) + ) + await started.wait() + second = asyncio.create_task( + function_app.GetPublishedDatasets(make_request()) + ) + await asyncio.sleep(0) + release.set() + responses = await asyncio.gather(first, second) + + self.assertEqual(thread_calls, 1) + repository.list_page.assert_called_once() + self.assertEqual( + {response.headers["X-Haste-Cache"] for response in responses}, + {"MISS", "HIT"}, + ) + + async def test_catalog_matching_etag_returns_empty_304(self) -> None: + caller = {"id": "viewer", "roles": {"authenticated"}} + repository = Mock() + repository.list_page.return_value = ([make_dataset("PUBLISHED")], 1) + with patch.object( + function_app, + "_get_active_publishing_caller", + new=AsyncMock(return_value=(caller, None)), + ), patch.object( + function_app, "PublishingRepository", return_value=repository + ): + first = await function_app.GetPublishedDatasets(make_request()) + response = await function_app.GetPublishedDatasets( + make_request(headers={"If-None-Match": first.headers["ETag"]}) + ) + + self.assertEqual(response.status_code, 304) + self.assertEqual(response.get_body(), b"") + self.assertEqual(response.headers["X-Haste-Cache"], "HIT") + repository.list_page.assert_called_once() + + async def test_catalog_query_fields_use_separate_cache_entries( + self, + ) -> None: + caller = {"id": "viewer", "roles": {"authenticated"}} + repository = Mock() + repository.list_page.return_value = ([], 0) + with patch.object( + function_app, + "_get_active_publishing_caller", + new=AsyncMock(return_value=(caller, None)), + ), patch.object( + function_app, "PublishingRepository", return_value=repository + ): + await function_app.GetPublishedDatasets(make_request()) + await function_app.GetPublishedDatasets( + make_request(params={"status": "PUBLISHED"}) + ) + + self.assertEqual(repository.list_page.call_count, 2) + + async def test_catalog_no_cache_refreshes_response(self) -> None: + caller = {"id": "viewer", "roles": {"authenticated"}} + repository = Mock() + repository.list_page.side_effect = [ + ([make_dataset("PENDING")], 1), + ([make_dataset("PUBLISHED")], 1), + ] + with patch.object( + function_app, + "_get_active_publishing_caller", + new=AsyncMock(return_value=(caller, None)), + ), patch.object( + function_app, "PublishingRepository", return_value=repository + ): + await function_app.GetPublishedDatasets(make_request()) + response = await function_app.GetPublishedDatasets( + make_request(headers={"Cache-Control": "no-cache"}) + ) + + self.assertEqual(response.headers["X-Haste-Cache"], "MISS") + self.assertEqual( + response_json(response)["publishedDatasets"][0]["status"], + "PUBLISHED", + ) + self.assertEqual(repository.list_page.call_count, 2) + + async def test_successful_mutation_invalidates_catalog_cache(self) -> None: + caller = {"id": "publisher-object-id", "roles": {"contributors"}} + repository = Mock() + repository.list_page.side_effect = [ + ([make_dataset("PENDING")], 1), + ([make_dataset("PUBLISHED")], 1), + ] + processor = Mock() + processor.update_metadata.return_value = make_dataset("PUBLISHED") + with patch.object( + function_app, + "_get_active_publishing_caller", + new=AsyncMock(return_value=(caller, None)), + ), patch.object( + function_app, "PublishingRepository", return_value=repository + ), patch.object( + function_app, "_publishing_processor", return_value=processor + ): + first = await function_app.GetPublishedDatasets(make_request()) + mutation = await function_app.PutUpdatePublishedDataset( + make_request( + method="PUT", + body={ + "projectId": PROJECT_ID, + "datasetId": DATASET_ID, + "name": "Updated dataset", + }, + ) + ) + second = await function_app.GetPublishedDatasets(make_request()) + + self.assertEqual(first.headers["X-Haste-Cache"], "MISS") + self.assertEqual(mutation.status_code, 200) + self.assertEqual(second.headers["X-Haste-Cache"], "MISS") + self.assertEqual( + response_json(second)["publishedDatasets"][0]["status"], + "PUBLISHED", + ) + self.assertEqual(repository.list_page.call_count, 2) + async def test_catalog_rejects_unbounded_page_size(self) -> None: caller = {"id": "viewer", "roles": {"authenticated"}} with patch.object( diff --git a/api/hastefuncapi/tests/test_session_bootstrap_route.py b/api/hastefuncapi/tests/test_session_bootstrap_route.py new file mode 100644 index 00000000..73f6c5c7 --- /dev/null +++ b/api/hastefuncapi/tests/test_session_bootstrap_route.py @@ -0,0 +1,194 @@ +import base64 +import io +import json +import os +import unittest +from contextlib import redirect_stderr +from unittest.mock import Mock, patch + +import azure.functions as func + +os.environ.setdefault("DEVELOPMENT_MODE", "true") +os.environ.setdefault("METADATA_STORAGE_TYPE", "local") +os.environ.setdefault("ARTIFACT_STORAGE_TYPE", "local") +os.environ.setdefault("DATA_PATH", "/tmp/haste-session-api-tests") +os.environ.setdefault("TEMP_DATA_PATH", "/tmp/haste-session-api-tests") + +with redirect_stderr(io.StringIO()): + from api.hastefuncapi import function_app + +from hastegeo.core.models.session import ( # noqa: E402 + SessionBootstrap, + SessionPublishing, + SessionUser, +) +from hastegeo.core.processors.session import SessionAccessError # noqa: E402 + + +def make_request(principal: dict | None = None) -> func.HttpRequest: + headers = {} + if principal is not None: + headers["x-ms-client-principal"] = base64.b64encode( + json.dumps(principal).encode("utf-8") + ).decode("ascii") + return func.HttpRequest( + method="GET", + url="http://localhost/api/GetSessionBootstrap", + headers=headers, + params={}, + route_params={}, + body=b"", + ) + + +def response_json(response: func.HttpResponse) -> dict: + return json.loads(response.get_body().decode("utf-8")) + + +class TestSessionBootstrapRoute(unittest.IsolatedAsyncioTestCase): + def setUp(self) -> None: + self.principal = { + "userId": "object-id", + "userDetails": "analyst@example.com", + "userRoles": ["authenticated", "contributors"], + } + self.result = SessionBootstrap( + user=SessionUser( + userId="analyst@example.com", + identityId="object-id", + userRoles=["authenticated", "contributors"], + settings={"theme": "dark"}, + status="Active", + ), + publishing=SessionPublishing( + publishingEnabled=True, + providers=[], + ), + ) + + async def test_returns_resolved_session(self) -> None: + processor = Mock() + processor.load.return_value = self.result + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, + "SessionBootstrapProcessor", + return_value=processor, + ) as processor_type: + response = await function_app.GetSessionBootstrap( + make_request(self.principal) + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response_json(response), self.result.model_dump()) + processor_type.assert_called_once_with( + config=function_app.config, + development_mode=False, + ) + processor.load.assert_called_once_with(self.principal) + + async def test_missing_principal_is_unauthenticated(self) -> None: + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, "SessionBootstrapProcessor" + ) as processor_type: + response = await function_app.GetSessionBootstrap(make_request()) + + self.assertEqual(response.status_code, 401) + self.assertEqual( + response_json(response)["error"]["code"], "UNAUTHENTICATED" + ) + processor_type.assert_not_called() + + async def test_unknown_user_is_forbidden(self) -> None: + processor = Mock() + processor.load.side_effect = SessionAccessError( + "An active HASTE user is required." + ) + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, + "SessionBootstrapProcessor", + return_value=processor, + ): + response = await function_app.GetSessionBootstrap( + make_request(self.principal) + ) + + self.assertEqual(response.status_code, 403) + self.assertEqual(response_json(response)["error"]["code"], "FORBIDDEN") + + async def test_blocked_user_returns_roleless_status_response(self) -> None: + processor = Mock() + blocked = self.result.model_copy(deep=True) + blocked.user.status = "Inactive" + blocked.user.userRoles = [] + blocked.publishing.publishingEnabled = False + processor.load.return_value = blocked + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, + "SessionBootstrapProcessor", + return_value=processor, + ): + response = await function_app.GetSessionBootstrap( + make_request(self.principal) + ) + + self.assertEqual(response.status_code, 200) + payload = response_json(response) + self.assertEqual(payload["user"]["status"], "Inactive") + self.assertEqual(payload["user"]["userRoles"], []) + self.assertFalse(payload["publishing"]["publishingEnabled"]) + + async def test_development_mode_uses_local_principal(self) -> None: + processor = Mock() + processor.load.return_value = self.result + with patch.object( + function_app, "DEVELOPMENT_MODE", True + ), patch.object( + function_app, + "SessionBootstrapProcessor", + return_value=processor, + ) as processor_type: + response = await function_app.GetSessionBootstrap(make_request()) + + self.assertEqual(response.status_code, 200) + processor_type.assert_called_once_with( + config=function_app.config, + development_mode=True, + ) + processor.load.assert_called_once_with( + { + "userId": "development@local", + "userDetails": "development@local", + "userRoles": ["authenticated", "administrators"], + } + ) + + async def test_internal_error_returns_safe_message(self) -> None: + processor = Mock() + processor.load.side_effect = RuntimeError("sensitive detail") + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, + "SessionBootstrapProcessor", + return_value=processor, + ): + response = await function_app.GetSessionBootstrap( + make_request(self.principal) + ) + + self.assertEqual(response.status_code, 500) + payload = response_json(response) + self.assertEqual(payload["error"]["code"], "INTERNAL_ERROR") + self.assertNotIn("sensitive detail", response.get_body().decode()) + + +if __name__ == "__main__": + unittest.main() diff --git a/api/hastefuncapi/tests/test_user_route_security.py b/api/hastefuncapi/tests/test_user_route_security.py new file mode 100644 index 00000000..d47c50ce --- /dev/null +++ b/api/hastefuncapi/tests/test_user_route_security.py @@ -0,0 +1,211 @@ +import base64 +import io +import json +import os +import unittest +from contextlib import redirect_stderr +from unittest.mock import Mock, patch + +import azure.functions as func + +os.environ.setdefault("DEVELOPMENT_MODE", "true") +os.environ.setdefault("METADATA_STORAGE_TYPE", "local") +os.environ.setdefault("ARTIFACT_STORAGE_TYPE", "local") +os.environ.setdefault("DATA_PATH", "/tmp/haste-user-security-tests") +os.environ.setdefault("TEMP_DATA_PATH", "/tmp/haste-user-security-tests") + +with redirect_stderr(io.StringIO()): + from api.hastefuncapi import function_app + + +def principal_header(email: str, roles: list[str] | None = None) -> str: + principal = { + "userId": "attacker-object-id", + "userDetails": email, + "userRoles": roles or ["authenticated", "contributors"], + } + return base64.b64encode(json.dumps(principal).encode()).decode() + + +def make_request( + user: dict, + principal_roles: list[str] | None = None, +) -> func.HttpRequest: + return func.HttpRequest( + method="PUT", + url="http://localhost/api/PutUser", + headers={ + "x-ms-client-principal": principal_header( + "attacker@example.com", principal_roles + ) + }, + params={}, + route_params={}, + body=json.dumps({"user": user, "action": "update"}).encode(), + ) + + +def user_record( + email: str = "attacker@example.com", + status: str = "Active", +) -> dict: + return { + "userId": email, + "email": email, + "name": email, + "userRoles": ["contributors"], + "settings": {}, + "status": status, + "deleted": False, + } + + +class TestPutUserSecurity(unittest.IsolatedAsyncioTestCase): + async def test_non_admin_cannot_mix_own_email_with_victim_id(self) -> None: + metadata = Mock() + metadata.load.return_value = [user_record()] + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, "MetadataProcessor", return_value=metadata + ): + response = await function_app.PutUser( + make_request( + { + **user_record(), + "userId": "victim@example.com", + } + ) + ) + + self.assertEqual(response.status_code, 403) + metadata.load.assert_called_once_with("acl") + metadata.save.assert_not_called() + + async def test_non_admin_cannot_create_through_update(self) -> None: + metadata = Mock() + metadata.load.return_value = [user_record("other@example.com")] + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, "MetadataProcessor", return_value=metadata + ): + response = await function_app.PutUser(make_request(user_record())) + + self.assertEqual(response.status_code, 403) + metadata.save.assert_not_called() + + async def test_non_admin_cannot_reactivate_self(self) -> None: + metadata = Mock() + metadata.load.return_value = [ + user_record( + status=function_app.config.get_user_statuses().INACTIVE.value + ) + ] + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, "MetadataProcessor", return_value=metadata + ): + response = await function_app.PutUser(make_request(user_record())) + + self.assertEqual(response.status_code, 403) + metadata.save.assert_not_called() + + async def test_active_non_admin_can_update_own_settings(self) -> None: + metadata = Mock() + metadata.load.return_value = [user_record()] + request_user = user_record() + request_user["settings"] = {"theme": "dark"} + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, "MetadataProcessor", return_value=metadata + ): + response = await function_app.PutUser(make_request(request_user)) + + self.assertEqual(response.status_code, 200) + saved_users = metadata.save.call_args.args[1] + self.assertEqual(saved_users[0]["settings"], {"theme": "dark"}) + + async def test_stale_principal_admin_role_does_not_bypass_acl( + self, + ) -> None: + request = make_request( + user_record("victim@example.com"), + ["authenticated", "administrators"], + ) + metadata = Mock() + metadata.load.return_value = [user_record()] + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, "MetadataProcessor", return_value=metadata + ): + response = await function_app.PutUser(request) + + self.assertEqual(response.status_code, 403) + metadata.save.assert_not_called() + + async def test_stale_admin_role_cannot_read_admin_settings(self) -> None: + metadata = Mock() + metadata.load.return_value = [user_record()] + request = func.HttpRequest( + method="GET", + url="http://localhost/api/GetAdminSettings", + headers={ + "x-ms-client-principal": principal_header( + "attacker@example.com", + ["authenticated", "administrators"], + ) + }, + params={}, + route_params={}, + body=b"", + ) + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, "MetadataProcessor", return_value=metadata + ): + response = await function_app.GetAdminSettings(request) + + self.assertEqual(response.status_code, 403) + metadata.load.assert_called_once_with("acl") + + async def test_non_admin_cannot_read_another_user(self) -> None: + caller = user_record() + caller["objectId"] = "attacker-object-id" + metadata = Mock() + metadata.load.return_value = [ + caller, + user_record("victim@example.com"), + ] + request = func.HttpRequest( + method="GET", + url=( + "http://localhost/api/GetUserById" "?userId=victim@example.com" + ), + headers={ + "x-ms-client-principal": principal_header( + "attacker@example.com" + ) + }, + params={"userId": "victim@example.com"}, + route_params={}, + body=b"", + ) + + with patch.object( + function_app, "DEVELOPMENT_MODE", False + ), patch.object( + function_app, "MetadataProcessor", return_value=metadata + ): + response = await function_app.GetUserById(request) + + self.assertEqual(response.status_code, 403) + metadata.load.assert_called_once_with("acl") + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/api/hastefuncapi.md b/docs/api/hastefuncapi.md index 70370881..a64ad9f2 100644 --- a/docs/api/hastefuncapi.md +++ b/docs/api/hastefuncapi.md @@ -85,6 +85,7 @@ These endpoints use `FUNCTION`-level auth regardless of development mode (intend | Method | Route | Description | |--------|-------|-------------| +| GET | `GetSessionBootstrap` | Trusted current-user, role, settings, and publishing capabilities for one-call application startup. Accepts no caller identity parameters. | | GET | `GetUsers` | All users. Requires `administrators` role. | | GET | `GetUserById` | Single user by `userId`. | | PUT | `PutUser` | Create or update a user. Handles invitations, reinvitations, role assignment, and reactivation. | @@ -92,6 +93,14 @@ These endpoints use `FUNCTION`-level auth regardless of development mode (intend | GET | `GetAdminSettings` | All admin settings. Requires `administrators` role. | | PUT | `PutAdminSettings` | Update admin settings. Requires `administrators` role. | +`GetSessionBootstrap` resolves identity from the SWA client-principal header +and performs no user write for a stable active session. Blocked accounts retain +their status response but receive no application roles. + +`GetPublishedDatasets` supports `ETag`/`If-None-Match` and returns an empty +`304` for an unchanged fresh representation. Its process-local cache is bounded +to five seconds and is invalidated after publishing mutations. + ### Utilities | Method | Route | Description | diff --git a/hastelib/src/hastegeo/core/models/session.py b/hastelib/src/hastegeo/core/models/session.py new file mode 100644 index 00000000..6cc4eee8 --- /dev/null +++ b/hastelib/src/hastegeo/core/models/session.py @@ -0,0 +1,23 @@ +from typing import Any + +from pydantic import BaseModel, Field + +from .publishing import ProviderInfo + + +class SessionUser(BaseModel): + userId: str + identityId: str + userRoles: list[str] = Field(default_factory=list) + settings: dict[str, Any] = Field(default_factory=dict) + status: str + + +class SessionPublishing(BaseModel): + publishingEnabled: bool + providers: list[ProviderInfo] = Field(default_factory=list) + + +class SessionBootstrap(BaseModel): + user: SessionUser + publishing: SessionPublishing diff --git a/hastelib/src/hastegeo/core/processors/session.py b/hastelib/src/hastegeo/core/processors/session.py new file mode 100644 index 00000000..41b167e3 --- /dev/null +++ b/hastelib/src/hastegeo/core/processors/session.py @@ -0,0 +1,202 @@ +from collections.abc import Callable, Mapping +from typing import Any + +from ..config import Config +from ..models.session import SessionBootstrap, SessionPublishing, SessionUser +from ..models.users import User +from ..publishing.registry import PublishingProviderRegistry +from .metadata import MetadataProcessor + +APPLICATION_ROLES = frozenset({"administrators", "contributors"}) + + +def application_roles(value: Any) -> set[str]: + if not isinstance(value, (list, tuple, set)): + return set() + return { + role.strip().lower() + for role in value + if isinstance(role, str) and role.strip().lower() in APPLICATION_ROLES + } + + +def find_principal_user( + raw_users: list[dict[str, Any]], + principal_id: str, + login: str, +) -> User | None: + users = [User(**raw_user) for raw_user in raw_users] + normalized_principal_id = principal_id.casefold() + if normalized_principal_id: + for user in users: + if ( + user.objectId + and user.objectId.casefold() == normalized_principal_id + ): + return user + + legacy_candidates = { + value.casefold() for value in (principal_id, login) if value + } + for user in users: + if user.objectId: + continue + identifiers = { + value.casefold() for value in (user.userId, user.email) if value + } + if identifiers.intersection(legacy_candidates): + return user + return None + + +def effective_application_roles( + principal_roles: Any, + acl_roles: Any, +) -> set[str]: + return application_roles(principal_roles).intersection( + application_roles(acl_roles) + ) + + +def index_unique_aad_users( + app_users: list[Mapping[str, Any]], +) -> dict[str, Mapping[str, Any]]: + users_by_login: dict[str, list[Mapping[str, Any]]] = {} + for app_user in app_users: + provider = str(app_user.get("provider") or "").strip().casefold() + login = str(app_user.get("login") or "").strip().casefold() + object_id = str(app_user.get("objectId") or "").strip() + if provider != "aad" or not login or not object_id: + continue + users_by_login.setdefault(login, []).append(app_user) + return { + login: candidates[0] + for login, candidates in users_by_login.items() + if len(candidates) == 1 + } + + +def bind_swa_object_id( + user: dict[str, Any], app_user: Mapping[str, Any] | None +) -> bool: + if app_user is None: + return False + object_id = str(app_user.get("objectId") or "").strip() + if not object_id: + return False + existing = str(user.get("objectId") or "").strip() + if existing and existing.casefold() != object_id.casefold(): + return False + if not existing: + user["objectId"] = object_id + return True + + +class SessionAccessError(PermissionError): + pass + + +class SessionBootstrapProcessor: + def __init__( + self, + config: Config | None = None, + processor_factory: Callable[..., MetadataProcessor] = ( + MetadataProcessor + ), + registry_factory: Callable[..., PublishingProviderRegistry] = ( + PublishingProviderRegistry + ), + development_mode: bool = False, + ) -> None: + self.config = config or Config() + self.processor_factory = processor_factory + self.registry_factory = registry_factory + self.development_mode = development_mode + + def load(self, principal: Mapping[str, Any]) -> SessionBootstrap: + principal_id = self._string(principal.get("userId")) + login = self._string(principal.get("userDetails")) + if not principal_id and not login: + raise SessionAccessError("Authentication is required.") + + user = self._load_user(principal_id, login) + active_status = self.config.get_user_statuses().ACTIVE.value + if user.deleted or user.status != active_status: + pending_status = self.config.get_user_statuses().PENDING.value + inactive_status = self.config.get_user_statuses().INACTIVE.value + return SessionBootstrap( + user=SessionUser( + userId=user.email or user.userId or login, + identityId=( + principal_id or user.objectId or user.userId or login + ), + userRoles=[], + settings=user.settings or {}, + status=( + pending_status + if not user.deleted and user.status == pending_status + else inactive_status + ), + ), + publishing=SessionPublishing( + publishingEnabled=False, + providers=[], + ), + ) + + effective_roles = sorted( + effective_application_roles( + principal.get("userRoles"), user.userRoles + ) + ) + if not effective_roles: + raise SessionAccessError("No active HASTE role is assigned.") + + registry = self.registry_factory(config=self.config) + return SessionBootstrap( + user=SessionUser( + userId=user.email or user.userId or login, + identityId=principal_id + or user.objectId + or user.userId + or login, + userRoles=effective_roles, + settings=user.settings or {}, + status=user.status, + ), + publishing=SessionPublishing( + publishingEnabled=bool( + self.config.publishing_config["publishing_enabled"] + ), + providers=registry.list_infos(), + ), + ) + + def _load_user(self, principal_id: str, login: str) -> User: + try: + raw_users = self.processor_factory( + data_type=self.config.get_metadata_types().USERS.value, + config=self.config, + ).load("acl") + except FileNotFoundError: + raw_users = [] + + user = find_principal_user(raw_users, principal_id, login) + if user is not None: + return user + + if self.development_mode: + active_status = self.config.get_user_statuses().ACTIVE.value + return User( + userId=login or principal_id, + objectId=principal_id or None, + email=login or principal_id, + userRoles=["administrators"], + status=active_status, + settings={}, + ) + raise SessionAccessError("An active HASTE user is required.") + + @staticmethod + def _string(value: Any) -> str: + return value.strip() if isinstance(value, str) else "" diff --git a/hastelib/src/hastegeo/core/utils/async_cache.py b/hastelib/src/hastegeo/core/utils/async_cache.py index 72a7db95..02b84b5f 100644 --- a/hastelib/src/hastegeo/core/utils/async_cache.py +++ b/hastelib/src/hastegeo/core/utils/async_cache.py @@ -101,3 +101,14 @@ async def clear(self) -> None: self._entries.clear() for task in tasks: task.cancel() + + async def invalidate(self) -> None: + """Drop cached values without cancelling current readers. + + In-flight loads are detached so callers after invalidation start a + fresh load. Detached results can still return to their original + callers, but ``_complete`` will not cache them. + """ + async with self._lock: + self._inflight.clear() + self._entries.clear() diff --git a/hastelib/tests/core/processors/test_session.py b/hastelib/tests/core/processors/test_session.py new file mode 100644 index 00000000..184efdc9 --- /dev/null +++ b/hastelib/tests/core/processors/test_session.py @@ -0,0 +1,279 @@ +import unittest +from unittest.mock import Mock + +from hastegeo.core.config import Config +from hastegeo.core.processors.session import ( + SessionAccessError, + SessionBootstrapProcessor, + bind_swa_object_id, + index_unique_aad_users, +) + + +class TestSessionBootstrapProcessor(unittest.TestCase): + def setUp(self) -> None: + self.config = Config() + self.active_status = self.config.get_user_statuses().ACTIVE.value + self.metadata = Mock() + self.processor_factory = Mock(return_value=self.metadata) + self.registry = Mock() + self.registry.list_infos.return_value = [] + self.registry_factory = Mock(return_value=self.registry) + self.processor = SessionBootstrapProcessor( + config=self.config, + processor_factory=self.processor_factory, + registry_factory=self.registry_factory, + ) + self.principal = { + "userId": "OBJECT-ID", + "userDetails": "analyst@example.com", + "userRoles": ["authenticated", "contributors", "administrators"], + } + + def test_stable_active_session_reads_acl_without_writing(self) -> None: + self.metadata.load.return_value = [ + { + "userId": "analyst@example.com", + "objectId": "object-id", + "email": "analyst@example.com", + "userRoles": ["authenticated", "contributors"], + "settings": {"theme": "dark"}, + "status": self.active_status, + } + ] + + result = self.processor.load(self.principal) + + self.metadata.load.assert_called_once_with("acl") + self.metadata.save.assert_not_called() + self.assertEqual(result.user.identityId, "OBJECT-ID") + self.assertEqual(result.user.settings, {"theme": "dark"}) + self.assertEqual(result.user.userRoles, ["contributors"]) + self.assertNotIn("administrators", result.user.userRoles) + + def test_bound_object_id_does_not_fall_back_to_reused_email(self) -> None: + self.metadata.load.return_value = [ + { + "userId": "analyst@example.com", + "objectId": "different-object-id", + "email": "analyst@example.com", + "userRoles": ["contributors"], + "status": self.active_status, + } + ] + + with self.assertRaises(SessionAccessError): + self.processor.load(self.principal) + + def test_authenticated_system_role_does_not_grant_access(self) -> None: + self.metadata.load.return_value = [ + { + "userId": "analyst@example.com", + "userRoles": ["authenticated"], + "status": self.active_status, + } + ] + + with self.assertRaises(SessionAccessError): + self.processor.load(self.principal) + + def test_legacy_email_match_is_case_insensitive(self) -> None: + self.metadata.load.return_value = [ + { + "userId": "Analyst@Example.com", + "userRoles": ["contributors"], + "status": self.active_status, + } + ] + + result = self.processor.load(self.principal) + + self.assertEqual(result.user.userId, "Analyst@Example.com") + self.assertEqual(result.user.userRoles, ["contributors"]) + + def test_inactive_user_returns_blocked_session(self) -> None: + self.metadata.load.return_value = [ + { + "userId": "analyst@example.com", + "userRoles": ["contributors"], + "status": self.config.get_user_statuses().INACTIVE.value, + } + ] + + result = self.processor.load(self.principal) + + self.assertEqual(result.user.userRoles, []) + self.assertEqual( + result.user.status, + self.config.get_user_statuses().INACTIVE.value, + ) + self.assertFalse(result.publishing.publishingEnabled) + self.metadata.save.assert_not_called() + + def test_deleted_user_returns_inactive_session(self) -> None: + self.metadata.load.return_value = [ + { + "userId": "analyst@example.com", + "userRoles": ["contributors"], + "status": self.active_status, + "deleted": True, + } + ] + + result = self.processor.load(self.principal) + + self.assertEqual(result.user.userRoles, []) + self.assertEqual( + result.user.status, + self.config.get_user_statuses().INACTIVE.value, + ) + + def test_unknown_user_is_denied(self) -> None: + self.metadata.load.return_value = [] + + with self.assertRaises(SessionAccessError): + self.processor.load(self.principal) + + def test_legacy_user_without_status_returns_inactive_session(self) -> None: + self.metadata.load.return_value = [ + { + "userId": "analyst@example.com", + "userRoles": ["contributors"], + } + ] + principal = { + "userDetails": "analyst@example.com", + "userRoles": ["contributors"], + } + + result = self.processor.load(principal) + + self.assertEqual(result.user.identityId, "analyst@example.com") + self.assertEqual( + result.user.status, + self.config.get_user_statuses().INACTIVE.value, + ) + self.assertEqual(result.user.userRoles, []) + + def test_role_mismatch_is_denied(self) -> None: + self.metadata.load.return_value = [ + { + "userId": "analyst@example.com", + "userRoles": ["administrators"], + "status": self.active_status, + } + ] + principal = dict(self.principal, userRoles=["contributors"]) + + with self.assertRaises(SessionAccessError): + self.processor.load(principal) + + def test_missing_principal_identity_is_denied(self) -> None: + with self.assertRaises(SessionAccessError): + self.processor.load({"userRoles": ["contributors"]}) + + self.processor_factory.assert_not_called() + + def test_development_mode_can_create_ephemeral_session(self) -> None: + self.metadata.load.side_effect = FileNotFoundError + processor = SessionBootstrapProcessor( + config=self.config, + processor_factory=self.processor_factory, + registry_factory=self.registry_factory, + development_mode=True, + ) + principal = { + "userId": "development@local", + "userDetails": "development@local", + "userRoles": ["authenticated", "administrators"], + } + + result = processor.load(principal) + + self.assertEqual(result.user.userId, "development@local") + self.assertEqual(result.user.userRoles, ["administrators"]) + self.metadata.save.assert_not_called() + + +class TestBindSwaObjectId(unittest.TestCase): + def test_binds_legacy_record_once(self) -> None: + user = {"userId": "analyst@example.com", "objectId": None} + + matched = bind_swa_object_id(user, {"objectId": "object-id"}) + + self.assertTrue(matched) + self.assertEqual(user["objectId"], "object-id") + + def test_accepts_matching_bound_identity(self) -> None: + user = {"objectId": "OBJECT-ID"} + + self.assertTrue(bind_swa_object_id(user, {"objectId": "object-id"})) + + def test_rejects_conflicting_bound_identity(self) -> None: + user = {"objectId": "old-object-id"} + + matched = bind_swa_object_id(user, {"objectId": "new-object-id"}) + + self.assertFalse(matched) + self.assertEqual(user["objectId"], "old-object-id") + + def test_rejects_management_record_without_object_id(self) -> None: + user = {"userId": "analyst@example.com", "objectId": None} + + matched = bind_swa_object_id(user, {"objectId": None}) + + self.assertFalse(matched) + self.assertIsNone(user["objectId"]) + + +class TestIndexUniqueAadUsers(unittest.TestCase): + def test_indexes_one_aad_identity_case_insensitively(self) -> None: + app_user = { + "login": "Analyst@Example.com", + "provider": "aad", + "objectId": "object-id", + } + + result = index_unique_aad_users([app_user]) + + self.assertEqual(result, {"analyst@example.com": app_user}) + + def test_ignores_non_aad_and_missing_object_ids(self) -> None: + result = index_unique_aad_users( + [ + { + "login": "analyst@example.com", + "provider": "github", + "objectId": "github-id", + }, + { + "login": "other@example.com", + "provider": "aad", + "objectId": None, + }, + ] + ) + + self.assertEqual(result, {}) + + def test_rejects_duplicate_aad_logins(self) -> None: + result = index_unique_aad_users( + [ + { + "login": "analyst@example.com", + "provider": "aad", + "objectId": "first-id", + }, + { + "login": "ANALYST@example.com", + "provider": "aad", + "objectId": "second-id", + }, + ] + ) + + self.assertEqual(result, {}) + + +if __name__ == "__main__": + unittest.main() diff --git a/hastelib/tests/core/utils/test_async_cache.py b/hastelib/tests/core/utils/test_async_cache.py index cf2dab3d..8b484ac5 100644 --- a/hastelib/tests/core/utils/test_async_cache.py +++ b/hastelib/tests/core/utils/test_async_cache.py @@ -173,6 +173,45 @@ async def factory() -> str: await request await asyncio.sleep(0) + async def test_invalidate_removes_cached_values(self) -> None: + factory = AsyncMock(side_effect=["first", "second"]) + await self.cache.get_or_create("key", factory) + + await self.cache.invalidate() + value, reused = await self.cache.get_or_create("key", factory) + + self.assertEqual(value, "second") + self.assertFalse(reused) + self.assertEqual(factory.await_count, 2) + + async def test_invalidate_detaches_stale_inflight_load(self) -> None: + old_started = asyncio.Event() + old_release = asyncio.Event() + + async def old_factory() -> str: + old_started.set() + await old_release.wait() + return "old" + + old_request = asyncio.create_task( + self.cache.get_or_create("key", old_factory) + ) + await old_started.wait() + + await self.cache.invalidate() + fresh, reused = await self.cache.get_or_create( + "key", lambda: asyncio.sleep(0, result="fresh") + ) + old_release.set() + old, _ = await old_request + cached, cached_reused = await self.cache.get_or_create( + "key", lambda: asyncio.sleep(0, result="unexpected") + ) + + self.assertEqual((old, fresh, cached), ("old", "fresh", "fresh")) + self.assertFalse(reused) + self.assertTrue(cached_reused) + def test_rejects_invalid_configuration(self) -> None: with self.assertRaises(ValueError): AsyncTTLCache(ttl_seconds=-1, max_entries=1) From 170a63dd5141bfa7ba9c548211b248933db9f606 Mon Sep 17 00:00:00 2001 From: prbatero <42007693+prbatero@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:11:58 -0400 Subject: [PATCH 3/7] feat(ui): parallelize application route loading Bootstrap sessions in one request, overlap independent route data and Azure Maps assets, and add retryable loading states. Deduplicate Published Datasets polling and cancel sibling artifact transfers on failure. --- ui/src/App.jsx | 48 +++--- ui/src/Components/AppBody.jsx | 25 ++- .../BuildingValidation/BuildingValidation.jsx | 110 +++++++------ .../loadValidationMapData.js | 24 +++ .../loadValidationMapData.test.js | 88 ++++++++++ .../Components/CreateEditImageLayerForm.jsx | 61 ++++--- .../Components/CreateEditImageLayerHelper.js | 15 +- .../CreateEditImageLayerHelper.test.js | 51 +++++- .../HelpDocs/HelpDocsImageLayers.jsx | 16 +- .../Components/HelpDocs/HelpDocsLabeling.jsx | 37 ++--- .../HelpDocs/HelpDocsModelCatalog.jsx | 14 +- .../HelpDocs/HelpDocsModelTraining.jsx | 12 +- .../Components/HelpDocs/HelpDocsOverview.jsx | 17 +- .../Components/HelpDocs/HelpDocsProjects.jsx | 14 +- .../Components/HelpDocs/HelpDocsResults.jsx | 18 +- ui/src/Components/Home.jsx | 64 +++++--- ui/src/Components/Home/loadHomeData.js | 16 ++ ui/src/Components/Home/loadHomeData.test.js | 64 ++++++++ .../InteractiveLabeler/InteractiveLabeler.jsx | 154 ++++++++---------- .../loadInteractiveArtifacts.js | 31 ++++ .../loadInteractiveArtifacts.test.js | 90 ++++++++++ .../loadInteractiveMetadata.js | 36 ++++ .../loadInteractiveMetadata.test.js | 74 +++++++++ .../Components/LabelingTool/LabelingTool.jsx | 19 ++- ui/src/Components/MapRoute.jsx | 59 +++++++ .../OpenDataCatalog/OpenDataCatalogPanel.jsx | 44 ++++- ui/src/Components/PublishedDatasets.jsx | 131 +++++++++++---- ui/src/Components/Visualizer/Visualizer.jsx | 20 ++- ui/src/Components/helpMediaLoading.test.js | 32 ++++ ui/src/Components/loadImageLayerFormData.js | 12 ++ ui/src/assets/css/style.css | 8 + ui/src/util/api.js | 56 +++---- ui/src/util/azureMapsLoader.js | 15 +- ui/src/util/azureMapsLoader.test.js | 80 ++++++++- ui/src/util/publishedDatasetsRequest.js | 38 +++++ ui/src/util/publishedDatasetsRequest.test.js | 93 +++++++++++ ui/src/util/sessionBootstrap.test.js | 80 +++++++++ ui/src/util/sessionStartup.js | 28 ++++ ui/src/util/sessionStartup.test.js | 48 ++++++ 39 files changed, 1466 insertions(+), 376 deletions(-) create mode 100644 ui/src/Components/BuildingValidation/loadValidationMapData.js create mode 100644 ui/src/Components/BuildingValidation/loadValidationMapData.test.js create mode 100644 ui/src/Components/Home/loadHomeData.js create mode 100644 ui/src/Components/Home/loadHomeData.test.js create mode 100644 ui/src/Components/InteractiveLabeler/loadInteractiveArtifacts.js create mode 100644 ui/src/Components/InteractiveLabeler/loadInteractiveArtifacts.test.js create mode 100644 ui/src/Components/InteractiveLabeler/loadInteractiveMetadata.js create mode 100644 ui/src/Components/InteractiveLabeler/loadInteractiveMetadata.test.js create mode 100644 ui/src/Components/MapRoute.jsx create mode 100644 ui/src/Components/helpMediaLoading.test.js create mode 100644 ui/src/Components/loadImageLayerFormData.js create mode 100644 ui/src/util/publishedDatasetsRequest.js create mode 100644 ui/src/util/publishedDatasetsRequest.test.js create mode 100644 ui/src/util/sessionBootstrap.test.js create mode 100644 ui/src/util/sessionStartup.js create mode 100644 ui/src/util/sessionStartup.test.js diff --git a/ui/src/App.jsx b/ui/src/App.jsx index 3489c36d..92eec448 100644 --- a/ui/src/App.jsx +++ b/ui/src/App.jsx @@ -14,7 +14,8 @@ import { Toaster, } from "@fluentui/react-components"; import { AppContext } from "./AppContext"; -import { apiValidateUser, apiGet } from "./util/api"; +import { apiValidateUser } from "./util/api"; +import { loadSession } from "./util/sessionStartup"; import { useTheme } from "./util/ThemeContext"; import { getPalette } from "./util/theme"; @@ -37,6 +38,7 @@ function App() { const isHome = location.pathname === '/' || location.pathname === '/home'; const [modalComponent, setModalComponent] = useState(null); + const [sessionError, setSessionError] = useState(false); const [navCollapsed, setNavCollapsed] = useState(() => { const stored = localStorage.getItem("haste-nav-collapsed"); return stored === null ? true : stored === "true"; @@ -52,28 +54,15 @@ function App() { }); }; - useEffect(() => { - const validateUser = async () => { - setIsLoading(true); - await apiValidateUser(setAppParams); - try { - const publishing = await apiGet("GetPublishingProviders"); - setAppParams((previous) => ({ - ...previous, - publishingEnabled: !!publishing.publishingEnabled, - publishingProviders: publishing.providers || [], - })); - } catch (error) { - console.error("Error loading publishing capabilities:", error); - setAppParams((previous) => ({ - ...previous, - publishingEnabled: false, - publishingProviders: [], - })); - } - setIsLoading(false); - }; + const validateUser = () => + loadSession({ + validateUser: apiValidateUser, + setAppParams, + setIsLoading, + setSessionError, + }); + useEffect(() => { validateUser(); //eslint-disable-next-line react-hooks/exhaustive-deps @@ -152,9 +141,20 @@ function App() { return ( <>
- {appParams.userStatus === "Inactive" || appParams.userStatus === "PendingAcceptance" ? ( + {sessionError ? ( +
+

Session unavailable

+

HASTE could not load your session. Try again.

+ +
+ ) : ["Inactive", "PendingAcceptance", "Deleted"].includes(appParams.userStatus) ? (
-
{appParams.userId} {appParams.userStatus === "PendingAcceptance" ? "account is pending acceptance" : "account is inactive"}
+
{appParams.userId} {appParams.userStatus === "PendingAcceptance" ? "account is pending acceptance" : appParams.userStatus === "Deleted" ? "account has been deleted" : "account is inactive"}

{appParams.userStatus === "PendingAcceptance" ? "Please accept the invitation, if it has expired please contact the app administrator." : "Please contact the app administrator."}

) : ( diff --git a/ui/src/Components/AppBody.jsx b/ui/src/Components/AppBody.jsx index 8f9a640a..8d8e37c7 100644 --- a/ui/src/Components/AppBody.jsx +++ b/ui/src/Components/AppBody.jsx @@ -7,37 +7,32 @@ import Loading from "./OtherComponents/Loading"; import PropType from "prop-types"; import { AppContext } from "../AppContext"; -import { loadAzureMaps } from "../util/azureMapsLoader"; - -const loadMapRoute = (importRoute) => () => - loadAzureMaps().then(() => importRoute()); +import { createMapRoute, RouteLoading } from "./MapRoute"; const AdminLabelingTool = lazy(() => import("./AdminLabelingTool")); const AdminSourceTypes = lazy(() => import("./AdminSourceTypes")); const AdminUsers = lazy(() => import("./AdminUsers")); -const BuildingValidation = lazy( - loadMapRoute(() => import("./BuildingValidation/BuildingValidation")) +const BuildingValidation = createMapRoute( + () => import("./BuildingValidation/BuildingValidation") ); const CreateEditImageLayerForm = lazy( - loadMapRoute(() => import("./CreateEditImageLayerForm")) + () => import("./CreateEditImageLayerForm") ); const Error404 = lazy(() => import("./Error404")); const HelpDocs = lazy(() => import("./HelpDocs")); const Home = lazy(() => import("./Home")); const ImageLayer = lazy(() => import("./ImageLayer")); -const InteractiveLabeler = lazy( - loadMapRoute(() => import("./InteractiveLabeler/InteractiveLabeler")) +const InteractiveLabeler = createMapRoute( + () => import("./InteractiveLabeler/InteractiveLabeler") ); -const LabelingTool = lazy( - loadMapRoute(() => import("./LabelingTool/LabelingTool")) +const LabelingTool = createMapRoute( + () => import("./LabelingTool/LabelingTool") ); const ModelCatalog = lazy(() => import("./ModelCatalog")); const Project = lazy(() => import("./Project")); const Projects = lazy(() => import("./Projects")); const PublishedDatasets = lazy(() => import("./PublishedDatasets")); -const Visualizer = lazy( - loadMapRoute(() => import("./Visualizer/Visualizer")) -); +const Visualizer = createMapRoute(() => import("./Visualizer/Visualizer")); const AppBody = ({ setModalComponent }) => { const { appParams } = useContext(AppContext); @@ -47,7 +42,7 @@ const AppBody = ({ setModalComponent }) => { return (
{appParams.isLoading && } - {routesReady && }> + {routesReady && }> {appParams.userRoles !== null && appParams.publishingEnabled && ( } /> )} diff --git a/ui/src/Components/BuildingValidation/BuildingValidation.jsx b/ui/src/Components/BuildingValidation/BuildingValidation.jsx index 695603e8..ddf6b298 100644 --- a/ui/src/Components/BuildingValidation/BuildingValidation.jsx +++ b/ui/src/Components/BuildingValidation/BuildingValidation.jsx @@ -13,6 +13,7 @@ import { DEFAULT_VALIDATION_SAMPLE, resolveSampleSize, } from "./validationConfig.js"; +import { loadValidationMapData } from "./loadValidationMapData.js"; import { loadImagery } from "../LabelingTool/LabelingToolHelper.js"; import { shouldIgnoreShortcut } from "../keyboardShortcuts.js"; import "../../assets/css/labels.css"; @@ -53,7 +54,7 @@ const useStyles = makeStyles({ // Filter values used by the right-panel dropdown and the map dim logic. // 'all' means no filter; 'unlabeled' means buildings with no label set yet; // the three class names match the values stored on labels[id].label. -export const FILTER_VALUES = ["all", "unlabeled", "Damaged", "NotDamaged", "Unknown"]; +const FILTER_VALUES = ["all", "unlabeled", "Damaged", "NotDamaged", "Unknown"]; function buildingMatchesFilter(feature, labels, filter) { if (filter === "all") return true; @@ -62,6 +63,25 @@ function buildingMatchesFilter(feature, labels, filter) { return lbl === filter; } +function extractCentroid(feature) { + try { + const geom = feature.geometry; + if (!geom) return null; + const coords = + geom.type === "Polygon" + ? geom.coordinates[0] + : geom.type === "MultiPolygon" + ? geom.coordinates[0][0] + : null; + if (!coords || coords.length === 0) return null; + const lng = coords.reduce((sum, coord) => sum + coord[0], 0) / coords.length; + const lat = coords.reduce((sum, coord) => sum + coord[1], 0) / coords.length; + return [lng, lat]; + } catch { + return null; + } +} + const BuildingValidation = () => { const styles = useStyles(); const { projectId, imageLayerId } = useParams(); @@ -111,6 +131,8 @@ const BuildingValidation = () => { // document on load and changed through the settings modal. const [sampleSize, setSampleSize] = useState(DEFAULT_VALIDATION_SAMPLE); const [configOpen, setConfigOpen] = useState(false); + const [loadError, setLoadError] = useState(false); + const [initAttempt, setInitAttempt] = useState(0); // Post-event is only genuinely showable once that layer exists. Without // this, the toggle defaults to "post" on a layer that has no post-event @@ -167,8 +189,13 @@ const BuildingValidation = () => { if (!window.atlas) return; setIsLoading(true, "Loading Building Validation"); try { + // eslint-disable-next-line react-hooks/immutability await createMap(); setIsMapReady(true); + setLoadError(false); + } catch (error) { + console.error("Failed to initialize building validation:", error); + setLoadError(true); } finally { setIsLoading(false); } @@ -184,7 +211,7 @@ const BuildingValidation = () => { } }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [initAttempt]); async function fetchFootprints(size) { return apiGet( @@ -221,29 +248,19 @@ const BuildingValidation = () => { } async function createMap() { - // Load imagery tile URLs (reuse labeling tool endpoint); may not exist if no labels yet - let layerData = null; - try { - layerData = await apiGet( - `GetLayerLabelingToolData?projectId=${projectId}&imageLayerId=${imageLayerId}` - ); - } catch { - // No label project yet — imagery won't be shown, validation still works - } - - // Load any existing validation labels. This comes first because it also - // carries the layer's configured sample size, which decides how many - // footprints to ask for below. - const validationData = await apiGet( - `GetBuildingValidation?projectId=${projectId}&imageLayerId=${imageLayerId}` - ); - const configuredSample = resolveSampleSize(validationData); + const { + layerData, + validationData, + footprintsGeoJSON, + sampleSize: configuredSample, + } = await loadValidationMapData({ + get: apiGet, + projectId, + imageLayerId, + resolveSampleSize, + }); setSampleSize(configuredSample); - // Load building footprints as GeoJSON — a deterministic sample of the - // configured size. - const footprintsGeoJSON = await fetchFootprints(configuredSample); - const existingLabels = validationData?.labels || {}; setLabels(existingLabels); @@ -414,7 +431,6 @@ const BuildingValidation = () => { mapRef.current.setCamera({ center: coords, zoom: 18, duration: 500 }); } } - // eslint-disable-next-line react-hooks/exhaustive-deps }, [labels, selectedIndex, features, filter, isDatasourceReady]); // When the filter changes such that the current selection no longer @@ -423,29 +439,10 @@ const BuildingValidation = () => { useEffect(() => { if (filteredIndices.length === 0) return; if (!filteredIndices.includes(selectedIndex)) { + // eslint-disable-next-line react-hooks/set-state-in-effect setSelectedIndex(filteredIndices[0]); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [filter]); - - function extractCentroid(feature) { - try { - const geom = feature.geometry; - if (!geom) return null; - const coords = - geom.type === "Polygon" - ? geom.coordinates[0] - : geom.type === "MultiPolygon" - ? geom.coordinates[0][0] - : null; - if (!coords || coords.length === 0) return null; - const lng = coords.reduce((s, c) => s + c[0], 0) / coords.length; - const lat = coords.reduce((s, c) => s + c[1], 0) / coords.length; - return [lng, lat]; - } catch { - return null; - } - } + }, [filteredIndices, selectedIndex]); // Web-Mercator slippy-tile math. Returns {x, y, z} for the tile that // contains the given lng/lat at zoom z. Matches the {z}/{x}/{y} URL @@ -649,7 +646,7 @@ const BuildingValidation = () => { setDialog("Saved", "Validation labels saved successfully.", [ { type: "primary", key: "close", text: "Close", onClick: () => setDialog() }, ]); - } catch (e) { + } catch { setDialog("Error", "Failed to save validation labels.", [ { type: "primary", key: "close", text: "Close", onClick: () => setDialog() }, ]); @@ -685,7 +682,10 @@ const BuildingValidation = () => { const labeledCount = Object.keys(labels).length; return ( -
+
{/* Back button — shares the Interactive Labeler navigation surface. */}
+
+ )} + {/* Right panel */} {isMapReady && features.length > 0 && ( null + ); + const validationData = await get(`GetBuildingValidation?${query}`); + const sampleSize = resolveSampleSize(validationData); + const [layerData, footprintsGeoJSON] = await Promise.all([ + layerPromise, + get(`GetBuildingFootprintsGeoJSON?${query}&sample=${sampleSize}`), + ]); + + return { + layerData, + validationData, + footprintsGeoJSON, + sampleSize, + }; +} \ No newline at end of file diff --git a/ui/src/Components/BuildingValidation/loadValidationMapData.test.js b/ui/src/Components/BuildingValidation/loadValidationMapData.test.js new file mode 100644 index 00000000..ae9f0dcc --- /dev/null +++ b/ui/src/Components/BuildingValidation/loadValidationMapData.test.js @@ -0,0 +1,88 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { loadValidationMapData } from "./loadValidationMapData.js"; + + +function deferred() { + let resolve; + let reject; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, resolve, reject }; +} + +test("overlaps optional imagery with required validation data", async () => { + const imagery = deferred(); + const validation = deferred(); + const footprints = deferred(); + const calls = []; + const loading = loadValidationMapData({ + projectId: "project-1", + imageLayerId: "layer-1", + resolveSampleSize: () => 300, + get: (endpoint) => { + calls.push(endpoint); + if (endpoint.startsWith("GetLayerLabelingToolData")) { + return imagery.promise; + } + if (endpoint.startsWith("GetBuildingValidation")) { + return validation.promise; + } + return footprints.promise; + }, + }); + + assert.equal(calls.length, 2); + validation.resolve({ labels: {} }); + await Promise.resolve(); + assert.equal(calls.length, 3); + imagery.resolve({ imagery: {} }); + footprints.resolve({ features: [] }); + + assert.deepEqual(await loading, { + layerData: { imagery: {} }, + validationData: { labels: {} }, + footprintsGeoJSON: { features: [] }, + sampleSize: 300, + }); +}); + +test("continues without optional imagery", async () => { + const result = await loadValidationMapData({ + projectId: "project-1", + imageLayerId: "layer-1", + resolveSampleSize: () => 100, + get: async (endpoint) => { + if (endpoint.startsWith("GetLayerLabelingToolData")) { + throw new Error("no labels"); + } + if (endpoint.startsWith("GetBuildingValidation")) { + return { labels: {} }; + } + return { features: [] }; + }, + }); + + assert.equal(result.layerData, null); + assert.deepEqual(result.footprintsGeoJSON, { features: [] }); +}); + +test("rejects when required validation data fails", async () => { + await assert.rejects( + loadValidationMapData({ + projectId: "project-1", + imageLayerId: "layer-1", + resolveSampleSize: () => 100, + get: async (endpoint) => { + if (endpoint.startsWith("GetBuildingValidation")) { + throw new Error("validation unavailable"); + } + return {}; + }, + }), + /validation unavailable/ + ); +}); \ No newline at end of file diff --git a/ui/src/Components/CreateEditImageLayerForm.jsx b/ui/src/Components/CreateEditImageLayerForm.jsx index 7a7202b5..1448519f 100644 --- a/ui/src/Components/CreateEditImageLayerForm.jsx +++ b/ui/src/Components/CreateEditImageLayerForm.jsx @@ -10,6 +10,8 @@ import { Dropdown, Option, Field, + MessageBar, + MessageBarBody, Tooltip, } from "@fluentui/react-components"; @@ -45,8 +47,29 @@ const CreateEditImageLayerModal = () => { const projectId = useParams().projectId; const imageLayerId = useParams().imageLayerId; - const [isUploading, setIsUploading] = useState(false); const [isCatalogOpen, setIsCatalogOpen] = useState(false); + const [loadError, setLoadError] = useState(false); + const isUploading = componentState + ? validateIsUploading( + componentState.preEventImageryUrls, + componentState.postEventImageryUrls, + componentState.userBuildingFootprintsUrls || [] + ) + : false; + + async function initComponent() { + setIsLoading(true); + try { + setComponentState( + await createComponentDefaultState(imageLayerId, projectId) + ); + setLoadError(false); + } catch { + setLoadError(true); + } finally { + setIsLoading(false); + } + } // Add a scene picked from the Open Data Catalog explorer into the pre/post // imagery array (with source-type + capture-date auto-fill). Returns the @@ -63,14 +86,7 @@ const CreateEditImageLayerModal = () => { } useEffect(() => { - async function initComponent() { - setIsLoading(true); - setComponentState( - await createComponentDefaultState(imageLayerId, projectId) - ); - setIsLoading(false); - } - + // eslint-disable-next-line react-hooks/set-state-in-effect initComponent(); return () => { @@ -80,19 +96,20 @@ const CreateEditImageLayerModal = () => { // eslint-disable-next-line react-hooks/exhaustive-deps }, []); - useEffect(() => { - if (componentState) { - setIsUploading( - validateIsUploading( - componentState.preEventImageryUrls, - componentState.postEventImageryUrls, - componentState.userBuildingFootprintsUrls || [] - ) - ); - } - - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [componentState]); + if (loadError) { + return ( +
+ + + Image layer details could not be loaded. + + + +
+ ); + } /* SUBMIT FUNCTION */ async function submit() { diff --git a/ui/src/Components/CreateEditImageLayerHelper.js b/ui/src/Components/CreateEditImageLayerHelper.js index 79b7ec26..8cd672bb 100644 --- a/ui/src/Components/CreateEditImageLayerHelper.js +++ b/ui/src/Components/CreateEditImageLayerHelper.js @@ -13,6 +13,7 @@ import { normalizeSourceTypeKey, } from "./sourceTypeOptions.js"; import { sourceImageryRef } from "./OpenDataCatalog/openDataCatalog.js"; +import { loadImageLayerFormData } from "./loadImageLayerFormData.js"; export { sourceTypeOptions, normalizeSourceTypeKey }; @@ -23,14 +24,11 @@ const imageryOriginOptions = [ export async function createComponentDefaultState(imageLayerId, projectId) { try { - //const settings = await apiGet("GetAdminSettings"); - var imageLayerToEdit = null; - if (imageLayerId) { - imageLayerToEdit = await apiGet("GetLayerDetailView?projectId=" + projectId + "&imageLayerId=" + imageLayerId); - } - - // Get Project Name - const project = await apiGet("GetProjectDetails?projectId=" + projectId); + const { imageLayerToEdit, project } = await loadImageLayerFormData( + imageLayerId, + projectId, + apiGet, + ); const tempState = imageLayerToEdit ? { @@ -116,6 +114,7 @@ export async function createComponentDefaultState(imageLayerId, projectId) { return tempState; } catch (error) { console.error("Error inializing component:", error); + throw error; } } diff --git a/ui/src/Components/CreateEditImageLayerHelper.test.js b/ui/src/Components/CreateEditImageLayerHelper.test.js index 0ea2bc11..1f932552 100644 --- a/ui/src/Components/CreateEditImageLayerHelper.test.js +++ b/ui/src/Components/CreateEditImageLayerHelper.test.js @@ -1,11 +1,60 @@ -import test from "node:test"; import assert from "node:assert/strict"; +import test from "node:test"; +import { loadImageLayerFormData } from "./loadImageLayerFormData.js"; import { sourceTypeOptions, normalizeSourceTypeKey, } from "./sourceTypeOptions.js"; +function deferred() { + let resolve; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + +test("loads edit layer and project details concurrently", async () => { + const layer = deferred(); + const project = deferred(); + const calls = []; + const loading = loadImageLayerFormData( + "layer-1", + "project-1", + (endpoint) => { + calls.push(endpoint); + return endpoint.startsWith("GetLayerDetailView") + ? layer.promise + : project.promise; + } + ); + + assert.equal(calls.length, 2); + layer.resolve({ imageLayerId: "layer-1" }); + project.resolve({ projectId: "project-1" }); + + assert.deepEqual(await loading, { + imageLayerToEdit: { imageLayerId: "layer-1" }, + project: { projectId: "project-1" }, + }); +}); + +test("create mode requests only project details", async () => { + const calls = []; + const result = await loadImageLayerFormData( + null, + "project-1", + async (endpoint) => { + calls.push(endpoint); + return { projectId: "project-1" }; + } + ); + + assert.deepEqual(calls, ["GetProjectDetails?projectId=project-1"]); + assert.equal(result.imageLayerToEdit, null); +}); + test("lists only the supported visible imagery source types", () => { const visibleSourceKeys = sourceTypeOptions .filter((option) => option.showInDropdown) diff --git a/ui/src/Components/HelpDocs/HelpDocsImageLayers.jsx b/ui/src/Components/HelpDocs/HelpDocsImageLayers.jsx index bdbc5347..bd323509 100644 --- a/ui/src/Components/HelpDocs/HelpDocsImageLayers.jsx +++ b/ui/src/Components/HelpDocs/HelpDocsImageLayers.jsx @@ -8,10 +8,6 @@ import PropTypes from 'prop-types'; import { useEffect } from 'react'; const HelpDocsImageLayers = ({ anchor }) => { - HelpDocsImageLayers.propTypes = { - anchor: PropTypes.string, - }; - useEffect(() => { if (anchor) { const element = document.getElementsByName(anchor)[0]; @@ -51,7 +47,7 @@ const HelpDocsImageLayers = ({ anchor }) => {

Create a New Image Layer

-

To create an Image Layer, you must first create a project. Once this is done, select the desired project from the list of projects. The project details will be displayed, which includes a button called "Create Image Layer." Clicking this will take you to the Image Layer creation form.

+

To create an Image Layer, you must first create a project. Once this is done, select the desired project from the list of projects. The project details will be displayed, which includes a button called "Create Image Layer." Clicking this will take you to the Image Layer creation form.

Browse the Open Data Catalog

The Open Data Catalog is the fastest way to add public disaster imagery without finding and copying source URLs manually. On the Create Image Layer form, select Browse Open Data Catalog, then:

@@ -67,7 +63,7 @@ const HelpDocsImageLayers = ({ anchor }) => {

Add imagery files by providing publicly accessible URLs or uploading files from a local directory that show the Area of Interest (AOI). You can also combine files from both a URL and a local directory. If multiple files are provided in a section, they will be merged into a single GeoTIFF image; therefore, all files in each section must correspond to the same AOI. All files must be valid GeoTIFF (.tif) files.

-