diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bc4a4821be..1e35a0cc27 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -314,6 +314,23 @@ jobs: # - PersistentMediaDirCheckSpec (#4925): user-uploaded media must resolve outside the application directory, # which a deploy rebuilds and deletes. Pure logic, no DB — and the failure it guards against is invisible to # every other test, since the broken config behaves correctly right up until the next release. + # - MediaIntegritySpec (#4926): the media-storage panel's calls on what counts as data loss. Pure logic, no DB. + # Its false-alarm cases matter as much as its real ones — a monitor that cries loss over an unreadable + # directory gets ignored, and an ignored monitor leaves us exactly where #4925 found us. + # - LostMediaLogSpec (#4926): the log line that is the only signal a media file has been destroyed. Both ways + # it can fail are silent — announcing every request buries the event, announcing nothing reads as health — + # so the lines themselves are asserted. Pure logic, no DB. + # - SingleFlightGateSpec (#4926): keeps a stuck media scan from being joined by a fresh copy every poll until + # the blocking-io pool is gone. Its whole job is invisible until storage hangs, and by then the panel that + # would have reported it is the thing that went dark. Pure logic, no DB. + # - HealthMediaPayloadSpec (#4926): the field names the health page reads the media panel by. A renamed case + # class field emits a different key, every value goes undefined, and a monitor reporting nothing looks + # exactly like a monitor reporting nothing wrong. Pure logic, no DB. + # - ImageControllerSpec (#4415, #4726, #4926): the crop write path and its share-preview invalidation, plus the + # loss lines a signed crop/pano URL emits when the bytes it was signed for are gone. Nothing else asserts the + # controller calls LostMediaLog at all, so without this the tripwire could be deleted with CI still green. + # - StoryUploadWindowSpec (#4926): the grace window that stops the loss tripwire firing at its own upload path. + # Reporting is once per media id, so a false alarm spends the line a real loss would have needed. Pure logic. # - PanoExpiredAtSpec / BackgroundJobRunTableSpec / JobRunServiceSpec / StreetLifecycleServiceSpec (#4928): the # transition records the admin trend and nightly-job panels read. Each seeds its own rows rather than hunting # for them, so they assert real behavior against an empty seed: the expiry stamp only lands on the false -> @@ -333,7 +350,7 @@ jobs: # the second as a success is how a rotated-out API key ends the re-audit signal behind a permanently green # badge. - name: Run gating/auth tests (health dashboard + route auth posture + geodesic distances) - run: sbt 'set Test / parallelExecution := false' 'testOnly controllers.HealthDashboardSpec service.HealthServiceSpec controllers.RouteAuthPostureSpec models.street.GeodesicDistanceSpec service.ExploreTutorialRouteSpec controllers.MobileDetectionSpec service.PanoDataServiceSpec service.ActivityBreakdownSpec service.ConfigServiceTrendSpec models.utils.CityScorecardSpec models.utils.ConfigTableVoidedArchiveSpec controllers.api.StatsApiSpec controllers.ExploreSubmissionSpec controllers.ValidateSubmissionSpec controllers.ExploreNoImageryRateLimitSpec models.pano.PanoExpiredAtSpec models.utils.BackgroundJobRunTableSpec service.JobRunServiceSpec service.NightlyJobStatusSpec service.StreetLifecycleServiceSpec models.audit.OutdatedImageryFlagSyncSpec models.audit.OutdatedImageryRoutingSpec models.street.UpToDateCoverageSpec models.street.NoImageryReportsSpec models.utils.EnumTypeParitySpec actor.ScheduledJobsSpec service.ImageryPollOutcomeSpec modules.PersistentMediaDirCheckSpec' + run: sbt 'set Test / parallelExecution := false' 'testOnly controllers.HealthDashboardSpec service.HealthServiceSpec controllers.RouteAuthPostureSpec models.street.GeodesicDistanceSpec service.ExploreTutorialRouteSpec controllers.MobileDetectionSpec service.PanoDataServiceSpec service.ActivityBreakdownSpec service.ConfigServiceTrendSpec models.utils.CityScorecardSpec models.utils.ConfigTableVoidedArchiveSpec controllers.api.StatsApiSpec controllers.ExploreSubmissionSpec controllers.ValidateSubmissionSpec controllers.ExploreNoImageryRateLimitSpec models.pano.PanoExpiredAtSpec models.utils.BackgroundJobRunTableSpec service.JobRunServiceSpec service.NightlyJobStatusSpec service.StreetLifecycleServiceSpec models.audit.OutdatedImageryFlagSyncSpec models.audit.OutdatedImageryRoutingSpec models.street.UpToDateCoverageSpec models.street.NoImageryReportsSpec models.utils.EnumTypeParitySpec actor.ScheduledJobsSpec service.ImageryPollOutcomeSpec modules.PersistentMediaDirCheckSpec service.MediaIntegritySpec service.LostMediaLogSpec service.SingleFlightGateSpec service.HealthMediaPayloadSpec controllers.ImageControllerSpec controllers.StoryUploadWindowSpec' env: DATABASE_URL: jdbc:postgresql://localhost:5432/sidewalk DATABASE_USER: sidewalk diff --git a/app/controllers/ImageController.scala b/app/controllers/ImageController.scala index af7e0e43db..569216c88e 100644 --- a/app/controllers/ImageController.scala +++ b/app/controllers/ImageController.scala @@ -8,7 +8,7 @@ import models.label.LabelTypeEnum import play.api.libs.json._ import play.api.mvc.{AnyContent, Request, RequestHeader} import play.api.{Configuration, Logger} -import service.ImageSigningService +import service.{ImageSigningService, LostMediaLog} import java.awt.Image import java.awt.image.BufferedImage @@ -25,6 +25,7 @@ class ImageController @Inject() ( signingService: ImageSigningService, shareImageCache: service.ShareImageCache, config: Configuration, + lostMediaLog: LostMediaLog, cpuEc: CpuIntensiveExecutionContext )(implicit ec: ExecutionContext) extends CustomBaseController(cc) { @@ -137,6 +138,15 @@ class ImageController @Inject() ( val contentType = if (file.getName.toLowerCase.endsWith(".png")) "image/png" else "image/jpeg" Future.successful(Ok.sendFile(file, inline = true).as(contentType)) case None => + // Reaching here means the file was on disk when this URL was signed (backupImageUrl and + // getBackupImageMetadata both check first) and is gone within the signature's ~75-minute life. For an + // pano the provider no longer serves nothing can re-fetch it, so say so (#4926) — the 404 stays bare. + lostMediaLog.reportMissing( + "pano", + panoId, + s"${panoDataService.backupImageDir(panoId).getAbsolutePath}/$panoId.{jpg,jpeg,png}", + irreplaceable = true + ) Future.successful(NotFound(s"Pano image not found: $panoId")) } } @@ -188,6 +198,10 @@ class ImageController @Inject() ( if (file.exists()) { Future.successful(Ok.sendFile(file, inline = true).as("image/png")) } else { + // Same signed-URL reasoning as serveBackupImage above: cropUrl only signs a crop it just saw on disk, so a + // miss here is a file that vanished — and nothing recreates it: the crop was captured in the labeler's + // browser at labeling time, from a pano the provider may no longer serve. + lostMediaLog.reportMissing("crop", s"$labelType/$labelId", file.getAbsolutePath, irreplaceable = true) Future.successful(NotFound("Crop image not found")) } } diff --git a/app/controllers/StoryController.scala b/app/controllers/StoryController.scala index 4a0bcdec62..a92351f618 100644 --- a/app/controllers/StoryController.scala +++ b/app/controllers/StoryController.scala @@ -9,11 +9,10 @@ import models.story.{Story, StoryMedia, StoryPhotoUpload, StoryRejection} import play.api.libs.json.{JsBoolean, Json} import play.api.{Configuration, Logger} import play.silhouette.api.Silhouette -import service.{ConfigService, ImageSigningService, RateLimiter, StoryService} +import service.{ConfigService, ImageSigningService, LostMediaLog, RateLimiter, StoryService} import java.io.File import java.time.{Duration, OffsetDateTime} -import java.util.concurrent.ConcurrentHashMap import javax.inject.{Inject, Singleton} import scala.concurrent.{ExecutionContext, Future} import scala.util.Try @@ -33,6 +32,7 @@ class StoryController @Inject() ( storyService: StoryService, signingService: ImageSigningService, rateLimiter: RateLimiter, + lostMediaLog: LostMediaLog, implicit val ec: ExecutionContext ) extends CustomBaseController(cc) { private val logger = Logger(this.getClass) @@ -228,27 +228,16 @@ class StoryController @Inject() ( } } - // serveStoryMedia's data-loss tripwire (#4925): one entry per media id already reported, so a busy page - // re-requesting one lost file reads as one loss in the log, not hundreds. - private val lostMediaLogged = ConcurrentHashMap.newKeySet[Int]() - - // The upload flow commits the media row before the file move lands (StoryService's place-before-commit windows), so - // a row this young with no bytes is almost certainly mid-upload, not loss. The window is sub-second; a minute is - // generous slack. - private val lostMediaGrace = Duration.ofMinutes(1) - /** - * Logs a media row whose bytes are missing from disk. This error is a post-#4925 tripwire someone is expected to - * investigate, so it is kept high-signal: skipped inside the upload grace window (a false alarm has real cost) and - * logged once per media id per instance. + * Reports a media row whose bytes are missing from disk (#4925). Kept high-signal by skipping the upload grace + * window, since a false alarm on this tripwire has real cost; `LostMediaLog` handles the rest. * * @param media The media row whose file is missing. * @param file Where the bytes should have been. */ private def logLostMedia(media: StoryMedia, file: File): Unit = { - val inUploadWindow = media.createdAt.isAfter(OffsetDateTime.now.minus(lostMediaGrace)) - if (!inUploadWindow && lostMediaLogged.add(media.storyMediaId)) { - logger.error(s"story_media ${media.storyMediaId} has no file on disk at ${file.getAbsolutePath}") + if (!StoryController.withinUploadWindow(media.createdAt, OffsetDateTime.now)) { + lostMediaLog.reportMissing("story_media", media.storyMediaId.toString, file.getAbsolutePath, irreplaceable = true) } } @@ -312,4 +301,22 @@ object StoryController { /** Cap on the stories the /stories page renders, so the page stays bounded as a city's story count grows. */ val ListingMax: Int = 500 + + // The upload flow commits the media row before the file move lands (StoryService's place-before-commit windows), + // so a row this young with no bytes is almost certainly mid-upload. The window is sub-second; a minute is generous + // slack. + private val lostMediaGrace = Duration.ofMinutes(1) + + /** + * Whether a media row is young enough that missing bytes read as an upload in progress rather than as loss. + * + * The data-loss log deduplicates per item, so a false alarm doesn't merely add noise — it spends the one line that + * media id will ever get, and a real loss discovered later says nothing at all. + * + * @param createdAt When the media row was committed. + * @param now The instant to judge it against. + * @return True while the row is inside the upload window and its file may still be landing. + */ + private[controllers] def withinUploadWindow(createdAt: OffsetDateTime, now: OffsetDateTime): Boolean = + createdAt.isAfter(now.minus(lostMediaGrace)) } diff --git a/app/executors/CustomExecutionContexts.scala b/app/executors/CustomExecutionContexts.scala index c4a2bd6336..fd858d7d74 100644 --- a/app/executors/CustomExecutionContexts.scala +++ b/app/executors/CustomExecutionContexts.scala @@ -27,3 +27,17 @@ object CpuIntensiveExecutionContext { extends CustomExecutionContext(system, "cpu-intensive") with CpuIntensiveExecutionContext } + +/** + * Execution context for blocking filesystem operations, isolated so that a call that never returns — a stat or a + * directory listing against an unreachable network mount — can only exhaust this pool and not the ones serving + * requests or running streams. + */ +trait BlockingIoExecutionContext extends ExecutionContext + +object BlockingIoExecutionContext { + @Singleton + class PekkoBased @Inject() (system: ActorSystem) + extends CustomExecutionContext(system, "blocking-io") + with BlockingIoExecutionContext +} diff --git a/app/models/utils/HealthTable.scala b/app/models/utils/HealthTable.scala index a9b443c052..4a3bb28224 100644 --- a/app/models/utils/HealthTable.scala +++ b/app/models/utils/HealthTable.scala @@ -54,6 +54,12 @@ class HealthTable @Inject() (protected val dbConfigProvider: DatabaseConfigProvi implicit private val grDbEnvInfo: GetResult[DbEnvInfo] = GetResult(r => DbEnvInfo(r.nextString(), r.nextString(), r.nextBoolean())) + implicit private val grSchemaRowCount: GetResult[SchemaRowCount] = + GetResult(r => SchemaRowCount(r.nextString(), r.nextInt())) + + implicit private val grSchemaMediaId: GetResult[SchemaMediaId] = + GetResult(r => SchemaMediaId(r.nextString(), r.nextInt())) + /** * Caps a health read so it can never hold a pool connection for long. A monitoring query must not add load — least * of all when the database is already stressed, which is exactly when this dashboard gets opened. `statement_timeout` @@ -275,4 +281,63 @@ class HealthTable @Inject() (protected val dbConfigProvider: DatabaseConfigProvi LEFT JOIN pano_data pd ON pd.pano_id = labeled.pano_id """.as[PanoBackupStats].head } + + /** + * The schema this instance actually reads and writes, which is what the media scan resolves its own city's + * directory from. Config alone can't answer it: `city-id` and the connection's schema are separate settings and a + * misconfigured instance has them disagreeing, which would make the scan look for files somewhere the app never + * writes them. + */ + def getCurrentSchema: DBIO[String] = bounded { sql"""SELECT current_schema()""".as[String].head } + + /** + * Schemas holding a readable `story_media` table, for the cross-city media-integrity scan (#4926). + * + * Same shape as [[getEvolutionSchemas]], including its trap: the table-privilege check takes the `pg_class` oid + * rather than a name, because the planner may evaluate the predicates in any order and the name-based form raises + * an error (instead of returning false) on a relation that the other filters would have excluded. + */ + def getStoryMediaSchemas: DBIO[Seq[String]] = bounded { + sql"""SELECT nspname FROM pg_catalog.pg_class + JOIN pg_catalog.pg_namespace ON pg_namespace.oid = pg_class.relnamespace + WHERE relname = 'story_media' AND relkind = 'r' + AND has_schema_privilege(nspname, 'USAGE') + AND has_table_privilege(pg_class.oid, 'SELECT') + ORDER BY nspname""".as[String] + } + + /** + * How many `story_media` rows each schema holds, in ONE query — the same `UNION ALL` construction and the same + * reason as [[getStuckEvolutionsForSchemas]]: a per-schema fan-out would want ~50 pool connections per poll on + * prod. Read before the ids so the scan can decline an implausibly large result rather than pulling it into heap. + * + * Every `schema` MUST already be validated as a bare identifier by the caller; the names are spliced literally. + * + * @param schemas Validated schema names to count. Must be non-empty. + * @return One row per schema, including the schemas that hold no media at all. + */ + def getStoryMediaCounts(schemas: Seq[String]): DBIO[Seq[SchemaRowCount]] = { + require(schemas.nonEmpty, "getStoryMediaCounts requires at least one schema (empty input builds invalid SQL)") + val union = schemas + .map(schema => s"""SELECT text '$schema' AS schema, count(*)::int AS media_rows FROM "$schema".story_media""") + .mkString("\nUNION ALL\n") + bounded(sql"""#$union""".as[SchemaRowCount]) + } + + /** + * Every `story_media` id in the given schemas, in ONE `UNION ALL` query. The ids themselves are needed — a count + * comparison would call a city with one lost photo and one orphaned file clean. + * + * Every `schema` MUST already be validated as a bare identifier by the caller; the names are spliced literally. + * + * @param schemas Validated schema names known to hold at least one row. Must be non-empty. + * @return One row per media id, tagged with its schema. + */ + def getStoryMediaIds(schemas: Seq[String]): DBIO[Seq[SchemaMediaId]] = { + require(schemas.nonEmpty, "getStoryMediaIds requires at least one schema (empty input builds invalid SQL)") + val union = schemas + .map(schema => s"""SELECT text '$schema' AS schema, story_media_id FROM "$schema".story_media""") + .mkString("\nUNION ALL\n") + bounded(sql"""#$union""".as[SchemaMediaId]) + } } diff --git a/app/modules/ExecutorsModule.scala b/app/modules/ExecutorsModule.scala index 632962820b..586caa9e27 100644 --- a/app/modules/ExecutorsModule.scala +++ b/app/modules/ExecutorsModule.scala @@ -9,5 +9,6 @@ import executors._ class ExecutorsModule extends AbstractModule { override def configure(): Unit = { bind(classOf[CpuIntensiveExecutionContext]).to(classOf[CpuIntensiveExecutionContext.PekkoBased]).asEagerSingleton() + bind(classOf[BlockingIoExecutionContext]).to(classOf[BlockingIoExecutionContext.PekkoBased]).asEagerSingleton() } } diff --git a/app/modules/PersistentMediaDirCheck.scala b/app/modules/PersistentMediaDirCheck.scala index f281667d50..f81c729eb1 100644 --- a/app/modules/PersistentMediaDirCheck.scala +++ b/app/modules/PersistentMediaDirCheck.scala @@ -1,6 +1,6 @@ package modules -import modules.PersistentMediaDirCheck.unsafeDirs +import modules.PersistentMediaDirCheck.{arms, unsafeDirs} import play.api.{Configuration, Environment, Logger, Mode} import service.MediaDirs @@ -23,17 +23,17 @@ import scala.util.{Failure, Success, Try} * incomplete file (the #4925 failure) would disarm the guard exactly when it is needed. Dev and test runs skip it; * there the application root is the hand-managed repo checkout, where the relative defaults are the point. * - * A directory holding irreplaceable bytes — user uploads, or our only copies of provider-expired imagery — is - * **fatal**: refusing to boot is far cheaper than accepting content we already know will be destroyed, and the test - * stage redeploys on every push to `develop` while prod waits for a release tag, so a missing variable surfaces on - * test long before it can reach prod. The rest is derived data whose loss costs rebuild time, so it logs and lets + * A directory holding irreplaceable bytes — user uploads, label crops, or imagery that cannot be re-fetched — is + * **fatal**: refusing to boot is far cheaper than accepting content we already know will be destroyed, + * and the test stage redeploys on every push to `develop` while prod waits for a release tag, so a missing variable + * surfaces on test long before it can reach prod. Cached share previews rebuild on demand, so that one logs and lets * the app run. */ @Singleton class PersistentMediaDirCheck @Inject() (config: Configuration, environment: Environment) { private val logger = Logger(this.getClass) - if (environment.mode == Mode.Prod) { + if (arms(environment)) { val unsafe = unsafeDirs(config, environment) unsafe.foreach(u => logger.error(u.reason)) @@ -59,23 +59,37 @@ object PersistentMediaDirCheck { * @param key Config key naming the directory. * @param envVar Environment variable a deployment sets it with, named in the failure message so the fix * doesn't require reading the config. - * @param irreplaceable Whether it holds bytes no rebuild can recreate — user uploads, or the only surviving copy - * of provider-expired imagery. These form the fatal tier; the rest only log. + * @param irreplaceable Whether it holds bytes no rebuild can recreate. These form the fatal tier; the rest only + * log. */ case class PersistentDir(key: String, envVar: String, irreplaceable: Boolean) /** A persistent directory that failed the check, with the loggable reason. */ case class UnsafeDir(dir: PersistentDir, reason: String) + /** + * Whether the check arms in this run mode. Defined here, next to the `if` it gates, because the Health dashboard + * reports what the check makes of each directory and has to agree with it about whether it is even watching — two + * copies of the rule would let the page claim a stage is guarded when it isn't. + * + * @param environment Play environment supplying the run mode. + * @return True in prod mode, which every staged binary runs in; false in the dev and test runs where + * the application root is a hand-managed checkout and the relative defaults are the point. + */ + def arms(environment: Environment): Boolean = environment.mode == Mode.Prod + val persistentDirs: Seq[PersistentDir] = Seq( - // Crops and share previews are derived: a crop can be re-cut from pano imagery and a share preview rebuilds on - // demand, so losing them costs rebuild time, not content. - PersistentDir("cropped.image.directory", "SIDEWALK_IMAGES_DIR", irreplaceable = false), - PersistentDir("share.image.directory", "SIDEWALK_SHARE_IMAGES_DIR", irreplaceable = false), - // The self-hosted pano store backs up GSV imagery Google has already expired (pano_data.has_backup) — for those - // panos it is the only copy left anywhere, as unrecoverable as a user upload. + // A crop is a screenshot of the pano canvas taken in the labeler's browser as the label was placed + // (Canvas.saveCanvasScreenshot); nothing here rebuilds one, and the Static API still we fall back to is a + // different image that only exists while the provider still serves that pano — about half the labels on prod sit + // on panos already marked expired. + PersistentDir("cropped.image.directory", "SIDEWALK_IMAGES_DIR", irreplaceable = true), + // Locally stored pano imagery (pano_data.has_backup) the providers no longer serve, so it cannot be re-fetched. PersistentDir("pano.images.directory", "SIDEWALK_PANO_DIR", irreplaceable = true), - PersistentDir("story.media.directory", "SIDEWALK_STORY_MEDIA_DIR", irreplaceable = true) + PersistentDir("story.media.directory", "SIDEWALK_STORY_MEDIA_DIR", irreplaceable = true), + // Share previews are the one cache here: each rebuilds on demand from the label's crop, or from a Street View + // still when the crop is gone. + PersistentDir("share.image.directory", "SIDEWALK_SHARE_IMAGES_DIR", irreplaceable = false) ) /** diff --git a/app/service/HealthService.scala b/app/service/HealthService.scala index 17cbc25eaa..210b5d1167 100644 --- a/app/service/HealthService.scala +++ b/app/service/HealthService.scala @@ -2,17 +2,21 @@ package service import actor.ScheduledJobs import com.google.inject.ImplementedBy +import executors.BlockingIoExecutionContext import models.utils.{BackgroundJobRun, BackgroundJobRunTable, HealthTable, JobRunStatus, JobRunTrigger} +import org.apache.pekko.actor.ActorSystem +import org.apache.pekko.pattern.after import play.api.cache.AsyncCacheApi import play.api.db.slick.{DatabaseConfigProvider, HasDatabaseConfigProvider} import play.api.libs.json._ -import play.api.{Configuration, Logger} +import play.api.{Configuration, Environment, Logger} import models.utils.MyPostgresProfile +import java.io.File import java.time.temporal.ChronoUnit import java.time.OffsetDateTime import javax.inject._ -import scala.concurrent.duration.Duration +import scala.concurrent.duration.{Duration, DurationInt, FiniteDuration} import scala.concurrent.{ExecutionContext, Future} /** A session that currently blocks one or more other sessions from acquiring a lock. */ @@ -86,6 +90,91 @@ case class PanoBackupStats( /** The connecting role's environment: database, role, and whether it can read every session's statement text. */ case class DbEnvInfo(database: String, role: String, canSeeAllQueries: Boolean) +/** How many `story_media` rows one schema holds. Internal to the media scan, not part of the dashboard payload. */ +case class SchemaRowCount(schema: String, rows: Int) + +/** One `story_media` id, tagged with its schema. Internal to the media scan, not part of the dashboard payload. */ +case class SchemaMediaId(schema: String, storyMediaId: Int) + +/** + * One persistent media directory as this instance resolves it. + * + * `label` and `severity` are computed server-side so the page holds no copy of the rules that decide when a + * directory is a problem — the same reason [[HealthThresholds]] travels in the payload. + * + * @param key Config key naming the directory. + * @param envVar Environment variable a deployment sets it with. + * @param irreplaceable Whether its contents are content rather than a rebuildable cache. + * @param path Where it resolves on this instance. + * @param status Machine-readable state: `ok`, `absent`, `not_writable`, `unsafe`, `unresolved`. + * @param label Display text for that state. + * @param severity Badge tone: `good`, `ok`, `warn`, `bad`. + * @param detail Longer explanation when there is one to give. + */ +case class MediaDirStatus( + key: String, + envVar: String, + irreplaceable: Boolean, + path: String, + status: String, + label: String, + severity: String, + detail: Option[String] +) + +/** + * One city's `story_media` rows measured against the files in its media directory. + * + * @param cityId City the schema belongs to; None when this instance's config doesn't name it. + * @param schema Database schema the rows came from. + * @param rows `story_media` rows in that schema. + * @param missing Rows whose bytes are gone — data loss (#4925). + * @param orphans Files with no row — a retraction whose file delete didn't land (#4054). + * @param missingIds Sample of the missing ids, to start looking from. + * @param orphanIds Sample of the orphaned ids. + * @param scanned False when the directory couldn't be read, so the counts mean nothing. + * @param unscannedReason Why it wasn't scanned, when it wasn't. The reasons differ enough — a schema no city on this + * stage claims, a directory this instance writes under another schema, one it may not read — + * that a single "not scanned" would send an operator looking in the wrong place. + */ +case class CityStoryMedia( + cityId: Option[String], + schema: String, + rows: Int, + missing: Int, + orphans: Int, + missingIds: Seq[Int], + orphanIds: Seq[Int], + scanned: Boolean, + unscannedReason: Option[String] +) + +/** + * The story-media integrity scan across every city schema visible from this instance. + * + * @param baseDir The resolved base directory the per-city subdirectories live under. + * @param cities One row per schema holding a `story_media` table, in schema order. + * @param missing Total rows with no file, across every scanned city. + * @param orphans Total files with no row, across every scanned city. + */ +case class StoryMediaIntegrity(baseDir: String, cities: Seq[CityStoryMedia], missing: Int, orphans: Int) + +/** + * The media-storage panel: where this instance keeps persistent media, and whether any of it has gone missing. + * + * @param directories One status per directory the boot check guards. + * @param enforced Whether `PersistentMediaDirCheck` arms in this run mode; false in dev, where the relative + * defaults landing in the checkout is the intended behavior rather than a fault. + * @param storyMedia The integrity scan, or None when it couldn't run. + * @param unavailable Why the scan couldn't run, when it didn't. + */ +case class MediaStorageHealth( + directories: Seq[MediaDirStatus], + enforced: Boolean, + storyMedia: Option[StoryMediaIntegrity], + unavailable: Option[String] +) + /** * Server-owned thresholds the dashboard uses to color each panel, echoed in the payload so the frontend never * hard-codes them (CLAUDE.md: domain values come from the backend). Seconds unless noted. @@ -157,6 +246,7 @@ case class DbHealthData( tableBloat: Seq[TableBloat], connections: Seq[ConnCount], panoBackups: Option[PanoBackupStats], + mediaStorage: Option[MediaStorageHealth], nightlyJobs: Seq[NightlyJobStatus], thresholds: HealthThresholds ) @@ -180,8 +270,11 @@ trait HealthService { class HealthServiceImpl @Inject() ( protected val dbConfigProvider: DatabaseConfigProvider, config: Configuration, + environment: Environment, cacheApi: AsyncCacheApi, healthTable: HealthTable, + actorSystem: ActorSystem, + blockingIoEc: BlockingIoExecutionContext, backgroundJobRunTable: BackgroundJobRunTable )(implicit val ec: ExecutionContext) extends HealthService @@ -251,8 +344,9 @@ class HealthServiceImpl @Inject() ( .recover { case e: Exception => logger.warn(s"Health: failed to read pano backup stats: ${e.getMessage}"); None } - val evoF = getStuckEvolutions - val jobsF = cacheApi + val mediaF = getMediaStorage + val evoF = getStuckEvolutions + val jobsF = cacheApi .getOrElseUpdate[Seq[NightlyJobStatus]]("health.jobs", slowTtl)(getNightlyJobs) .recover(logAndEmpty("nightly jobs")) @@ -265,6 +359,7 @@ class HealthServiceImpl @Inject() ( bloat <- bloatF conn <- connF pano <- panoF + media <- mediaF jobs <- jobsF } yield DbHealthData( generatedAt = OffsetDateTime.now().toString, @@ -278,6 +373,7 @@ class HealthServiceImpl @Inject() ( tableBloat = bloat, connections = conn, panoBackups = pano, + mediaStorage = media, nightlyJobs = jobs, thresholds = thresholds ) @@ -380,6 +476,155 @@ class HealthServiceImpl @Inject() ( } } + // Ceiling on `story_media` rows the scan will pull into memory. Prod holds a single-digit number today; this is a + // guard against a future where stories take off, not a working limit. Exceeding it reports the scan as unavailable + // rather than silently comparing a truncated set, which would invent orphans out of the rows it never fetched. + private val maxStoryMediaRows = 200000 + + // Wall-clock ceiling on the filesystem half of the scan. The directories can sit on a network mount, and a stat + // against a dead mount never returns — the thread stays parked on the blocking pool, but the poll must not. + private val mediaScanTimeout: FiniteDuration = 5.seconds + + // One scan at a time: `withTimeout` protects the poll from a stuck filesystem call, but only this keeps a stuck one + // from being joined by a fresh copy every poll until the whole blocking-io pool is parked. See [[SingleFlightGate]]. + private val mediaScanGate = new SingleFlightGate + + // Schema -> city id, read from configuration alone. The database knows the schema, the directory is named for the + // city, and nothing but this mapping joins them. ConfigService.availableCityIds would do it with one existence + // query per city — the ~50-connection fan-out this dashboard exists to catch (#4559). + private lazy val cityIdBySchema: Map[String, String] = config + .get[Seq[String]]("city-params.city-ids") + .flatMap(cityId => config.getOptional[String](s"city-params.db-schema.$cityId").map(_ -> cityId)) + .toMap + + /** + * Where this instance keeps persistent media, and whether any `story_media` row has lost its bytes (#4926). + * + * The whole signal is cached at the pano TTL: it is the slowest one here (a database round trip plus a directory + * listing per city) and the thing it detects — a deploy having deleted a directory — does not change minute to + * minute. A failure yields None so the panel can say so, rather than sinking the rest of the dashboard. + */ + private def getMediaStorage: Future[Option[MediaStorageHealth]] = { + cacheApi + .getOrElseUpdate[Option[MediaStorageHealth]]("health.media", panoTtl) { + // A scan still running past its deadline all but always means a filesystem call that will not return. + // Reporting that beats both starting another one on top of it and rendering a stale all-clear. + val busy = Some(unreachableMedia("A previous media scan has not returned; storage may be offline.")) + // The gate opens on the underlying scan, not on the timeout, so a stuck one keeps the next poll out. + withTimeout( + mediaScanGate.runOrElse(busy) { + for { + dirs <- Future(MediaIntegrity.directoryStatuses(config, environment))(blockingIoEc) + integrity <- storyMediaIntegrity + } yield Some(MediaStorageHealth(dirs, enforced, integrity._1, integrity._2)) + }, + "media storage scan" + ) + } + .recover { case e: Exception => + logger.warn(s"Health: failed to read media storage: ${e.getMessage}"); None + } + } + + /** Whether the boot check arms on this instance; read from the check itself so the page can't claim a false guard. */ + private def enforced: Boolean = modules.PersistentMediaDirCheck.arms(environment) + + /** The panel with nothing but a reason on it, for when even stat-ing the directories would block. */ + private def unreachableMedia(reason: String): MediaStorageHealth = + MediaStorageHealth(Seq.empty, enforced, None, Some(reason)) + + /** + * Compares every visible city's `story_media` rows against the files on disk. + * + * Reads the schemas, then their row counts, then their ids — three cheap round trips behind a five-minute cache + * rather than one query per city. If the base directory itself is unreadable, the scan reports itself unavailable + * instead of declaring every row lost: a monitor that cries data loss over a missing mount would be worse than no + * monitor at all. + * + * @return The scan, or the reason it couldn't run. + */ + private def storyMediaIntegrity: Future[(Option[StoryMediaIntegrity], Option[String])] = { + // Resolve inside the blocking future: MediaDirs.baseDir throws on an unusable value, and a synchronous throw + // here would escape the caller's `.recover` instead of degrading to an unavailable panel. + Future { + val baseDir = MediaDirs.baseDir(config, environment, "story.media.directory") + (baseDir, MediaIntegrity.scanRefusal(baseDir.getAbsolutePath, baseDir.isDirectory, baseDir.canRead)) + }(blockingIoEc).flatMap { + case (_, Some(refusal)) => Future.successful((None, Some(refusal))) + case (baseDir, None) => + db.run(healthTable.getStoryMediaSchemas).map(_.filter(_.matches("^[A-Za-z0-9_]+$"))).flatMap { schemas => + if (schemas.isEmpty) + Future.successful((Some(StoryMediaIntegrity(baseDir.getAbsolutePath, Seq.empty, 0, 0)), None)) + else + db.run(healthTable.getStoryMediaCounts(schemas)).flatMap { counts => + val total = counts.map(_.rows).sum + if (total > maxStoryMediaRows) { + Future.successful((None, Some(s"$total story_media rows is more than this scan will load at once."))) + } else { + val populated = counts.filter(_.rows > 0).map(_.schema) + val idsF = + if (populated.isEmpty) Future.successful(Seq.empty[SchemaMediaId]) + else db.run(healthTable.getStoryMediaIds(populated)) + for { + ids <- idsF + current <- db.run(healthTable.getCurrentSchema) + cities <- scanCities(baseDir, current, counts, ids) + } yield (Some(cities), None) + } + } + } + } + } + + /** + * Lists each city's directory once and diffs it against that city's ids — one listing per city, whatever the row + * count, which is what keeps this affordable as stories grow. + * + * This instance's own schema takes its city from `city-id` rather than from the schema mapping, because that is + * what `StoryService` builds its write path from: the two settings are independent, and on an instance where they + * disagree the scan has to look where the files actually are rather than where the mapping says they should be. + * + * @param baseDir Resolved base directory holding the per-city subdirectories. + * @param currentSchema The schema this instance reads and writes. + * @param counts Row counts per schema, which decide the rows reported. + * @param ids Every media id, tagged with its schema. + */ + private def scanCities( + baseDir: File, + currentSchema: String, + counts: Seq[SchemaRowCount], + ids: Seq[SchemaMediaId] + ): Future[StoryMediaIntegrity] = { + val idsBySchema = ids.groupBy(_.schema).view.mapValues(_.map(_.storyMediaId)).toMap + val targets = MediaIntegrity.scanTargets( + counts.map(_.schema), + currentSchema, + config.get[String]("city-id"), + cityIdBySchema + ) + Future { + counts.sortBy(_.schema).map { case SchemaRowCount(schema, _) => + val mediaIds = idsBySchema.getOrElse(schema, Seq.empty) + targets.get(schema) match { + case Some(ScanTarget.Dir(cityId)) => + MediaIntegrity.compareCity(cityId, schema, mediaIds, MediaIntegrity.listing(new File(baseDir, cityId))) + case Some(ScanTarget.Unlocatable(reason)) => MediaIntegrity.unscannedCity(None, schema, mediaIds, reason) + case None => MediaIntegrity.unscannedCity(None, schema, mediaIds, s"no scan target for schema $schema") + } + } + }(blockingIoEc).map { cities => + StoryMediaIntegrity(baseDir.getAbsolutePath, cities, cities.map(_.missing).sum, cities.map(_.orphans).sum) + } + } + + /** Fails a future that outlives the media-scan budget, so one unreachable mount can't hold the poll open. */ + private def withTimeout[T](f: Future[T], label: String): Future[T] = { + val timeout = after(mediaScanTimeout, actorSystem.scheduler)( + Future.failed(new java.util.concurrent.TimeoutException(s"$label did not finish within $mediaScanTimeout")) + ) + Future.firstCompletedOf(Seq(f, timeout)) + } + private def logAndEmpty[T](label: String): PartialFunction[Throwable, Seq[T]] = { case e: Exception => logger.warn(s"Health: failed to read $label: ${e.getMessage}") Seq.empty[T] @@ -401,14 +646,18 @@ object HealthService { implicit private val jsonConfig: JsonConfiguration = JsonConfiguration(JsonNaming.SnakeCase) - implicit val blockingSessionWrites: Writes[BlockingSession] = Json.writes[BlockingSession] - implicit val idleTxnSessionWrites: Writes[IdleTxnSession] = Json.writes[IdleTxnSession] - implicit val activeQueryWrites: Writes[ActiveQuery] = Json.writes[ActiveQuery] - implicit val stuckEvolutionWrites: Writes[StuckEvolution] = Json.writes[StuckEvolution] - implicit val tableBloatWrites: Writes[TableBloat] = Json.writes[TableBloat] - implicit val connCountWrites: Writes[ConnCount] = Json.writes[ConnCount] - implicit val panoBackupStatsWrites: Writes[PanoBackupStats] = Json.writes[PanoBackupStats] - implicit val nightlyJobStatusWrites: Writes[NightlyJobStatus] = Json.writes[NightlyJobStatus] - implicit val healthThresholdsWrites: Writes[HealthThresholds] = Json.writes[HealthThresholds] - implicit val dbHealthDataWrites: Writes[DbHealthData] = Json.writes[DbHealthData] + implicit val blockingSessionWrites: Writes[BlockingSession] = Json.writes[BlockingSession] + implicit val idleTxnSessionWrites: Writes[IdleTxnSession] = Json.writes[IdleTxnSession] + implicit val activeQueryWrites: Writes[ActiveQuery] = Json.writes[ActiveQuery] + implicit val stuckEvolutionWrites: Writes[StuckEvolution] = Json.writes[StuckEvolution] + implicit val tableBloatWrites: Writes[TableBloat] = Json.writes[TableBloat] + implicit val connCountWrites: Writes[ConnCount] = Json.writes[ConnCount] + implicit val panoBackupStatsWrites: Writes[PanoBackupStats] = Json.writes[PanoBackupStats] + implicit val mediaDirStatusWrites: Writes[MediaDirStatus] = Json.writes[MediaDirStatus] + implicit val cityStoryMediaWrites: Writes[CityStoryMedia] = Json.writes[CityStoryMedia] + implicit val storyMediaIntegrityWrites: Writes[StoryMediaIntegrity] = Json.writes[StoryMediaIntegrity] + implicit val mediaStorageHealthWrites: Writes[MediaStorageHealth] = Json.writes[MediaStorageHealth] + implicit val nightlyJobStatusWrites: Writes[NightlyJobStatus] = Json.writes[NightlyJobStatus] + implicit val healthThresholdsWrites: Writes[HealthThresholds] = Json.writes[HealthThresholds] + implicit val dbHealthDataWrites: Writes[DbHealthData] = Json.writes[DbHealthData] } diff --git a/app/service/LostMediaLog.scala b/app/service/LostMediaLog.scala new file mode 100644 index 0000000000..9910025173 --- /dev/null +++ b/app/service/LostMediaLog.scala @@ -0,0 +1,56 @@ +package service + +import play.api.Logger + +import java.util.Collections +import javax.inject.Singleton + +/** + * Announces media whose bytes have gone missing, once per item. + * + * A media row with no file on disk answers every request with a plain 404, indistinguishable from an id that never + * existed — which is why a destroyed story photo went unnoticed for six days (#4925). The response has to stay a bare + * 404 (telling a prober which ids exist would be worse), so the log is the only place the loss can be stated. + * + * Deduplicated because the interesting event is the loss, not the traffic: one popular page re-requesting a lost file + * would otherwise write thousands of identical lines and bury it. The tracking set is bounded — a lost mount can make + * every pano in a city report at once — so eviction eventually lets a still-lost item announce itself again, which is + * the right way for this to fail. + */ +@Singleton +class LostMediaLog { + private val logger = Logger(this.getClass) + + // Comfortably above the number of distinct items any single incident produces, and small enough that the worst case + // (a whole city's panos reporting at once) costs a few hundred KB rather than growing with the pano table. + private val maxTrackedItems = 2000 + + // Access-ordered LRU: the eldest *least recently reported* key is evicted once the map is full. Only ever touched + // through `put` below, inside the synchronized wrapper — access order mutates on read, so unsynchronized reads + // would corrupt it. + private val reported: java.util.Map[String, java.lang.Boolean] = Collections.synchronizedMap( + new java.util.LinkedHashMap[String, java.lang.Boolean](256, 0.75f, true) { + override def removeEldestEntry(eldest: java.util.Map.Entry[String, java.lang.Boolean]): Boolean = + size() > maxTrackedItems + } + ) + + /** + * Logs a media item whose bytes are missing from disk, unless it has already been reported recently. + * + * Every caller goes through here so the lines share one shape and a loss inventory can be grepped out of the logs + * whole. Severity follows the same tiering as `PersistentMediaDirCheck`: content no rebuild can recreate is an + * error, derived content that can be regenerated is a warning. + * + * @param kind What was lost, named after its table or endpoint (`story_media`, `pano`, `crop`). + * @param id Identifies the item within its kind; included verbatim in the message. + * @param path Where the bytes should have been, for whoever goes looking. + * @param irreplaceable Whether losing this destroys content (error) rather than costing a rebuild (warning). + */ + def reportMissing(kind: String, id: String, path: String, irreplaceable: Boolean): Unit = { + if (reported.put(s"$kind:$id", java.lang.Boolean.TRUE) == null) { + val message = s"$kind $id has no file on disk at $path" + if (irreplaceable) logger.error(message) else logger.warn(message) + } + } +} diff --git a/app/service/MediaIntegrity.scala b/app/service/MediaIntegrity.scala new file mode 100644 index 0000000000..c2f3af0e9c --- /dev/null +++ b/app/service/MediaIntegrity.scala @@ -0,0 +1,263 @@ +package service + +// The dashboard reports on exactly the directories the boot check guards, from the same list, so the page and the +// check can never disagree about which directories matter or which of them is currently unsafe (#4925). +import modules.PersistentMediaDirCheck +import modules.PersistentMediaDirCheck.PersistentDir +import play.api.{Configuration, Environment} + +import java.io.File +import scala.util.{Failure, Success, Try} + +/** + * What listing one city's media directory found. + * + * `File.list` answers null for a directory that isn't there and for one this process may not read alike, and those + * mean opposite things to a data-loss monitor: nothing uploaded yet, versus no idea what is in there. Collapsing them + * would let a permissions change announce a whole city's photos as destroyed, so they stay distinct all the way to + * the page. + */ +sealed trait DirListing +object DirListing { + + /** The names the directory holds. */ + case class Listed(names: Seq[String]) extends DirListing + + /** No directory at that path: no upload has landed for this city yet, or one that had landed is gone. */ + case object Absent extends DirListing + + /** The path is there but unreadable, so nothing can be concluded about what it holds. */ + case object Unreadable extends DirListing +} + +/** Where one schema's story media lives on this instance, or why this instance can't say. */ +sealed trait ScanTarget +object ScanTarget { + + /** The city subdirectory holding that schema's media. */ + case class Dir(cityId: String) extends ScanTarget + + /** No directory can be attributed to the schema, phrased for the operator reading the panel. */ + case class Unlocatable(reason: String) extends ScanTarget +} + +/** + * The pure logic behind the Health dashboard's media-storage panel (#4926): what each persistent media directory + * looks like from this instance, and which `story_media` rows have lost their bytes. + * + * Split from [[HealthService]] — which owns the database reads, the cache, and the thread pool — so a spec can pin + * the parts that are easy to get quietly wrong (an absent directory reported as data loss, a blank config value + * treated as the filesystem root) without booting an application or touching a database. + */ +object MediaIntegrity { + + /** Longest list of ids the payload carries per city; the counts are the signal, the ids are just a starting point. */ + private val MaxSampleIds = 20 + + /** `StoryService.storyMediaFile` names every file this way, which is what lets one directory listing replace N stats. */ + private val StoryMediaFileName = """^story_(\d+)\.jpg$""".r + + /** + * How each persistent media directory resolves on this instance, in `persistentDirs` order. + * + * @param config Application configuration. + * @param environment Play environment supplying the application root and the run mode. + * @return One status per directory, carrying its own display label and severity so the page holds no + * copy of the rules. + */ + def directoryStatuses(config: Configuration, environment: Environment): Seq[MediaDirStatus] = { + val enforced = PersistentMediaDirCheck.arms(environment) + val unsafe = PersistentMediaDirCheck.unsafeDirs(config, environment).map(u => u.dir.key -> u.reason).toMap + + PersistentMediaDirCheck.persistentDirs.map { dir => + Try(MediaDirs.baseDir(config, environment, dir.key)) match { + case Failure(e) => + status(dir, "—", "unresolved", "unusable value", "bad", Some(e.getMessage)) + case Success(resolved) => + val probe = DirProbe(resolved.exists(), resolved.canRead(), resolved.canWrite()) + dirStatus(dir, resolved.getAbsolutePath, probe, unsafe.get(dir.key), enforced) + } + } + } + + /** + * What a stat of one media directory saw — the only facts about it the status rules turn on. + * + * @param exists Whether anything is at the path. + * @param readable Whether this process may read it. + * @param writable Whether this process may write to it. + */ + case class DirProbe(exists: Boolean, readable: Boolean, writable: Boolean) + + /** + * What one directory's observed state means, separated from observing it so every branch is reachable from a spec: + * the permission branches can't be provoked from a test that runs as root, which CI and the dev container both do. + * + * @param dir The directory being judged. + * @param path Where it resolved, for display. + * @param probe What a stat of it saw. + * @param unsafeReason The boot check's objection to it, when it has one. + * @param enforced Whether the boot check arms on this instance. When it doesn't, a directory inside the build + * tree is the intended dev arrangement rather than a fault, and saying otherwise would make the + * dev dashboard permanently red and teach everyone to ignore it. + * @return The status, carrying its own display label and severity so the page holds no copy of the + * rules. + */ + private[service] def dirStatus( + dir: PersistentDir, + path: String, + probe: DirProbe, + unsafeReason: Option[String], + enforced: Boolean + ): MediaDirStatus = unsafeReason match { + case Some(reason) => + val severity = if (!enforced) "ok" else if (dir.irreplaceable) "bad" else "warn" + // The dev label leads with the verdict, because a status that only names a location leaves the reader to work + // out whether it is a problem — and here it isn't one. + val label = if (enforced) "a deploy will delete this" else "ok for dev (inside the build tree)" + status(dir, path, "unsafe", label, severity, Some(reason)) + // Not created yet is the normal state until the first upload — the write paths mkdirs on demand. + case None if !probe.exists => status(dir, path, "absent", "not created yet", "ok", None) + case None if !probe.readable => + val detail = s"${dir.envVar} points at a path this process cannot read, so nothing in it can be verified." + status(dir, path, "not_readable", "not readable", "bad", Some(detail)) + case None if !probe.writable => + val detail = s"${dir.envVar} points at a path this process cannot write to, so uploads will fail." + status(dir, path, "not_writable", "not writable", "bad", Some(detail)) + case None => status(dir, path, "ok", "ok", "good", None) + } + + private def status( + dir: PersistentDir, + path: String, + key: String, + label: String, + severity: String, + detail: Option[String] + ): MediaDirStatus = + MediaDirStatus(dir.key, dir.envVar, dir.irreplaceable, path, key, label, severity, detail) + + /** + * Whether the story-media scan can run against this base directory at all, and if not, what to say instead. + * + * Both refusals report the scan as unavailable rather than as loss, and the second is the one that is easy to get + * wrong: `isDirectory` answers true for a directory this process may not read, and every per-city listing beneath + * an unreadable base comes back empty — which would announce every story photo on the stage as destroyed. A monitor + * that cries data loss over a permissions change gets muted, and a muted monitor leaves us where #4925 found us. + * + * @param path Where the base directory resolved, named in the message so an operator knows what to look at. + * @param isDirectory Whether a directory is there. + * @param canRead Whether this process may read it. + * @return The reason the scan declined, or None when it can proceed. + */ + def scanRefusal(path: String, isDirectory: Boolean, canRead: Boolean): Option[String] = { + // Nothing has been uploaded on this stage yet, or the directory is gone. Either way there is nothing to compare + // against, and the directory panel already reports the state of the path itself. + if (!isDirectory) Some(s"No media directory at $path to scan.") + else if (!canRead) Some(s"Media directory $path is not readable by this process.") + else None + } + + /** + * Which city's subdirectory each schema's media lives in, or why it can't be located. + * + * This instance's own schema takes `city-id`, because that is what `StoryService` builds its write path from — + * the schema and `city-id` are independent settings, so on an instance where they disagree the scan has to look + * where the files actually are. That claim is exclusive: no second schema may be pointed at the same directory, + * or it would report the first schema's files as orphans. Every schema that loses a directory that way carries + * the reason with it, since "no city configured" and "this instance took that directory" send an operator looking + * in very different places. + * + * @param schemas Schemas the scan covers. + * @param currentSchema The schema this instance reads and writes. + * @param currentCity The `city-id` this instance writes media under. + * @param configured Schema to city id, from configuration alone. + * @return One target per schema, in no particular order. + */ + def scanTargets( + schemas: Seq[String], + currentSchema: String, + currentCity: String, + configured: Map[String, String] + ): Map[String, ScanTarget] = schemas.map { schema => + schema -> { + if (schema == currentSchema) ScanTarget.Dir(currentCity) + else + configured.get(schema) match { + case Some(`currentCity`) => + ScanTarget.Unlocatable( + s"this instance writes $currentCity media under schema $currentSchema, so the $currentCity directory " + + s"can't also be read as $schema" + ) + case Some(cityId) => ScanTarget.Dir(cityId) + case None => ScanTarget.Unlocatable(s"no city on this stage is configured to use schema $schema") + } + } + }.toMap + + /** + * Compares one city's `story_media` rows against the files in its media directory. + * + * Both directions matter. A row with no file is destroyed content (#4925). A file with no row is the opposite + * failure: a retraction whose row delete landed and whose file delete did not, which leaves a photo on disk that + * its author believes they deleted — the hard-delete contract stories were built on (#4054). + * + * @param cityId City the schema belongs to, which names the directory that was listed. + * @param schema Database schema the rows came from. + * @param mediaIds `story_media` ids in that schema. + * @param listing What the city's directory held. An absent directory is real loss — the write path creates it and + * never removes it — but an unreadable one proves nothing, so it reports unscanned instead. + * @return Counts in both directions, with a short sample of ids to start looking from. + */ + def compareCity(cityId: String, schema: String, mediaIds: Seq[Int], listing: DirListing): CityStoryMedia = { + val rows = mediaIds.distinct.toSet + listing match { + case DirListing.Unreadable => + unscannedCity(Some(cityId), schema, mediaIds, s"the $cityId media directory is not readable by this process") + case DirListing.Absent => diff(cityId, schema, rows, Set.empty) + case DirListing.Listed(names) => + diff( + cityId, + schema, + rows, + names.flatMap { case StoryMediaFileName(id) => id.toIntOption; case _ => None }.toSet + ) + } + } + + /** + * One city's row whose directory could not be scanned, so its counts stand for nothing and read as blank. + * + * @param cityId City the schema belongs to, when one is known. + * @param schema Database schema the rows came from. + * @param mediaIds `story_media` ids in that schema, which are still worth reporting as a row count. + * @param reason Why the scan couldn't run, phrased for the operator reading the panel. + */ + def unscannedCity(cityId: Option[String], schema: String, mediaIds: Seq[Int], reason: String): CityStoryMedia = + CityStoryMedia(cityId, schema, mediaIds.distinct.size, 0, 0, Seq.empty, Seq.empty, scanned = false, Some(reason)) + + private def diff(cityId: String, schema: String, rows: Set[Int], onDisk: Set[Int]): CityStoryMedia = { + val missing = (rows -- onDisk).toSeq.sorted + val orphans = (onDisk -- rows).toSeq.sorted + CityStoryMedia( + Some(cityId), schema, rows.size, missing.size, orphans.size, missing.take(MaxSampleIds), + orphans.take(MaxSampleIds), scanned = true, None + ) + } + + /** + * What a media directory holds, telling an absent directory apart from one this process may not read. + * + * Blocking; call on a blocking pool. + * + * @param dir Directory to list. + * @return Its file names, or which of the two null-answering states `File.list` was in. + */ + def listing(dir: File): DirListing = Option(dir.list()) match { + case Some(names) => DirListing.Listed(names.toSeq) + // `list` also answers null on an I/O error against a directory that is there, which is no more conclusive than a + // permissions refusal — so anything that exists but won't list is Unreadable rather than Absent. + case None if dir.exists() => DirListing.Unreadable + case None => DirListing.Absent + } +} diff --git a/app/service/PanoDataService.scala b/app/service/PanoDataService.scala index 1396ac95c7..7cd0255ace 100644 --- a/app/service/PanoDataService.scala +++ b/app/service/PanoDataService.scala @@ -367,6 +367,7 @@ trait PanoDataService { def cropExists(labelId: Int, labelType: LabelTypeEnum.Base): Boolean def cropUrl(labelId: Int, labelType: LabelTypeEnum.Base): Option[String] def localBackupImageFile(panoId: String): Option[File] + def backupImageDir(panoId: String): File def getLocalBackupImage(panoId: String): Future[Option[PanoData]] } @@ -747,12 +748,15 @@ class PanoDataServiceImpl @Inject() ( * `///.`. Tries jpg/jpeg/png in order. */ def localBackupImageFile(panoId: String): Option[File] = { - val dir = new File(panosBaseDir, panoId.take(2)) + val dir = backupImageDir(panoId) Seq("jpg", "jpeg", "png").iterator .map(ext => new File(dir, s"$panoId.$ext")) .find(_.exists()) } + /** The directory a pano's backup image lives in, whether or not one is there — named so a miss can say where it looked. */ + def backupImageDir(panoId: String): File = new File(panosBaseDir, panoId.take(2)) + /** * Returns the pano_data row for a pano if a self-hosted image exists AND all required fields are populated. * diff --git a/app/service/SingleFlightGate.scala b/app/service/SingleFlightGate.scala new file mode 100644 index 0000000000..29e1cfb9d0 --- /dev/null +++ b/app/service/SingleFlightGate.scala @@ -0,0 +1,47 @@ +package service + +import java.util.concurrent.atomic.AtomicBoolean +import scala.concurrent.{ExecutionContext, Future} +import scala.util.control.NonFatal + +/** + * Lets one piece of work run at a time, answering everyone who arrives while it is running with a stand-in value + * instead of starting a second copy. + * + * A deadline and a gate solve different halves of the same problem, and only the deadline is obvious. `withTimeout` + * protects the *caller* — it stops one slow call from holding a poll open — but it cannot cancel the work it gave up + * waiting on: a filesystem call against an unreachable mount returns when the mount does, or never. So a poller that + * only had a deadline would start a fresh copy every cycle, and each one would park a thread that never comes back; + * the pool is gone within minutes, and the panel that exists to report storage trouble stays dead long after the + * mount recovers. The gate protects the *pool*: parking one thread is the price of asking at all, parking all of them + * is not. + * + * The gate opens when the underlying work completes, never when a caller stops waiting for it — that distinction is + * the whole point, and is what [[service.SingleFlightGateSpec]] pins. + */ +class SingleFlightGate { + + private val inFlight = new AtomicBoolean(false) + + /** + * Runs `work` if nothing is already running, and otherwise answers `busy` without starting anything. + * + * @param busy Value to answer callers who arrive while work is in flight. By-name, so building it costs nothing on + * the common path. + * @param work The work to run. Failing — or throwing before it returns a future at all — reopens the gate, since a + * failure is a completion: only work that is genuinely still running should keep the next caller out. + * @return The work's own future, so a caller can time it out without affecting the gate. + */ + def runOrElse[T](busy: => T)(work: => Future[T]): Future[T] = { + if (!inFlight.compareAndSet(false, true)) Future.successful(busy) + else { + val started = + try work + catch { case NonFatal(e) => Future.failed(e) } + // parasitic: reopening the gate is a flag write, so it runs on whichever thread completed the work rather than + // waiting for a scheduler that a saturated pool may not get to. + started.onComplete(_ => inFlight.set(false))(ExecutionContext.parasitic) + started + } + } +} diff --git a/app/views/admin/dashboard/health.scala.html b/app/views/admin/dashboard/health.scala.html index e5bc66ae17..61b8218553 100644 --- a/app/views/admin/dashboard/health.scala.html +++ b/app/views/admin/dashboard/health.scala.html @@ -23,6 +23,7 @@

Health

+
+

Media storage

+

Media that must outlive a deploy, and whether any of it is gone. A deploy rebuilds the whole build tree, so anything kept inside it dies with the next release while its database rows survive — how #4925 destroyed a story photo. Missing is a row with no file; orphaned is a file with no row.

+
+
+

+
+

Pano downloads

Backup-image coverage for this city's labeled panos. The viewer shows a locally-hosted backup when one exists and otherwise falls back to the live source, so a labeled pano whose source has expired and has no backup can no longer be shown.

diff --git a/conf/application.conf b/conf/application.conf index b6789350f6..bebc550162 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -85,6 +85,19 @@ cpu-intensive { } } +# Execution context for blocking filesystem work — today the admin Health dashboard's media-integrity scan, which +# stats and lists the media directories. A thread blocked on an unreachable network mount never comes back, so this +# work is isolated from every other pool: the default pool serves requests, and cpu-intensive doubles as the stream +# materializer below, so a hung mount there would stall the streaming API. Threads, not fork-join tasks, because the +# work is blocking rather than compute-bound. +blocking-io { + executor = "thread-pool-executor" + throughput = 1 + thread-pool-executor { + fixed-pool-size = 4 + } +} + # Sets the execution context that Materializer uses for running streams. We tend to use this for the CPU-intensive API. pekko.stream.materializer.dispatcher = "cpu-intensive" @@ -259,13 +272,15 @@ custom.news.ribbon.link = null # at boot in prod mode, and every consumer resolves its path through service.MediaDirs so the check can't drift from # the write paths. -# Directory to store cropped images. +# Directory for label crops, organized as ///crop_.png. Each crop is a screenshot +# of the pano canvas taken in the labeler's browser as the label was placed, and nothing here rebuilds one. +# Irreplaceable, like the pano and story dirs. cropped.image.directory = ".crops" cropped.image.directory = ${?SIDEWALK_IMAGES_DIR} -# Directory containing self-hosted pano images, organized as ///.. These back -# up GSV imagery Google has already expired (pano_data.has_backup), so for those panos this holds the only copy -# anywhere — irreplaceable, like the story media below. +# Directory containing self-hosted pano images, organized as ///.. Holds +# imagery the providers no longer serve (pano_data.has_backup), which cannot be re-fetched — irreplaceable, like the +# story media below. pano.images.directory = ".panos" pano.images.directory = ${?SIDEWALK_PANO_DIR} diff --git a/docs/deployment-and-stages.md b/docs/deployment-and-stages.md index aa759f9d23..5d4b256399 100644 --- a/docs/deployment-and-stages.md +++ b/docs/deployment-and-stages.md @@ -292,10 +292,14 @@ outside the build tree** via its environment variable (a variable that is set bu | Config key | Env var | Holds | Missing on a deployed stage | |---|---|---|---| | `story.media.directory` | `SIDEWALK_STORY_MEDIA_DIR` | User-uploaded story photos (**irreplaceable**) | **App refuses to start** | -| `pano.images.directory` | `SIDEWALK_PANO_DIR` | Self-hosted pano store — the only copies of GSV imagery Google has expired (**irreplaceable**) | **App refuses to start** | -| `cropped.image.directory` | `SIDEWALK_IMAGES_DIR` | Label crops (re-derivable from pano imagery) | Error logged at boot | +| `pano.images.directory` | `SIDEWALK_PANO_DIR` | Locally stored pano imagery the app serves itself (**irreplaceable**) | **App refuses to start** | +| `cropped.image.directory` | `SIDEWALK_IMAGES_DIR` | Label crops — browser captures of the pano as it was labeled, with no rebuild path (**irreplaceable**) | **App refuses to start** | | `share.image.directory` | `SIDEWALK_SHARE_IMAGES_DIR` | Cached social-share previews (regenerable) | Error logged at boot | +Crops sit in the fatal tier because nothing in this app regenerates one: `/saveImage` stores a canvas screenshot the +labeler's browser took as the label was placed, and the Street View Static still used as a fallback elsewhere is a +different, smaller image of a pano the provider must still serve — which it often no longer does. + `PersistentMediaDirCheck` enforces this at boot in **prod mode** — what every staged binary runs in — so it covers every deployed stage *and* a staged binary run by hand (export the four variables to `/tmp` paths for that; CI's `e2e-smoke` job does exactly this). It deliberately does not key on `ENV_TYPE`: that variable arrives through the @@ -310,7 +314,9 @@ forgotten variable surfaces on test long before it can reach prod. meaningful while it models the exact resolution the write paths use), add it to `persistentDirs` in `PersistentMediaDirCheck`, decide whether its contents are irreplaceable (fatal) or derived (logged), and have the deployment tooling export its variable. Losing a story photo this way (#4925) took three weeks to notice, so the -check — not a comment in `application.conf` — is what holds the contract. +check — not a comment in `application.conf` — is what holds the contract. Adding it to `persistentDirs` also puts it +on the **Media storage** panel of the Owner-only `/admin/health` page, which shows where each directory resolves on +the running instance and counts any `story_media` row whose file has gone missing (#4926). ### Asset caching diff --git a/public/css/admin-dashboard/admin-dashboard.css b/public/css/admin-dashboard/admin-dashboard.css index 8aec411197..a68cd712a6 100644 --- a/public/css/admin-dashboard/admin-dashboard.css +++ b/public/css/admin-dashboard/admin-dashboard.css @@ -1987,6 +1987,15 @@ img.activity-feed-thumb:hover { box-shadow: 0 2px 6px rgb(0 0 0 / 18%); } } .ac-table tbody tr:hover { background: var(--color-neutral-100); } +/* A filesystem path is the only unbounded string in these tables, and a deploy-tree path is long enough to push the + Status column past the edge — the one column an operator is reading. Let a path break rather than scroll; scoped to + the path cell so the config keys and variable names beside it stay whole. */ +.ac-table td.ac-path code { + /* The shared `code` styling is nowrap, which suppresses wrapping outright — overflow-wrap alone can't act. */ + white-space: normal; + overflow-wrap: anywhere; +} + .ac-num { text-align: right; white-space: nowrap; diff --git a/public/js/admin-dashboard/HealthPage.js b/public/js/admin-dashboard/HealthPage.js index 62376003a4..7347be7bbd 100644 --- a/public/js/admin-dashboard/HealthPage.js +++ b/public/js/admin-dashboard/HealthPage.js @@ -71,6 +71,7 @@ class HealthPage { this.#renderBloat(data.table_bloat || []); this.#renderConnections(data.connections || []); this.#renderPanos(data.pano_backups || null); + this.#renderMediaStorage(data.media_storage || null); this.#renderNightlyJobs(data.nightly_jobs || []); } catch (e) { AdminShell.setHtml('health-pulse', `Could not load health data. ${AdminShell.esc(e.message)}`); @@ -128,6 +129,7 @@ class HealthPage { const evolutions = data.stuck_evolutions || []; const conns = (data.connections || []).reduce((sum, c) => sum + (c.count || 0), 0); const atRisk = data.pano_backups?.at_risk; + const media = data.media_storage || null; const bloated = (data.table_bloat || []).filter((b) => this.#bloatTone(b) !== 'good').length; const longIdle = idle.filter((s) => (s.idle_seconds || 0) >= t.idle_txn_warn_seconds).length; const activeBad = active.filter((q) => (q.query_seconds || 0) >= t.active_query_bad_seconds).length; @@ -140,6 +142,27 @@ class HealthPage { // A missing value ("—") means unknown, not healthy, so tone it neutral ('ok') instead of 'good' (green). const panoTone = AdminShell.nil(atRisk) ? 'ok' : atRisk > 0 ? 'warn' : 'good'; this.#setKpi('kpi-panos', AdminShell.nil(atRisk) ? '—' : HealthPage.#compact(atRisk), panoTone); + const [mediaValue, mediaTone] = HealthPage.#mediaKpi(media); + this.#setKpi('kpi-media', mediaValue, mediaTone); + } + + /** + * The missing-media KPI. The value is only ever a count of missing files or "—" for unknown — this tile is labelled + * "Missing media files", so putting any other number under it would misreport. A directory a deploy will delete is + * bad news before anything has been lost from it, so it colors the tile red without supplying its number, and the + * table below names which directory. Orphans are a lesser fault and only warn. + * + * @param {?Object} media - The media_storage payload, or null when it couldn't be read. + * @returns {[(string|number), string]} Value and tone, spread into #setKpi. + */ + static #mediaKpi(media) { + // Unknown is not healthy: tone it neutral rather than green. + if (!media) return ['—', 'ok']; + const unsafeDirs = (media.directories || []).filter((d) => d.severity === 'bad').length; + if (!media.story_media) return ['—', unsafeDirs > 0 ? 'bad' : 'ok']; + const { missing, orphans } = media.story_media; + if (missing > 0 || unsafeDirs > 0) return [HealthPage.#compact(missing), 'bad']; + return [HealthPage.#compact(missing), orphans > 0 ? 'warn' : 'good']; } /** Renders the "updated Ns ago · db · role" meta line, including whether other sessions' query text is visible. */ @@ -338,6 +361,115 @@ class HealthPage { + 'and these figures approximate what is actually on disk.'); } + // ---- Panel: media storage -------------------------------------------------------------------------------------- + + /** + * Renders the persistent-media directories and the story-media integrity scan. + * + * Every label, severity and threshold here is server-computed: the page must not hold its own copy of the rules + * that decide when a directory is unsafe, or it would drift from the boot check that enforces them. + * + * @param {?Object} media - The media_storage payload, or null when it couldn't be read. + */ + #renderMediaStorage(media) { + if (!media) { + AdminShell.setHtml('health-media-dirs', '

Media storage status is unavailable.

'); + AdminShell.setHtml('health-media-story', ''); + AdminShell.setHtml('health-media-note', ''); + return; + } + + // A scan that couldn't even stat the directories sends none, and an empty table would read as "none configured". + if (!media.directories?.length) { + const why = AdminShell.esc(media.unavailable || 'Media storage status is unavailable.'); + AdminShell.setHtml('health-media-dirs', `

${why}

`); + AdminShell.setHtml('health-media-story', ''); + AdminShell.setHtml('health-media-note', ''); + return; + } + + const dirRows = (media.directories || []).map((d) => { + // Muted text rather than a badge: this is what losing the contents would cost, not something that has + // happened, and an amber badge next to the live Status badge reads as a second alarm on a healthy row. + const recoverable = d.irreplaceable ? 'No — nothing to rebuild it from' : 'Yes — rebuilt on demand'; + // The wipe-zone reason names the key and the path, which are already columns, and then repeats the same + // sentence about deploys on every row — four copies of it in the narrowest column is what made these rows six + // times taller than every other table on the page. The badge says the state; the note below says the fix once. + const detail = d.detail && d.status !== 'unsafe' ? `
${AdminShell.esc(d.detail)}` : ''; + return ` + + ${AdminShell.esc(d.key)} + ${AdminShell.esc(d.env_var)} + ${AdminShell.esc(d.path)} + ${recoverable} + ${AdminShell.esc(d.label)}${detail} + `; + }).join(''); + this.#table('health-media-dirs', ['Config key', 'Env var', 'Resolves to', 'Recoverable?', 'Status'], dirRows); + + const scan = media.story_media; + if (!scan) { + AdminShell.setHtml('health-media-story', + `

${AdminShell.esc(media.unavailable || 'Story media was not scanned.')}

`); + } else if (!scan.cities.length) { + this.#renderEmpty('health-media-story', 'No city has any story media yet.'); + } else { + const rows = scan.cities.map((c) => ` + + ${AdminShell.esc(c.city_id || c.schema)} + ${AdminShell.num(c.rows)} + ${HealthPage.#mediaCount(c, 'missing', 'bad')} + ${HealthPage.#mediaCount(c, 'orphans', 'warn')} + ${HealthPage.#mediaDetail(c)} + `).join(''); + this.#table('health-media-story', + ['City', ['Media rows', true], ['Missing', true], ['Orphaned', true], 'Notes'], rows); + } + + const notes = [`Story media lives under ${AdminShell.esc(scan ? scan.base_dir : '—')}, one + subdirectory per city; a city hosted elsewhere reads as unscanned.`]; + // Said once here rather than repeated in every unsafe row, which is where it used to live. + if (media.directories.some((d) => d.status === 'unsafe')) { + notes.push(`A directory inside the build tree is deleted by the next release: point its environment variable at + storage outside the application (docs/deployment-and-stages.md).`); + } + if (!media.enforced) { + notes.push(`Not production mode, so that boot check is inactive here and the relative defaults landing in the + checkout are expected.`); + } + AdminShell.setHtml('health-media-note', notes.join(' ')); + } + + /** + * A missing/orphan count cell, badged only when non-zero so a clean fleet reads as quiet. + * + * @param {Object} city - One city row from the scan. + * @param {string} field - Which count to render. + * @param {string} tone - Badge tone to use when the count is non-zero. + * @returns {string} Cell HTML. + */ + static #mediaCount(city, field, tone) { + if (!city.scanned) return '—'; + const n = city[field] || 0; + return n > 0 ? `${AdminShell.num(n)}` : AdminShell.num(n); + } + + /** + * The ids worth looking at for one city, or why it wasn't scanned. + * + * @param {Object} city - One city row from the scan. + * @returns {string} Cell HTML. + */ + static #mediaDetail(city) { + // The reason is server-side: "no city configured", "this instance writes that directory under another schema" + // and "not readable" send an operator to very different places, and only the backend knows which applies. + if (!city.scanned) return `${AdminShell.esc(city.unscanned_reason || 'not scanned')}`; + const parts = []; + if (city.missing_ids?.length) parts.push(`missing ${city.missing_ids.join(', ')}`); + if (city.orphan_ids?.length) parts.push(`orphaned ${city.orphan_ids.join(', ')}`); + return parts.length ? `${AdminShell.esc(parts.join(' · '))}` : '—'; + } + // ---- Panel: nightly jobs --------------------------------------------------------------------------------------- /** diff --git a/test/controllers/ImageControllerSpec.scala b/test/controllers/ImageControllerSpec.scala index 37051d2cd4..e9e2e2d9df 100644 --- a/test/controllers/ImageControllerSpec.scala +++ b/test/controllers/ImageControllerSpec.scala @@ -10,13 +10,20 @@ import play.api.mvc.Cookie import play.api.test.CSRFTokenHelper._ import play.api.test.FakeRequest import play.api.test.Helpers._ -import service.{PanoDataService, ShareImageCache} +import models.label.LabelTypeEnum +import service.{ImageSigningService, PanoDataService, ShareImageCache} import util.AnonSession +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.classic.{Level, Logger => LogbackLogger} +import ch.qos.logback.core.read.ListAppender +import org.slf4j.LoggerFactory + import java.awt.image.BufferedImage import java.io.{ByteArrayOutputStream, File} import java.util.Base64 import javax.imageio.ImageIO +import scala.jdk.CollectionConverters._ /** * Functional tests for `POST /saveImage` (#4415, #4726). Boots the real app and drives the endpoint over HTTP through @@ -42,8 +49,9 @@ class ImageControllerSpec extends PlaySpec with AnonSession with GuiceOneAppPerS implicit lazy val mat: Materializer = app.materializer - private val panoDataService: PanoDataService = app.injector.instanceOf[PanoDataService] - private val shareImageCache: ShareImageCache = app.injector.instanceOf[ShareImageCache] + private val panoDataService: PanoDataService = app.injector.instanceOf[PanoDataService] + private val shareImageCache: ShareImageCache = app.injector.instanceOf[ShareImageCache] + private val signingService: ImageSigningService = app.injector.instanceOf[ImageSigningService] // Far outside the range of any real label id, so writing a crop here can't clobber one. private val syntheticLabelId = Int.MaxValue - 4726 @@ -143,4 +151,96 @@ class ImageControllerSpec extends PlaySpec with AnonSession with GuiceOneAppPerS status(resp) mustBe BAD_REQUEST } } + + /** + * Runs `f` with an appender on `LostMediaLog`'s logger and returns every line it wrote. + * + * The level is forced rather than inherited so these cases pin the controller's own severity choice instead of + * whatever the ambient logback config permits, and the console is detached for the duration so a suite that + * deliberately provokes data-loss alarms doesn't print any. + */ + private def capturedLoss(f: => Any): Seq[ILoggingEvent] = { + val logger = LoggerFactory.getLogger(classOf[service.LostMediaLog]).asInstanceOf[LogbackLogger] + val appender = new ListAppender[ILoggingEvent] + val original = logger.getLevel + appender.start() + logger.addAppender(appender) + logger.setLevel(Level.WARN) + logger.setAdditive(false) + try { val _ = f } + finally { + logger.setAdditive(true) + logger.setLevel(original) + logger.detachAppender(appender) + appender.stop() + } + appender.list.asScala.toSeq + } + + // A signed serving URL is only ever minted for a file that was on disk at the time (PanoDataService.cropUrl and + // backupImageUrl both check first), so a miss on one of these means the bytes vanished inside the signature's + // ~75-minute life. That is the loss #4925 had no way to notice: the response has to stay an ordinary 404, since + // telling a prober which ids exist would be worse, which leaves the log as the only place it can be said (#4926). + "Serving media whose bytes are gone" should { + "answer a signed crop URL whose file has been deleted with a plain 404, and say the crop is gone" in { + val session = freshAnonSession() + status(postCrop(session, syntheticLabelId)) mustBe OK + val url = panoDataService.cropUrl(syntheticLabelId, LabelTypeEnum.CurbRamp).value + val file = cropFileFor(syntheticLabelId) + file.delete() mustBe true + + try { + val events = capturedLoss { + val resp = route(app, FakeRequest(GET, url).withCookies(session: _*)).get + status(resp) mustBe NOT_FOUND + } + + events must have size 1 + // A crop exists in one place only — it was captured in the labeler's browser as the label was placed — so + // its disappearance is the error tier, not a rebuild cost. + events.head.getLevel mustBe Level.ERROR + val message = events.head.getFormattedMessage + message must include("crop") + message must include(syntheticLabelId.toString) + // Whoever reads this line goes looking on disk, so it has to name the exact path that was checked. + message must include(file.getAbsolutePath) + } finally cleanUp(syntheticLabelId) + } + + "answer a signed pano URL with no backup image with a plain 404, every time it is asked" in { + // Nothing can re-fetch a pano the provider no longer serves, so the loss is the error tier. The log + // deduplicates the repeat (LostMediaLogSpec pins that); the responses must not. + val panoId = "sidewalkSpecNoSuchPano4926" + val url = signingService.signedUrl(s"/backupImage/$panoId") + + val events = capturedLoss { + status(route(app, FakeRequest(GET, url)).get) mustBe NOT_FOUND + status(route(app, FakeRequest(GET, url)).get) mustBe NOT_FOUND + } + + events must have size 1 + events.head.getLevel mustBe Level.ERROR + val message = events.head.getFormattedMessage + message must include("pano") + message must include(panoId) + // The store sharded by the pano id's first two characters, which is where someone restoring a backup must look. + message must include(panoDataService.backupImageDir(panoId).getAbsolutePath) + } + + "stay silent when the crop is where it should be, so the log holds losses and nothing else" in { + // A different label id from the case above: reporting is once per item, so reusing one would let dedup supply + // the silence this is trying to check for. And a false alarm is worse than noise here — it burns the single + // line that item will ever get. + val session = freshAnonSession() + try { + status(postCrop(session, otherSyntheticLabelId)) mustBe OK + val url = panoDataService.cropUrl(otherSyntheticLabelId, LabelTypeEnum.CurbRamp).value + + val events = capturedLoss { + status(route(app, FakeRequest(GET, url).withCookies(session: _*)).get) mustBe OK + } + events mustBe empty + } finally cleanUp(otherSyntheticLabelId) + } + } } diff --git a/test/controllers/StoryUploadWindowSpec.scala b/test/controllers/StoryUploadWindowSpec.scala new file mode 100644 index 0000000000..6f68452a63 --- /dev/null +++ b/test/controllers/StoryUploadWindowSpec.scala @@ -0,0 +1,49 @@ +package controllers + +import org.scalatestplus.play.PlaySpec + +import java.time.OffsetDateTime +import java.time.temporal.ChronoUnit + +/** + * The grace window that keeps the story-media loss tripwire from crying wolf at its own upload path (#4925, #4926). + * + * `StoryService` commits the media row before the file move lands, so for a moment a perfectly healthy upload looks + * exactly like a destroyed one. Reporting it would cost more than noise: the loss log reports once per media id, so a + * false alarm spends the single line that id will ever get, and the real loss — if it ever comes — passes in silence. + * A window too generous is the opposite failure, a stretch of time in which real loss goes unannounced. + * + * No app or database boot: the interesting cases are the two boundaries, which a live clock can't pin down. + */ +class StoryUploadWindowSpec extends PlaySpec { + + private val committed = OffsetDateTime.parse("2026-08-20T10:00:00Z") + + "StoryController.withinUploadWindow" should { + "treat a row committed moments ago as an upload still landing" in { + StoryController.withinUploadWindow(committed, committed.plus(200, ChronoUnit.MILLIS)) mustBe true + } + + "still cover a row a few seconds old, since the window is slack over a sub-second race" in { + StoryController.withinUploadWindow(committed, committed.plusSeconds(30)) mustBe true + } + + "call a row past the window loss, which is the whole point of having one" in { + StoryController.withinUploadWindow(committed, committed.plusSeconds(61)) mustBe false + } + + "close the window on the minute rather than leaving it ambiguous" in { + StoryController.withinUploadWindow(committed, committed.plusMinutes(1)) mustBe false + } + + "call a row committed long ago loss, however far back it goes" in { + StoryController.withinUploadWindow(committed, committed.plusDays(30)) mustBe false + } + + "treat a row timestamped ahead of the clock as still landing, since skew is not evidence of loss" in { + // Self-correcting rather than permanent: once the clock passes the row's timestamp by a minute, it reports. + StoryController.withinUploadWindow(committed.plusMinutes(5), committed) mustBe true + StoryController.withinUploadWindow(committed.plusMinutes(5), committed.plusMinutes(7)) mustBe false + } + } +} diff --git a/test/js/healthMediaPanel.test.js b/test/js/healthMediaPanel.test.js new file mode 100644 index 0000000000..f67b0f9c3d --- /dev/null +++ b/test/js/healthMediaPanel.test.js @@ -0,0 +1,311 @@ +/** + * Tests for the Health dashboard's Media storage panel (public/js/admin-dashboard/HealthPage.js, issue #4926). + * + * The panel is the human-readable half of the tripwire #4925 went without: a story photo was destroyed by a deploy + * and answered 404 for six days with nothing anywhere saying so. Its failure mode is quiet in both directions — + * showing zeros for a city it could not actually scan reads as "all present", and painting a dev checkout red for + * the relative defaults it is supposed to use teaches everyone to ignore the panel entirely. + * + * So every judgment call here belongs to the server (MediaIntegrity computes each row's label and severity, and + * HealthMediaPayloadSpec pins the field names below). What is tested here is that the page renders what it was told + * and invents nothing: an unscanned city must not read as a clean one, and the KPI must never call an unknown + * healthy. + * + * HealthPage and AdminShell are plain top-level declarations in a concatenated bundle (no window assignment), so + * both are eval'd with an explicit window epilogue, the way the other class suites do it. + */ + +const fs = require('fs'); +const path = require('path'); + +const JS_DIR = path.resolve(__dirname, '..', '..', 'public/js/admin-dashboard'); +const SHELL_SRC = fs.readFileSync(path.join(JS_DIR, 'AdminShell.js'), 'utf8'); +const PAGE_SRC = fs.readFileSync(path.join(JS_DIR, 'HealthPage.js'), 'utf8'); + +/** The panel's containers, plus the KPI tile it fills. Absent ids are no-ops, so only what's asserted is needed. */ +const MARKUP = ` +
+ +
+
+

+`; + +/** Every threshold the untested panels read, so a render can't throw before reaching the media panel. */ +const THRESHOLDS = { + idle_txn_warn_seconds: 60, idle_txn_bad_seconds: 300, lock_wait_warn_seconds: 5, lock_wait_bad_seconds: 30, + active_query_warn_seconds: 30, active_query_bad_seconds: 120, bloat_warn_ratio: 0.2, bloat_bad_ratio: 0.4, + bloat_min_dead_tuples: 1000, vacuum_age_warn_seconds: 86400, conn_pool_max: 25, conn_warn_active: 15, + conn_bad_active: 20 +}; + +const OK_DIR = { + key: 'story.media.directory', env_var: 'SIDEWALK_STORY_MEDIA_DIR', irreplaceable: true, + path: '/srv/sidewalk/story-media', status: 'ok', label: 'ok', severity: 'good' +}; + +const UNSAFE_DIR = { + key: 'pano.images.directory', env_var: 'SIDEWALK_PANO_DIR', irreplaceable: true, + path: '/app/target/universal/stage/.pano-images', status: 'unsafe', label: 'a deploy will delete this', + severity: 'bad', detail: 'pano.images.directory resolves inside the build output tree that a deploy deletes.' +}; + +/** One city whose rows and files all line up. */ +const cleanCity = (overrides = {}) => ({ + city_id: 'chicago-il', schema: 'sidewalk_chicago', rows: 3, missing: 0, orphans: 0, + missing_ids: [], orphan_ids: [], scanned: true, ...overrides +}); + +/** A payload with the media panel populated and every other panel empty. */ +const payloadWith = (mediaStorage) => ({ + generated_at: '2026-08-20T00:00:00Z', current_database: 'sidewalk', current_role: 'sidewalk', + can_see_all_queries: true, blocking_sessions: [], idle_in_transaction: [], active_queries: [], + stuck_evolutions: [], table_bloat: [], connections: [], pano_backups: null, + media_storage: mediaStorage, thresholds: THRESHOLDS +}); + +describe('HealthPage media storage panel', () => { + let HealthPage; + + beforeEach(() => { + // init() installs poll intervals; fake timers keep them from firing into a torn-down DOM. + jest.useFakeTimers(); + document.body.innerHTML = MARKUP; + window.eval(`${SHELL_SRC}\nwindow.AdminShell = AdminShell;\n${PAGE_SRC}\nwindow.HealthPage = HealthPage;`); + HealthPage = window.HealthPage; + }); + + afterEach(() => { + jest.useRealTimers(); + delete global.fetch; + }); + + /** Renders one payload through the real load path and hands back the panel's containers. */ + async function render(mediaStorage) { + global.fetch = jest.fn().mockResolvedValue({ ok: true, json: async () => payloadWith(mediaStorage) }); + await new HealthPage({ healthUrl: '/adminapi/dbHealth' }).init(); + // A render that threw is caught and reported in the pulse line, which would otherwise leave the media + // assertions failing against empty containers with no hint of why. + expect(document.getElementById('health-pulse').innerHTML).not.toContain('Could not load'); + return { + dirs: document.getElementById('health-media-dirs').innerHTML, + story: document.getElementById('health-media-story').innerHTML, + note: document.getElementById('health-media-note').innerHTML, + kpi: document.getElementById('kpi-media') + }; + } + + describe('the directory table', () => { + it('names each directory, the variable that fixes it, and where it resolved', async () => { + const { dirs } = await render({ directories: [OK_DIR], enforced: true, story_media: null }); + + expect(dirs).toContain('story.media.directory'); + // A path with no variable name beside it doesn't tell an operator what to change. + expect(dirs).toContain('SIDEWALK_STORY_MEDIA_DIR'); + expect(dirs).toContain('/srv/sidewalk/story-media'); + }); + + it('states recoverability as flat text, since it is a consequence and not a live alarm', async () => { + // An amber badge here sat beside the Status badge on a perfectly healthy row and read as a second alarm, + // which is what made the column unreadable. + const { dirs } = await render({ directories: [OK_DIR], enforced: true, story_media: null }); + + expect(dirs).toContain('No — nothing to rebuild it from'); + expect(dirs).not.toContain('ac-badge--warn'); + }); + + it('says of a rebuildable directory that losing it costs a rebuild, not content', async () => { + const shareImages = { ...OK_DIR, key: 'share.image.directory', irreplaceable: false }; + const { dirs } = await render({ directories: [shareImages], enforced: true, story_media: null }); + + expect(dirs).toContain('Yes — rebuilt on demand'); + }); + + it('leads a dev checkout\'s status with the verdict, not with the location', async () => { + // "inside the build tree" alone left the reader to work out whether that was a problem. + const devUnsafe = { ...UNSAFE_DIR, label: 'ok for dev (inside the build tree)', severity: 'ok' }; + const { dirs } = await render({ directories: [devUnsafe], enforced: false, story_media: null }); + + expect(dirs).toContain('ok for dev'); + }); + + it('shows the server\'s own label and severity rather than deciding either here', async () => { + const { dirs } = await render({ directories: [UNSAFE_DIR], enforced: true, story_media: null }); + + expect(dirs).toContain('ac-badge--bad'); + expect(dirs).toContain('a deploy will delete this'); + }); + + it('says how to fix an unsafe directory once, below the table', async () => { + // The wipe-zone reason repeats the same sentence about deploys on every row, and four copies of it in the + // narrowest column is what made these rows tower over every other table on the page. + const { dirs, note } = await render({ + directories: [UNSAFE_DIR, { ...OK_DIR, status: 'unsafe', severity: 'bad', detail: UNSAFE_DIR.detail }], + enforced: true, story_media: null + }); + + expect(dirs).not.toContain('resolves inside the build output tree'); + expect(note).toContain('deleted by the next release'); + }); + + it('keeps a non-wipe-zone explanation, which the table is the only place to read', async () => { + const notReadable = { + ...OK_DIR, status: 'not_readable', label: 'not readable', severity: 'bad', + detail: 'SIDEWALK_STORY_MEDIA_DIR points at a path this process cannot read.' + }; + const { dirs } = await render({ directories: [notReadable], enforced: true, story_media: null }); + + expect(dirs).toContain('cannot read'); + }); + + it('explains that a dev checkout is meant to look like this', async () => { + // Without this the dev dashboard is permanently red for doing exactly what it is configured to do, and a + // panel that is always red is a panel nobody reads on the day it matters. + const { note } = await render({ directories: [UNSAFE_DIR], enforced: false, story_media: null }); + + expect(note).toContain('Not production mode'); + }); + + it('says why there is nothing to show when the scan could not even stat the directories', async () => { + const { dirs, story } = await render({ + directories: [], enforced: true, story_media: null, + unavailable: 'A previous media scan has not returned; storage may be offline.' + }); + + expect(dirs).toContain('storage may be offline'); + // An empty table here would read as "no media directories are configured", which is a different problem. + expect(dirs).not.toContain(' { + const scanOf = (cities, missing, orphans) => ({ + directories: [OK_DIR], enforced: true, + story_media: { base_dir: '/srv/sidewalk/story-media', cities, missing, orphans } + }); + + it('badges a city that has lost files, and lists ids to start looking from', async () => { + const lost = cleanCity({ missing: 1, missing_ids: [331] }); + const { story } = await render(scanOf([lost], 1, 0)); + + expect(story).toContain('chicago-il'); + expect(story).toContain('ac-badge--bad'); + expect(story).toContain('missing 331'); + }); + + it('badges orphaned files at the lesser tone, since they are a half-finished retraction', async () => { + const orphaned = cleanCity({ orphans: 2, orphan_ids: [7, 8] }); + const { story } = await render(scanOf([orphaned], 0, 2)); + + expect(story).toContain('ac-badge--warn'); + expect(story).toContain('orphaned 7, 8'); + }); + + it('leaves a clean city unbadged, so a healthy fleet reads as quiet', async () => { + const { story } = await render(scanOf([cleanCity()], 0, 0)); + + expect(story).toContain('chicago-il'); + expect(story).not.toContain('ac-badge--bad'); + expect(story).not.toContain('ac-badge--warn'); + }); + + it('shows an unscanned city as unknown rather than as zero losses', async () => { + // Rendering a 0 here is the panel's worst possible lie: it says "every photo is present" about a city it + // never managed to look at. + const unscanned = cleanCity({ + city_id: null, scanned: false, unscanned_reason: 'no city on this stage is configured to use schema X' + }); + const { story } = await render(scanOf([unscanned], 0, 0)); + + expect(story).toContain('sidewalk_chicago'); + expect(story).toContain('no city on this stage is configured'); + expect(story).not.toContain('ac-badge--bad'); + expect(story).toContain('—'); + }); + + it('says plainly when no city has any story media, rather than showing an empty table', async () => { + const { story } = await render(scanOf([], 0, 0)); + + expect(story).toContain('No city has any story media yet.'); + }); + + it('names the directory the per-city subdirectories live under', async () => { + const { note } = await render(scanOf([cleanCity()], 0, 0)); + + expect(note).toContain('/srv/sidewalk/story-media'); + }); + + it('escapes a server-supplied reason instead of writing it into the page as markup', async () => { + const hostile = cleanCity({ scanned: false, unscanned_reason: '' }); + const { story } = await render(scanOf([hostile], 0, 0)); + + expect(story).not.toContain(' { + const kpiFor = async (mediaStorage) => { + const { kpi } = await render(mediaStorage); + return { text: kpi.textContent, tone: kpi.className }; + }; + + it('shows the missing count and reds the tile when anything is gone', async () => { + const { text, tone } = await kpiFor({ + directories: [OK_DIR], enforced: true, + story_media: { base_dir: '/srv/media', cities: [cleanCity({ missing: 2 })], missing: 2, orphans: 0 } + }); + + expect(text).toBe('2'); + expect(tone).toContain('health-kpi--bad'); + }); + + it('reds the tile for a directory a deploy will delete, before anything has been lost from it', async () => { + const { text, tone } = await kpiFor({ + directories: [UNSAFE_DIR], enforced: true, + story_media: { base_dir: '/srv/media', cities: [cleanCity()], missing: 0, orphans: 0 } + }); + + // The tile is labelled "Missing media files", so it can only ever show a count of those; the directory + // table below is what names the doomed directory. + expect(text).toBe('0'); + expect(tone).toContain('health-kpi--bad'); + }); + + it('warns, without reddening, when the only fault is files nobody has a row for', async () => { + const { text, tone } = await kpiFor({ + directories: [OK_DIR], enforced: true, + story_media: { base_dir: '/srv/media', cities: [cleanCity({ orphans: 1 })], missing: 0, orphans: 1 } + }); + + expect(text).toBe('0'); + expect(tone).toContain('health-kpi--warn'); + }); + + it('goes green only when the scan ran and found nothing wrong', async () => { + const { tone } = await kpiFor({ + directories: [OK_DIR], enforced: true, + story_media: { base_dir: '/srv/media', cities: [cleanCity()], missing: 0, orphans: 0 } + }); + + expect(tone).toContain('health-kpi--good'); + }); + + it('shows unknown, never a zero, when the scan could not run', async () => { + // A count of unsafe directories under a tile labelled "Missing media files" would misreport, and a 0 + // would claim a clean result the scan never produced. + const { text, tone } = await kpiFor({ directories: [OK_DIR], enforced: true, story_media: null }); + + expect(text).toBe('—'); + expect(tone).toContain('health-kpi--ok'); + }); + + it('tones unknown neutral rather than green when the whole payload is missing', async () => { + const { text, tone } = await kpiFor(null); + + expect(text).toBe('—'); + expect(tone).toContain('health-kpi--ok'); + }); + }); +}); diff --git a/test/modules/PersistentMediaDirCheckSpec.scala b/test/modules/PersistentMediaDirCheckSpec.scala index 466b28fef6..605d4a3bc2 100644 --- a/test/modules/PersistentMediaDirCheckSpec.scala +++ b/test/modules/PersistentMediaDirCheckSpec.scala @@ -9,8 +9,8 @@ import scala.io.Source import scala.util.Using /** - * The deployment contract behind the media directories: anything irreplaceable — user uploads, our only copies of - * provider-expired panos — has to land outside the build output tree, because a deploy deletes that whole tree + * The deployment contract behind the media directories: anything irreplaceable — user uploads, imagery that cannot + * be re-fetched — has to land outside the build output tree, because a deploy deletes that whole tree * (`sbt clean`) and rebuilds it (#4925). * * Nothing else can catch a violation. The configuration that lost a story photo was correct in dev, correct in CI, @@ -74,11 +74,12 @@ class PersistentMediaDirCheckSpec extends PlaySpec { } "the fatal set" should { - // Refusing to boot is only justified for bytes no rebuild can recreate: the story photos users gave us, and the - // self-hosted pano store — it backs up GSV imagery Google has already expired, so for those panos it is the only - // copy anywhere. Crops and share previews re-derive from them, so they must stay warn-only. + // Refusing to boot is only justified for bytes no rebuild can recreate: the story photos users gave us, imagery + // the providers no longer serve, and the label crops, each captured once in a labeler's browser. Cached share + // previews rebuild on demand, so that one must stay warn-only. "be exactly the irreplaceable directories" in { - persistentDirs.filter(_.irreplaceable).map(_.key) mustBe Seq("pano.images.directory", "story.media.directory") + persistentDirs.filter(_.irreplaceable).map(_.key) mustBe + Seq("cropped.image.directory", "pano.images.directory", "story.media.directory") } // The failure message tells the operator which variable to set. If this mapping drifts from application.conf, diff --git a/test/service/HealthMediaPayloadSpec.scala b/test/service/HealthMediaPayloadSpec.scala new file mode 100644 index 0000000000..5111701acc --- /dev/null +++ b/test/service/HealthMediaPayloadSpec.scala @@ -0,0 +1,96 @@ +package service + +import org.scalatestplus.play.PlaySpec +import play.api.libs.json.{JsObject, Json} + +/** + * The wire contract between the media-storage panel's payload and the page that renders it (#4926). + * + * `HealthPage.js` reads these fields by name, and nothing else connects the two: rename a case-class field and the + * writer quietly emits a different key, every value the panel reads goes undefined, and the page renders a table of + * blanks — a storage monitor that reports nothing, which is indistinguishable from a storage monitor reporting that + * nothing is wrong. Each key set below is exactly what `#renderMediaStorage` and `#mediaKpi` consume. + * + * Pure serialization — no app boot, no database. + */ +class HealthMediaPayloadSpec extends PlaySpec { + + import HealthService._ + + private val dir = MediaDirStatus( + key = "story.media.directory", envVar = "SIDEWALK_STORY_MEDIA_DIR", irreplaceable = true, + path = "/srv/sidewalk/story-media", status = "unsafe", label = "a deploy will delete this", severity = "bad", + detail = Some("resolves inside the build output tree") + ) + + private val city = CityStoryMedia( + cityId = Some("chicago-il"), + schema = "sidewalk_chicago", + rows = 3, + missing = 1, + orphans = 2, + missingIds = Seq(1), + orphanIds = Seq(7, 8), + scanned = true, + unscannedReason = Some("not scanned") + ) + + private val health = MediaStorageHealth( + directories = Seq(dir), + enforced = true, + storyMedia = Some(StoryMediaIntegrity("/srv/sidewalk/story-media", Seq(city), 1, 2)), + unavailable = Some("storage may be offline") + ) + + private def keysOf(json: JsObject): Set[String] = json.keys.toSet + + "The media storage payload" should { + "carry every field the panel's directory table reads, under the names it reads them by" in { + keysOf(Json.toJson(dir).as[JsObject]) mustBe + Set("key", "env_var", "irreplaceable", "path", "status", "label", "severity", "detail") + } + + "carry every field the panel's per-city table reads" in { + keysOf(Json.toJson(city).as[JsObject]) mustBe + Set("city_id", "schema", "rows", "missing", "orphans", "missing_ids", "orphan_ids", "scanned", + "unscanned_reason") + } + + "carry the scan's own totals and base directory" in { + keysOf(Json.toJson(StoryMediaIntegrity("/srv/media", Seq(city), 1, 2)).as[JsObject]) mustBe + Set("base_dir", "cities", "missing", "orphans") + } + + "carry the panel's own four fields" in { + keysOf(Json.toJson(health).as[JsObject]) mustBe Set("directories", "enforced", "story_media", "unavailable") + } + + "hang the whole panel off media_storage, which is the key the page looks for" in { + val payload = DbHealthData( + generatedAt = "2026-08-20T00:00:00Z", + currentDatabase = "sidewalk", + currentRole = "sidewalk", + canSeeAllQueries = true, + blockingSessions = Seq.empty, + idleInTransaction = Seq.empty, + activeQueries = Seq.empty, + stuckEvolutions = Seq.empty, + tableBloat = Seq.empty, + connections = Seq.empty, + panoBackups = None, + mediaStorage = Some(health), + nightlyJobs = Seq.empty, + thresholds = HealthThresholds(1, 2, 3, 4, 5, 6, 0.2, 0.4, 1000, 60, 40, 20, 30, 24, 7) + ) + (Json.toJson(payload) \ "media_storage" \ "story_media" \ "missing").as[Int] mustBe 1 + } + + "omit the optional fields rather than send nulls, which is what the page's fallbacks expect" in { + // `unscanned_reason` absent is how a scanned city says it has nothing to explain, and `detail` absent is how a + // healthy directory does; the page falls back on absence, so emitting an explicit null would be a change. + val clean = Json.toJson(city.copy(unscannedReason = None)).as[JsObject] + clean.keys must not contain "unscanned_reason" + Json.toJson(dir.copy(detail = None)).as[JsObject].keys must not contain "detail" + } + } +} diff --git a/test/service/HealthServiceSpec.scala b/test/service/HealthServiceSpec.scala index 4e3fa54bc9..3c53a173d1 100644 --- a/test/service/HealthServiceSpec.scala +++ b/test/service/HealthServiceSpec.scala @@ -1,6 +1,7 @@ package service import models.utils.{HealthTable, MyPostgresProfile} +import org.scalatest.BeforeAndAfterAll import org.scalatestplus.play.PlaySpec import org.scalatestplus.play.guice.GuiceOneAppPerSuite import play.api.Application @@ -8,6 +9,8 @@ import play.api.db.slick.DatabaseConfigProvider import play.api.inject.guice.GuiceApplicationBuilder import slick.dbio.DBIO +import java.io.File +import java.nio.file.Files import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.duration._ import scala.concurrent.{Await, Future} @@ -29,13 +32,25 @@ import scala.concurrent.{Await, Future} * Read-only. Requires a Postgres+PostGIS database (DATABASE_URL / DATABASE_USER / DATABASE_PASSWORD, as in dev/CI). * Scheduling actors are disabled so background actors can't contend for the pool during the run. */ -class HealthServiceSpec extends PlaySpec with GuiceOneAppPerSuite { +class HealthServiceSpec extends PlaySpec with GuiceOneAppPerSuite with BeforeAndAfterAll { + + // The scan compares database rows against files on disk, so a checkout that has never had an upload has no + // directory to read and the whole panel degrades to "unavailable" — under which every assertion below would pass + // without testing anything. Pointing the app at a directory this spec owns makes the interesting path the only one. + private val mediaDir: File = Files.createTempDirectory("health-service-spec-media").toFile + + /** The subdirectory `StoryService` would write this instance's uploads into: named for `city-id`, not the schema. */ + private lazy val cityDir: File = new File(mediaDir, config.get[String]("city-id")) override def fakeApplication(): Application = - new GuiceApplicationBuilder().disable[modules.ActorModule].build() + new GuiceApplicationBuilder() + .disable[modules.ActorModule] // No eager background actors during tests. + .configure("story.media.directory" -> mediaDir.getAbsolutePath) + .build() private val healthService = app.injector.instanceOf[HealthService] private val healthTable = app.injector.instanceOf[HealthTable] + private val config = app.injector.instanceOf[play.api.Configuration] // Keep the DatabaseConfig as a stable val and call .db.run inline; binding .db to its own val would infer a // path-dependent existential type that needs -language:existentials. private val dbConfig = app.injector.instanceOf[DatabaseConfigProvider].get[MyPostgresProfile] @@ -43,6 +58,26 @@ class HealthServiceSpec extends PlaySpec with GuiceOneAppPerSuite { private def await[T](f: Future[T], d: Duration = 60.seconds): T = Await.result(f, d) private def run[T](action: DBIO[T]): T = Await.result(dbConfig.db.run(action), 60.seconds) + // A file with no `story_media` row behind it. Whether the scan attributes it to this instance's own schema is the + // whole question: it can only do that by naming the directory the way the write path does. + private val orphanId: Int = Int.MaxValue - 4926 + private lazy val orphanFile: File = new File(cityDir, s"story_$orphanId.jpg") + + // Seeded before any test, because the scan is cached: a first call made against an empty directory would answer + // every later assertion from that cached all-clear. + override def beforeAll(): Unit = { + super.beforeAll() + val _ = cityDir.mkdirs() + val _ = orphanFile.createNewFile() + } + + override def afterAll(): Unit = { + val _ = orphanFile.delete() + val _ = cityDir.delete() + val _ = mediaDir.delete() + super.afterAll() + } + "HealthTable catalog queries" should { // Each of these asserts the SQL executes and maps into its DTO against a real PostGIS DB; the result may legitimately // be empty (a healthy DB has no blocking locks), so the value is that `run` completes without throwing. @@ -76,6 +111,19 @@ class HealthServiceSpec extends PlaySpec with GuiceOneAppPerSuite { // The single UNION-ALL query must execute across all discovered schemas at once (the fan-out safety property). run(healthTable.getStuckEvolutionsForSchemas(schemas)).size must be >= 0 } + "count and list story_media across every schema in one union query each" in { + // Every city schema gets the table from evolution 339, and the app under test applies evolutions on boot, so an + // empty result means discovery is broken rather than that this database is unusual. + val schemas = run(healthTable.getStoryMediaSchemas).filter(_.matches("^[A-Za-z0-9_]+$")) + schemas must not be empty + val counts = run(healthTable.getStoryMediaCounts(schemas)) + counts.map(_.schema) must contain theSameElementsAs schemas + // The counts decide which schemas the id read covers, so a disagreement between them would make the scan + // report rows it never fetched as missing. What each id/file pairing then *means* is pinned by + // MediaIntegritySpec — seeding a story here would need a user, a label and an audit task, none of which the CI + // seed has. + run(healthTable.getStoryMediaIds(schemas)) must have size counts.map(_.rows).sum.toLong + } } "HealthService.getDbHealth" should { @@ -92,6 +140,73 @@ class HealthServiceSpec extends PlaySpec with GuiceOneAppPerSuite { t.bloatBadRatio must be >= t.bloatWarnRatio } + "report on every media directory the boot check guards" in { + val media = await(healthService.getDbHealth).mediaStorage.value + media.directories.map(_.key) mustBe modules.PersistentMediaDirCheck.persistentDirs.map(_.key) + // Each row has to name the variable that fixes it; a path alone doesn't tell an operator what to change. + media.directories.foreach(_.envVar must not be empty) + } + + "keep the story-media scan's totals consistent with its per-city rows" in { + val media = await(healthService.getDbHealth).mediaStorage.value + // The scan reports itself unavailable when there is no media directory to read, which is the normal state of a + // dev checkout that has never had an upload. Either way it must never report a total its rows don't support. + media.storyMedia match { + case None => media.unavailable mustBe defined + case Some(scan) => + scan.missing mustBe scan.cities.map(_.missing).sum + scan.orphans mustBe scan.cities.map(_.orphans).sum + // An unscanned city's counts stand for nothing, and the row has to say which of the several reasons put it + // there or the operator reading it has nowhere to start. + scan.cities.filterNot(_.scanned).foreach { city => + city.missing mustBe 0 + city.unscannedReason mustBe defined + } + } + } + + "scan the story-media directory it was configured with" in { + val scan = await(healthService.getDbHealth).mediaStorage.value.storyMedia.value + scan.baseDir mustBe mediaDir.getAbsolutePath + } + + "find this instance's own files under the directory the write path builds, not the one the schema implies" in { + // `city-id` and the connection's schema are independent settings, and on an instance where they disagree the + // scan has to look where StoryService writes rather than where the schema mapping says it should — resolving + // it the other way reported a photo sitting right there as destroyed. The file seeded here has no row, so the + // current schema's row can only account for it if the directory was resolved the write path's way. + val scan = await(healthService.getDbHealth).mediaStorage.value.storyMedia.value + val mine = scan.cities.find(_.schema == run(healthTable.getCurrentSchema)).value + + mine.cityId.value mustBe config.get[String]("city-id") + mine.scanned mustBe true + mine.orphanIds must contain(orphanId) + } + + "claim that directory exclusively, so no other schema reads those same files as its own orphans" in { + val scan = await(healthService.getDbHealth).mediaStorage.value + + val claimed = scan.storyMedia.value.cities.flatMap(_.cityId) + claimed mustBe claimed.distinct + // Every city that lost the directory has to say why, or the operator reading the row has nowhere to start. + scan.storyMedia.value.cities.filterNot(_.scanned).foreach(_.unscannedReason mustBe defined) + } + + "cover every schema holding a story_media table, in a stable order" in { + // The panel is read down the page, and a scan that silently dropped a city would look exactly like a city with + // nothing to report. + val scan = await(healthService.getDbHealth).mediaStorage.value.storyMedia.value + val schemas = run(healthTable.getStoryMediaSchemas).filter(_.matches("^[A-Za-z0-9_]+$")) + + scan.cities.map(_.schema) mustBe schemas.sorted + } + + "agree with the boot check about whether the media-directory contract is being enforced" in { + // Two copies of the arming rule would let the page claim a stage is guarded when the check isn't watching. + val media = await(healthService.getDbHealth).mediaStorage.value + media.enforced mustBe modules.PersistentMediaDirCheck.arms(app.injector.instanceOf[play.api.Environment]) + } + "survive a burst of concurrent polls without exhausting the connection pool" in { // Simulate many Owner tabs polling at once, from a cold cache (the worst case). If getDbHealth fanned out one // query per city schema, 30 concurrent calls would each demand ~one-connection-per-schema and, on a many-city diff --git a/test/service/LostMediaLogSpec.scala b/test/service/LostMediaLogSpec.scala new file mode 100644 index 0000000000..00d0a11a98 --- /dev/null +++ b/test/service/LostMediaLogSpec.scala @@ -0,0 +1,100 @@ +package service + +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.classic.{Level, Logger => LogbackLogger} +import ch.qos.logback.core.read.ListAppender +import org.scalatestplus.play.PlaySpec +import org.slf4j.LoggerFactory + +import scala.jdk.CollectionConverters._ + +/** + * The one thing standing between a destroyed file and nobody finding out for six days (#4925). + * + * Its whole contract is what it says and how often — and both failure modes are silent. Announcing every request + * buries the loss in its own noise; announcing nothing is indistinguishable from health, which is the state #4926 + * exists to end. So the log lines themselves are the assertions here. + */ +class LostMediaLogSpec extends PlaySpec { + + /** + * Runs `f` against a fresh log with an appender attached, and returns everything it logged. + * + * The level is forced rather than inherited so the spec pins the code's own severity choice instead of whatever + * the ambient logback config happens to permit. + */ + private def captured(f: LostMediaLog => Unit): Seq[ILoggingEvent] = { + val logger = LoggerFactory.getLogger(classOf[LostMediaLog]).asInstanceOf[LogbackLogger] + val appender = new ListAppender[ILoggingEvent] + val original = logger.getLevel + appender.start() + logger.addAppender(appender) + logger.setLevel(Level.WARN) + // Detach from the console for the duration: these cases deliberately report thousands of losses, and a suite that + // buries its own output in fake alarms is the noise problem this class exists to avoid, one level up. + logger.setAdditive(false) + try f(new LostMediaLog) + finally { + logger.setAdditive(true) + logger.setLevel(original) + logger.detachAppender(appender) + appender.stop() + } + appender.list.asScala.toSeq + } + + "LostMediaLog" should { + "say what was lost and where its bytes should have been" in { + val events = captured(_.reportMissing("story_media", "331", "/srv/media/chicago-il/story_331.jpg", true)) + events must have size 1 + val message = events.head.getFormattedMessage + message must include("story_media") + message must include("331") + message must include("/srv/media/chicago-il/story_331.jpg") + } + + "announce a lost item once, however many times it is requested" in { + // One popular page re-requesting a lost file would otherwise write the same line thousands of times and bury + // the event in its own noise. + val events = captured { log => + (1 to 25).foreach(_ => log.reportMissing("story_media", "331", "/srv/media/story_331.jpg", true)) + } + events must have size 1 + } + + "announce every distinct item, so a whole directory going missing reads as a whole directory" in { + val events = captured { log => + Seq("331", "332", "333").foreach(id => log.reportMissing("story_media", id, s"/srv/media/$id.jpg", true)) + } + events.map(_.getFormattedMessage).mkString(" ") must include("333") + events must have size 3 + } + + "keep kinds apart, since a story and a pano can share an id" in { + val events = captured { log => + log.reportMissing("story_media", "1", "/srv/media/story_1.jpg", irreplaceable = true) + log.reportMissing("pano", "1", "/srv/panos/1.jpg", irreplaceable = true) + } + events must have size 2 + } + + "log content no rebuild can recreate at ERROR, and rebuildable content at WARN" in { + // The tiering is the same call PersistentMediaDirCheck makes, and it is what decides whether anyone is paged. + val error = captured(_.reportMissing("pano", "abc", "/srv/panos/abc.jpg", irreplaceable = true)) + error.head.getLevel mustBe Level.ERROR + val warn = captured(_.reportMissing("share_image", "7", "/srv/share/share_7.jpg", irreplaceable = false)) + warn.head.getLevel mustBe Level.WARN + } + + "let a still-lost item announce itself again once it has been crowded out" in { + // The tracking set is bounded, because a dead mount can make every pano in a city report at once. Eviction + // eventually re-announcing a loss is the right way for that bound to fail; going permanently quiet is not. + val events = captured { log => + log.reportMissing("pano", "first", "/srv/panos/first.jpg", irreplaceable = true) + (1 to 5000).foreach(i => log.reportMissing("pano", s"filler$i", s"/srv/panos/$i.jpg", irreplaceable = true)) + log.reportMissing("pano", "first", "/srv/panos/first.jpg", irreplaceable = true) + } + events.last.getFormattedMessage must include("pano first ") + } + } +} diff --git a/test/service/MediaIntegritySpec.scala b/test/service/MediaIntegritySpec.scala new file mode 100644 index 0000000000..eeeaebde17 --- /dev/null +++ b/test/service/MediaIntegritySpec.scala @@ -0,0 +1,298 @@ +package service + +import modules.PersistentMediaDirCheck.persistentDirs +import org.scalatestplus.play.PlaySpec +import play.api.{Configuration, Environment, Mode} + +import java.io.File +import java.nio.file.Files + +/** + * The media-storage panel's judgment calls (#4926): which of them mean data has been lost, and — just as important — + * which of them don't. + * + * A monitor that reports loss when a directory is merely absent, or on a dev checkout where the relative defaults are + * the intended behavior, gets ignored, and an ignored monitor is worth nothing. So the false-alarm cases are pinned + * here as tightly as the real ones. + * + * Pure logic — no app boot and no database; the only filesystem it touches is a temp directory it creates itself. + */ +class MediaIntegritySpec extends PlaySpec { + + private val appRoot = new File("/srv/sidewalk/target/universal/stage") + + private def env(mode: Mode): Environment = Environment(appRoot, getClass.getClassLoader, mode) + + private def config(path: String): Configuration = + Configuration.from(persistentDirs.map(_.key -> path).toMap) + + private def statusFor(path: String, mode: Mode): MediaDirStatus = + MediaIntegrity.directoryStatuses(config(path), env(mode)).head + + /** A temp directory holding the given file names, cleaned up by the JVM's temp handling. */ + private def dirContaining(names: String*): File = { + val dir = Files.createTempDirectory("media-integrity-spec").toFile + dir.deleteOnExit() + names.foreach { name => + val f = new File(dir, name) + f.createNewFile() + f.deleteOnExit() + } + dir + } + + "directoryStatuses" should { + "cover every directory the boot check guards, so the page can't quietly omit one" in { + MediaIntegrity.directoryStatuses(config("/srv/media"), env(Mode.Prod)).map(_.key) mustBe persistentDirs.map(_.key) + } + + "flag a directory a deploy would delete as bad when it holds irreplaceable content" in { + val prod = MediaIntegrity.directoryStatuses(config(".story-media"), env(Mode.Prod)) + prod.filter(_.irreplaceable).foreach { d => + d.status mustBe "unsafe" + d.severity mustBe "bad" + } + prod.filterNot(_.irreplaceable).foreach(_.severity mustBe "warn") + } + + "not alarm on the same directory in dev, where the relative defaults are supposed to land in the checkout" in { + MediaIntegrity.directoryStatuses(config(".story-media"), env(Mode.Dev)).foreach { d => + d.status mustBe "unsafe" + d.severity mustBe "ok" + } + } + + "report a directory that doesn't exist yet as normal, since the write paths create it on first upload" in { + val status = statusFor("/srv/sidewalk-media/nothing-here-yet", Mode.Prod) + status.status mustBe "absent" + status.severity mustBe "ok" + } + + "report a usable directory as ok" in { + val status = statusFor(dirContaining().getAbsolutePath, Mode.Prod) + status.status mustBe "ok" + status.severity mustBe "good" + } + + "surface a blank value rather than resolving it, since it would target the filesystem root" in { + val status = statusFor("", Mode.Prod) + status.status mustBe "unresolved" + status.severity mustBe "bad" + status.detail.value must include("set but empty") + } + + "name the environment variable that fixes each directory" in { + MediaIntegrity.directoryStatuses(config(".story-media"), env(Mode.Prod)).map(_.envVar) mustBe + persistentDirs.map(_.envVar) + } + } + + private val irreplaceableDir = persistentDirs.find(_.irreplaceable).value + + // The permission branches can't be provoked through the filesystem from a suite that runs as root — which CI and + // the dev container both do, and where chmod 000 still reads and writes fine — so they are pinned on the rules + // themselves. + "dirStatus" should { + "call a directory this process cannot read bad, since nothing in it can be verified" in { + val probe = MediaIntegrity.DirProbe(exists = true, readable = false, writable = true) + val status = MediaIntegrity.dirStatus(irreplaceableDir, "/srv/media", probe, None, enforced = true) + status.status mustBe "not_readable" + status.severity mustBe "bad" + status.detail.value must include(irreplaceableDir.envVar) + } + + "call a directory this process cannot write to bad, since uploads will fail against it" in { + val probe = MediaIntegrity.DirProbe(exists = true, readable = true, writable = false) + val status = MediaIntegrity.dirStatus(irreplaceableDir, "/srv/media", probe, None, enforced = true) + status.status mustBe "not_writable" + status.severity mustBe "bad" + } + + "report an unsafe directory before either permission, since a deploy deleting it outranks both" in { + val probe = MediaIntegrity.DirProbe(exists = true, readable = false, writable = false) + val status = + MediaIntegrity.dirStatus(irreplaceableDir, "/srv/media", probe, Some("in the wipe zone"), enforced = true) + status.status mustBe "unsafe" + } + } + + private val configuredCities = Map("sidewalk_chicago" -> "chicago-il", "sidewalk_seattle" -> "seattle-wa") + + /** The reason a schema has no directory, failing the test if it turned out to have one. */ + private def unlocatableReason(target: ScanTarget): String = target match { + case ScanTarget.Unlocatable(reason) => reason + case ScanTarget.Dir(cityId) => fail(s"expected no directory for this schema, got $cityId") + } + + "scanTargets" should { + "use the configured city for every schema when nothing disagrees" in { + MediaIntegrity.scanTargets( + Seq("sidewalk_chicago", "sidewalk_seattle"), + "sidewalk_chicago", + "chicago-il", + configuredCities + ) mustBe Map( + "sidewalk_chicago" -> ScanTarget.Dir("chicago-il"), + "sidewalk_seattle" -> ScanTarget.Dir("seattle-wa") + ) + } + + "follow this instance's own city-id, since that is what the write path builds its path from" in { + // A dev container left with city-id and the connection's schema disagreeing writes media under the city-id. + val targets = MediaIntegrity.scanTargets( + Seq("sidewalk_chicago", "sidewalk_seattle"), + "sidewalk_chicago", + "seattle-wa", + configuredCities + ) + targets("sidewalk_chicago") mustBe ScanTarget.Dir("seattle-wa") + } + + "not let a second schema claim a directory this instance already writes to" in { + // Otherwise the other schema lists the same files and reports every one of them as an orphan. + val targets = MediaIntegrity.scanTargets( + Seq("sidewalk_chicago", "sidewalk_seattle"), + "sidewalk_chicago", + "seattle-wa", + configuredCities + ) + // And it has to say which of the two reasons applies: the operator staring at this row on a misconfigured + // container needs to know the city is configured and its directory was taken, not go hunting for a config gap. + val reason = unlocatableReason(targets("sidewalk_seattle")) + reason must include("sidewalk_chicago") + reason must include("seattle-wa") + } + + "say plainly when no city on the stage names a schema, rather than guessing at its directory" in { + val targets = + MediaIntegrity.scanTargets(Seq("sidewalk_elsewhere"), "sidewalk_chicago", "chicago-il", configuredCities) + unlocatableReason(targets("sidewalk_elsewhere")) must include("sidewalk_elsewhere") + } + } + + "compareCity" should { + "report a city whose rows all have files as clean" in { + val result = MediaIntegrity.compareCity( + "chicago-il", + "sidewalk_chicago", + Seq(1, 2), + DirListing.Listed(Seq("story_1.jpg", "story_2.jpg")) + ) + result.missing mustBe 0 + result.orphans mustBe 0 + result.rows mustBe 2 + result.scanned mustBe true + } + + "report a row with no file as missing — the #4925 loss" in { + val result = + MediaIntegrity.compareCity("chicago-il", "sidewalk_chicago", Seq(1, 2), DirListing.Listed(Seq("story_1.jpg"))) + result.missing mustBe 1 + result.missingIds mustBe Seq(2) + result.orphans mustBe 0 + } + + "report a file with no row as orphaned — a retraction whose file delete didn't land" in { + val result = MediaIntegrity.compareCity( + "chicago-il", + "sidewalk_chicago", + Seq(1), + DirListing.Listed(Seq("story_1.jpg", "story_7.jpg")) + ) + result.orphans mustBe 1 + result.orphanIds mustBe Seq(7) + result.missing mustBe 0 + } + + "count both directions at once, which a row-count comparison alone would call clean" in { + val result = + MediaIntegrity.compareCity("chicago-il", "sidewalk_chicago", Seq(1), DirListing.Listed(Seq("story_9.jpg"))) + result.missing mustBe 1 + result.orphans mustBe 1 + } + + "ignore files that aren't story media, so a stray README isn't reported as an orphan" in { + val result = MediaIntegrity.compareCity( + "chicago-il", + "sidewalk_chicago", + Seq(1), + DirListing.Listed(Seq("story_1.jpg", "README.txt", "story_1.jpg.bak", "story_.jpg")) + ) + result.orphans mustBe 0 + result.missing mustBe 0 + } + + "treat an absent city directory as loss, since the write path creates it and never removes it" in { + val result = MediaIntegrity.compareCity("chicago-il", "sidewalk_chicago", Seq(1, 2), DirListing.Absent) + result.missing mustBe 2 + result.scanned mustBe true + } + + "report a city with no rows and no directory as clean rather than as a fault" in { + val result = MediaIntegrity.compareCity("chicago-il", "sidewalk_chicago", Seq.empty, DirListing.Absent) + result.missing mustBe 0 + result.orphans mustBe 0 + result.scanned mustBe true + } + + "decline to call an unreadable directory data loss, and say why it declined" in { + // A directory this process may not read holds exactly the same photos it held a moment ago. Counting all of + // them as destroyed would put the fleet's whole story archive on the panel as lost over a permissions change, + // and a monitor that does that once gets ignored forever after. + val result = MediaIntegrity.compareCity("chicago-il", "sidewalk_chicago", Seq(1, 2), DirListing.Unreadable) + result.scanned mustBe false + result.missing mustBe 0 + result.rows mustBe 2 + result.unscannedReason.value must include("not readable") + } + } + + "unscannedCity" should { + "still report the rows it knows about, so the city doesn't read as empty" in { + val result = MediaIntegrity.unscannedCity(None, "sidewalk_somewhere", Seq(1, 2), "no city names this schema") + result.scanned mustBe false + result.rows mustBe 2 + result.missing mustBe 0 + result.unscannedReason.value mustBe "no city names this schema" + } + } + + "scanRefusal" should { + "run the scan against a readable directory" in { + MediaIntegrity.scanRefusal("/srv/media", isDirectory = true, canRead = true) mustBe None + } + + "decline, naming the path, when there is no directory to compare against" in { + // No upload has landed on this stage yet, or the whole directory is gone; the per-directory table above already + // reports which, and inventing per-city loss counts from it would say the wrong one. + val refusal = MediaIntegrity.scanRefusal("/srv/media", isDirectory = false, canRead = true).value + refusal must include("/srv/media") + refusal must include("No media directory") + } + + "decline rather than report loss when the directory is there but unreadable" in { + // The trap this exists for: `isDirectory` is true for a directory this process may not read, and every listing + // beneath it comes back empty — which would announce every story photo on the stage as destroyed. + val refusal = MediaIntegrity.scanRefusal("/srv/media", isDirectory = true, canRead = false).value + refusal must include("/srv/media") + refusal must include("not readable") + } + } + + "listing" should { + "read a directory's contents" in { + MediaIntegrity.listing(dirContaining("story_3.jpg")) mustBe DirListing.Listed(Seq("story_3.jpg")) + } + + "call a path with nothing at it absent, which for a city directory means no upload has landed" in { + MediaIntegrity.listing(new File("/srv/sidewalk-media/no-such-directory")) mustBe DirListing.Absent + } + + "call something that is there but won't list unreadable rather than absent" in { + // `File.list` answers null for both, and the two are opposite verdicts: nothing was ever written here, versus + // this process cannot see what is here. A regular file is the case a root-running suite can actually provoke. + val notADirectory = new File(dirContaining("story_3.jpg"), "story_3.jpg") + MediaIntegrity.listing(notADirectory) mustBe DirListing.Unreadable + } + } +} diff --git a/test/service/SingleFlightGateSpec.scala b/test/service/SingleFlightGateSpec.scala new file mode 100644 index 0000000000..ac35d3b042 --- /dev/null +++ b/test/service/SingleFlightGateSpec.scala @@ -0,0 +1,90 @@ +package service + +import org.scalatest.concurrent.ScalaFutures +import org.scalatestplus.play.PlaySpec + +import java.util.concurrent.atomic.AtomicInteger +import scala.concurrent.duration._ +import scala.concurrent.{Await, Future, Promise} + +/** + * The gate that keeps a stuck media scan from being joined by a fresh copy on every poll (#4926). + * + * Its whole job is invisible when storage is healthy and decisive when it isn't: a filesystem call against a dead + * mount never returns and cannot be cancelled, so a dashboard polling every ~20 seconds parks a thread per poll until + * the blocking-io pool is gone — and the panel that exists to report storage trouble goes dark exactly when storage + * is the trouble. The cases below pin the one distinction that makes that work: the gate opens when the *work* + * finishes, never when a caller stops waiting for it. + * + * Pure logic — no app boot, no database, no filesystem. + */ +class SingleFlightGateSpec extends PlaySpec with ScalaFutures { + + private def await[T](f: Future[T]): T = Await.result(f, 5.seconds) + + "SingleFlightGate" should { + "run the work and hand back its result when nothing else is running" in { + val gate = new SingleFlightGate + await(gate.runOrElse("busy")(Future.successful("scanned"))) mustBe "scanned" + } + + "answer a caller who arrives mid-flight with the stand-in, without starting a second copy" in { + val gate = new SingleFlightGate + val started = new AtomicInteger(0) + val blocker = Promise[String]() + + val first = gate.runOrElse("busy") { started.incrementAndGet(); blocker.future } + val second = gate.runOrElse("busy") { started.incrementAndGet(); Future.successful("second") } + + await(second) mustBe "busy" + // The point isn't the answer the second caller got, it's that no second scan exists to park a second thread. + started.get mustBe 1 + blocker.success("first") + await(first) mustBe "first" + } + + "keep the gate shut while the work runs on, even after the caller has given up waiting for it" in { + // The deadline protects the poll; only this protects the pool. A timeout that reopened the gate would let the + // next poll start another scan on top of a filesystem call that has not returned and may never. + val gate = new SingleFlightGate + val blocker = Promise[String]() + val first = gate.runOrElse("busy")(blocker.future) + + // What a caller timing out looks like from here: it stops waiting, and the work keeps running. + first.isCompleted mustBe false + await(gate.runOrElse("busy")(Future.successful("second"))) mustBe "busy" + + blocker.success("late") + await(first) mustBe "late" + await(gate.runOrElse("busy")(Future.successful("third"))) mustBe "third" + } + + "reopen once the work completes, so a recovered mount is reported instead of a permanent stand-in" in { + val gate = new SingleFlightGate + await(gate.runOrElse("busy")(Future.successful("first"))) mustBe "first" + await(gate.runOrElse("busy")(Future.successful("second"))) mustBe "second" + } + + "treat a failure as a completion, since only work still running should keep the next caller out" in { + val gate = new SingleFlightGate + val failed = gate.runOrElse("busy")(Future.failed(new RuntimeException("mount gone"))) + whenReady(failed.failed)(_.getMessage mustBe "mount gone") + await(gate.runOrElse("busy")(Future.successful("after"))) mustBe "after" + } + + "reopen when the work throws before it returns a future at all" in { + // A synchronous throw is the easiest way to wedge a gate shut forever: nothing ever completes to reopen it. + val gate = new SingleFlightGate + val thrown = gate.runOrElse("busy")(throw new IllegalStateException("bad config")) + whenReady(thrown.failed)(_.getMessage mustBe "bad config") + await(gate.runOrElse("busy")(Future.successful("after"))) mustBe "after" + } + + "never build the stand-in on the common path, since it is only meaningful when something is stuck" in { + val gate = new SingleFlightGate + val built = new AtomicInteger(0) + await(gate.runOrElse { built.incrementAndGet(); "busy" }(Future.successful("scanned"))) mustBe "scanned" + built.get mustBe 0 + } + } +}