feat(board): a board and a seat carry a picture somebody chose - #955
Conversation
49d1534 to
0c176ed
Compare
|
The one red check — What the number wasThe last assertion was
Checked rather than reasoned about, in the same headless chromium the suite drives:
Why the art is exonerated by the failure itselfBecause the correction divides by a density taken from the descriptor, not from the bytes, The changeOnly assertThat(photo.fetchedWidth)
.describedAs("the fetched copys own pixel width, against the width its name claims")
.isEqualTo(photo.chosenWidth)The property being proved is unchanged and is now proved directly rather than through a number that means something else: a band asking for 400 pixels is sent a narrow copy, and the bytes behind that address are as wide as its name says. The two assertions that carry the saving — Verification
|
0c176ed to
ba44142
Compare
The system test's last assertion compared `img.naturalWidth` against the width named in the copy the browser fetched, and those are not the same number. Under a `srcset` of `w` descriptors the html spec has a browser divide a picture's intrinsic width by its current pixel density, and that density is the chosen descriptor over the `sizes` value — so a 640 copy asked for at 400px reports 400, and that number can only ever be the `sizes` value. Nothing is wrong with the stored art: 400 is 640 / 1.6, which is reachable only if the bytes behind `-640.webp` really are 640 pixels wide. Confirmed in chromium, where a ladder whose 640 rung is served 320-wide bytes reports 200 rather than 400. So the test reads the fetched copy's own width off a second `img` carrying no `srcset`, where nothing is density-corrected, and asserts that against the width its address claims. What is being proved is unchanged and is now proved directly: a band asking for 400 pixels is sent a narrow copy, and the bytes behind that address are as wide as its name says.
ba44142 to
b889703
Compare
|
Rebased onto One conflict, in the boards adapter's import block: Verified after the rebase, exit codes not grepped output:
The integration and system suites are CI's to run, since they need a real database and the compose stack. |
Slice 3 of #922. A board answers with its group photograph and a seat with its portrait, both uploaded through the file service and served at a ladder of widths, and the twenty-six photographs the repository has always shipped are put onto the records the seed files name. The page that draws them is #930's.
Two new picture kinds
FileTypegainsBOARD_PHOTOandBOARD_PORTRAIT. Both are publicly readable and both admit the same content types as the existing page pictures.BOARD_PHOTOboard-photosBOARD_PORTRAITboard-portraitsA board photo is a game banner's twin — drawn full-bleed across a band, so it takes the same ceiling and the same ladder, and nothing about it is a new decision.
The portrait's ladder is argued the way
ROSTER_ICONargues its cap down from the other icons. A seat shows a thumbnail of about 96px beside the name and opens to a picture of about 320px. So 160 covers the thumbnail, 320 covers it on a dense display and covers the opened picture, and 640 covers the opened picture on a dense display. Nothing on the page draws a portrait wider than that, so the ladder stops at 640 — a portrait carrying a banner's copies would be twenty-one more group photographs for a reader to download.Two things about the portrait are not a copy of an icon's rules and are worth naming:
maxImageEdgecaps the longest edge, and a portrait is taller than it is wide, so 960 there is about 640 across — which is exactly the widest rung. An icon is square, so forGAME_ICONandROSTER_ICONthe ceiling and the widest rung are the same number; for a portrait they cannot be.ROSTER_ICONtakes. 15 MB rather than an icon's 5 because what arrives is a phone camera's output: the ceiling is applied after the file is admitted rather than instead of admitting it.What I found about the unique
picture_idconstraintboardsandboard_memberseach carry a unique key on(picture_id, deleted_at)—uk_boards_picture_deleted_atanduk_board_members_picture_deleted_at, both fromV28. Storage is content-addressed, so two rows naming one art file resolve to oneFilerow that only one of them can hold, and the second insert violates the key. Esports sidesteps this because a team's banner belongs to the fielding; a board's photograph has no such escape.No two rows name the same art today. All twenty-six names are distinct, and so are all twenty-six files' SHA-256 digests, checked rather than assumed — so nothing could collide even by accident of identical bytes. Two things now hold that:
ShippedBoardArtFilesTestfails the build if a name is ever used twice, alongside the two checks its esports twin makes (a row naming art nobody committed, a committed file no row names). That is the real guard: the collision becomes a build failure rather than a start-up log line.ShippedBoardArtasks whether that stored picture is already held by another board or seat; if it is, it logs which one and leaves it there. A determinate winner and a sentence, instead of a constraint violation surfacing half way through a start.ShippedBoardArtITasserts it by handing board 9's photograph to board 8 and running the loader again: board 8 keeps it, board 9 stays without one, nothing throws.One consequence worth recording for #930 and #935, which I have not changed: because the key is on
picture_id, oneFilerow backs at most one board. If a person uploads byte-identical bytes for two different boards, storage hands both saves the same row and the second save fails on the key rather than being refused in words. That is the constraint working as written — a picture belongs to one board — but the answer a person gets is not a good one. It is not reachable through the shipped art and it is not what this ticket asks for, so it is left alone and named here.The art
Twenty-six WebP masters under
services/api/src/main/resources/db/seed/boards/art/, 2.1 MB in total, with aCREDITS.mdbeside them the way the esports art has one. Five board photographs (boards 5 to 9) and twenty-one portraits.Named flat, lowercase and hyphenated, and the name is derived from the CSV cell with no table in between. A row's
photoorportraitcell is the file's name with.webpfor an extension —board6andboard6-amber— so there is nothing that can fall out of step with the files. I considered the nested rule (art/board9/board9.webp, taken from the row'simagevalue) and kept the flat one: it derives the filename from a cell just as directly, it does not tie the art's layout to a column #935 deletes, and it matches the esports house style.board7-reiniisboard7/reinout.jpg, named for the seat rather than for the asset;CREDITS.mdrecords that.Board 6's portraits are cut from the
*BIG.jpgoriginals, not the small copies beside them — 3283x4924 up to 3803x5705 rather than about 1100x1700. A portrait is enlarged when a seat opens, so resolution thrown away here cannot come back; those six are the only portraits in the repository with a larger copy available, and they are the only six that fill all three rungs. I checked the framing matches before switching (RMSE under 0.2% against the small copies at a common size, which is resampling noise).Sizes: photographs fitted inside 2560x1440 at quality 82, portraits to 960 on the longest edge at quality 85, resized with ImageMagick's Lanczos filter where a resize was needed and encoded with
cwebp. Nothing is upscaled — three photographs and fifteen portraits keep their originals' own size, which is why fifteen portraits' ladders stop at 320.CREDITS.mdhas the table and the rights note: these are the association's own photographs of its own boards, so none of the publisher-art caveats indb/seed/esports/artapply.Nothing is deleted from
src/assets/. That is #935's, and theimagecolumns must keep working until then.The seed files, and the checksum
boards.csvgains aphotocolumn andseats.csvaportraitcolumn, each naming an art file without its extension — exactly the wayteams.csvnames a banner. Five and twenty-one cells are filled; the rest are blank, because boards 1 to 4 were never photographed and board 10 has not sat.The rows were rewritten through a real CSV reader rather than by appending to lines, because one blurb (Taha's) carries a newline inside a quoted field and is two physical lines. A no-op round trip through that reader is byte-identical to the committed file, so the only change in either file is a trailing column.
R__Boards_seeddoes not store pictures and is unchanged except for its header. Storing a picture needs the storage volume and the converter, which a migration runner has neither of, so the two new columns are read byShippedBoardArtonce the application is up. Its counters are untouched, so the CodeQLjava/constant-comparisonfix in flight elsewhere does not conflict.Editing the files moves the migration's checksum, and that is correct.
getChecksum()hashes the two files' contents, which is what makes an edit take effect at all. Flyway re-runs the repeatable seed, the seed is idempotent — the update statements carry aNOT (a <=> ? AND …)guard — and a row that already agrees is not even written.BoardSeedLoadIT's "a second run changes nothing" case still passes.ShippedBoardArtA
@Componentin the board module, applied onApplicationReadyEventbyShippedBoardArtOnStartupso the work goes through the proxy. Every rule is the esports loader's, and every one is asserted:systemaccount, because nobody chose them. A missing account is a warning and no work rather than a failed start.attempt, and again in the startup bean, so art that did not land never stops the application coming up.Responses and requests
BoardResponse.photoandBoardMemberResponse.portraitare the sameImageshape the esports payloads already carry — address, path, own dimensions and stored widths — mapped through the existingFile.asImage()rather than a second mapper. So the frontend'ssrcsetOf/sizeOfincomponents/island/pictures.tswork on them unchanged.Create and update take a stored picture path, resolved through
StoredPictures, so a path naming nothing stored is refused with400 PictureNotStoredand a path naming a picture of the wrong kind is refused the same way — a photograph field will not take a portrait. Both are asserted.pictureIdis gone from the board payloads, replaced byphoto. It was a rawfilesrow id, no client ever sent it, and leaving two request fields writing one column would have been two ways to set a picture that could disagree.photocarries the same fact plus the address and the widths.imagedeliberately survives, on both the board and the seat, and answers alongside the new pictures. This is the expand half of an expand–contract:/boardstill draws the asset file names and #935 drops the columns.BoardControllerITasserts a board answering withimageandphotoin the same payload, so a future change that quietly drops one fails.Board.vueandBoardManager.vueare untouched — they readimage, which still answers.Permissions are unchanged. The upload endpoint is
hasPermission('__NO_TARGET__', 'Team', 'write'), which isBOARDand above, andBoardPermissiongrants write to exactly the same people, so nothing needed widening for a board member to upload a photograph.Generated client
Regenerated the way CI's Validate OpenAPI client generation job does —
./gradlew :services:api:dumpOpenApiSpec,normalize_api_spec,yarn gen:blueshell,yarn lint:gen. The diff isservices/api/openapi.jsonplustypes.gen.ts, and every changed line intypes.gen.tsis a board type or one of the two newFileTypevalues. Nothing else moved.Tests
Exit codes checked rather than output grepped. The integration suite ran against a throwaway MariaDB 10.11.10 container on port 3315 (and again on 3316 after the art was recut) with
blueshell-testcreated fromservices/api/0_init.sqland CI's ownMYSQL_*env,cwebpand ImageMagick present../gradlew :services:api:compileKotlin :services:api:compileTestKotlin :services:api:compileIntegrationTestKotlin./gradlew :tests:system:compileTestKotlin./gradlew :services:api:test./gradlew :services:api:integrationTest --tests "net.blueshell.api.board.*"./gradlew :services:api:integrationTest(full suite)yarn typecheckyarn lint --max-warnings=0yarn test:unitnpx playwright test --grep "board"npx playwright test(whole e2e suite)New tests:
ShippedBoardArtIT— a Spring integration test against the real converter and the real storage volume. A board the file names has a photograph of the right kind; it is stored at the widths its kind lists, stopping at the master's own width; a seat has a portrait at a portrait's widths; a portrait narrower than the ladder is stored at the rungs it can fill; every picture the loader stores has more than one width and none wider than itself; five boards and twenty-one seats get art and the five boards the files do not name get none; a second run changes nothing; deleted bytes are written again at the address they had, for both kinds; a chosen photograph and a chosen portrait are not replaced; and a photograph another board already holds is left there.ShippedBoardArtFilesTest— a pure unit test on the files themselves: every name is on the classpath, every committed file is named by a row, no picture is named twice, and the counts are what they should be so the other three cannot pass against files that name nothing.BoardControllerIT— extended: a photograph round-tripping through create, update and a public read with its size and its widths, besideimage; a portrait round-tripping through add, update and read; a path naming nothing stored refused; and a portrait refused where a photograph is expected.BoardUseCasesTest— the photograph and the portrait taken from a stored path, cleared when none is named, and applied to a seat somebody already holds.BoardArtSystemTest— against the real stack: the api answers with a seeded photograph, a real browser composes asrcsetfrom the published widths, fetches one of the narrow copies rather than the master and decodes it; and the same for a portrait, whose widest copy is 640 rather than a banner's. Driven from the api's own origin, since the board page that will draw these is The board page reads as a timeline #930's and nothing here should depend on a CORS rule.What I could not verify
yarn test:e2efails wholesale on this machine, on cleanmainas much as on this branch. Every test times out inpage.goto, because the page requestshttps://fonts.googleapis.com/css?family=Roboto…and this machine's DNS cannot resolve it, so theloadevent never fires. I proved it is not this branch by reverting the two frontend files, rebuilding and watching the same test fail, then found the pending request with a probe script. The numbers above come fromnpx playwright testwith--host-resolver-rules=MAP fonts.googleapis.com ~NOTFOUNDin a throwaway config, which is not committed; with the request failing fast instead of hanging, the whole suite is green. CI has network access and is unaffected.Closes #929
Diff breakdown —
█added░removed, scaled to the largest row.