feat: return the whole Cache from GET /layer/{layerName}/{cacheType} - #175
Open
razbroc wants to merge 5 commits into
Open
feat: return the whole Cache from GET /layer/{layerName}/{cacheType}#175razbroc wants to merge 5 commits into
razbroc wants to merge 5 commits into
Conversation
IMapProxyJsonDocument.caches was typed as a single Cache rather than a map of them, so every caches[name] lookup resolved to any through the Cache type's permissive index signature, and the casts around those lookups were unchecked. No behaviour change.
The endpoint returned only the Cache Source, so a consumer needing the Cache's
grids, format or sources had to fall back on the deprecated GET /layer/{name},
which has no Cache Type selection and cannot reach a Redis Cache at all.
The Cache is now spread verbatim under its resolved name, with the Cache Source
left in place at 'cache' so existing consumers are unaffected and the new fields
are additive. Cache Type resolution is unchanged.
readCacheType lifts the Cache Type out of an untrusted entry, so an entry that is
not an object or holds no Cache Source is the same 400 as any other Cache Type
mismatch instead of a 500. There is deliberately no notion of a malformed Cache:
nothing else about the entry is inspected.
Every Cache Source schema now requires only 'type' and permits additional properties, so a Cache Source missing an optional field no longer makes a valid response contract-invalid. getCacheResponse requires only cacheName and cache, documents sources, grids, format, upscale_tiles and minimize_meta_requests as optional, and permits additional properties at both levels: production redis Caches carry neither upscale_tiles nor minimize_meta_requests, so anything stricter would make real responses violate their own spec. geopackage was an accepted cacheType with no response schema. It is added to both the oneOf and the discriminator mapping - without the mapping entry a real geopackage response fails the contract regardless of tests. The response examples showed only the Cache Source and are updated to the whole Cache. The request schemas are separate objects and are untouched.
…Type exists The Cache Type mismatch answered 400 on master, with the message 'layer cache not found with requested cache type'. The status and the message disagreed, and the message was the honest half: when the Cache Type is in the enum but the Layer owns no Cache of that type, nothing about the request is malformed - the addressed Cache simply is not there. Every 'the addressed Cache is not there' case now answers alike: Layer absent, no Cache under the resolved name, and a Cache Type that cannot be confirmed. A Cache Type outside the enum still answers 400 from request validation, before the manager runs. The unreadable entry follows the mismatch to 404 rather than keeping a 400 of its own. Telling 'the Cache Type is confirmably something else' from 'the Cache Type could not be confirmed' would reintroduce the malformedness concept the design deliberately does not carry, and a 400 would blame the caller for the server's own corrupt configuration. The operator's signal stays the warn log carrying the offending entry. BEHAVIOUR CHANGE to a released endpoint: a consumer branching on 400 sees 404.
…e check in LayersManager
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues: MAPCO-11508
Further information:
GET /layer/{layerName}/{cacheType}returned only the Cache Source — the storage backend nested undercache— so a consumer needing the Cache'sgrids,formatorsourceshad to fall back on the deprecatedGET /layer/{name}, which has no Cache Type selection and therefore cannot reach a Redis Cache at all.Three commits:
1.
fix:type the mapproxy caches section as a map of Caches — prefactor, no behaviour change.IMapProxyJsonDocument.cacheswas typed as a singleIMapProxyCache, so everycaches[name]lookup resolved toanythrough the Cache type's permissive index signature. Threeno-unsafe-member-accesssuppressions deleted.2.
feat:return the whole Cache — the Cache is spread verbatim under its resolved name, with the Cache Source left in place atcache, so existing consumers are unaffected and every new field is additive. Cache Type resolution is unchanged: a redis request still resolves to the-redissuffixed Cache, and the returnedcacheNameis the resolved name.readCacheTypelifts the Cache Type out of an untrusted entry, so an entry that is not an object, or holds no Cache Source, is now the same answer as any ordinary Cache Type mismatch rather than a 500. There is deliberately no notion of a malformed Cache — nothing else about the entry is inspected. Existing error message wording is preserved.3.
feat:loosen the Cache Source schemas, publish the geopackage schema — every Cache Source schema now requires onlytypeand permits additional properties;getCacheResponserequires onlycacheNameandcache, documents the five optional fields, and permits additional properties at both levels. This is forced by production data: redis Caches in the live configuration carry neitherupscale_tilesnorminimize_meta_requests, so anything stricter would make real responses violate their own spec.geopackagewas an acceptedcacheTypewith no response schema; it is added to both theoneOfand the discriminator mapping. Request schemas are separate objects and are untouched — insert validation is unchanged.4.⚠️ behaviour change to a released endpoint.
fix:answer 404 rather than 400 when no Cache of the requested Cache Type exists —The mismatch answered 400 on master, with the message
layer cache not found with requested cache type. The status and the message disagreed, and the message was the honest half: when the Cache Type is in the enum but the Layer owns no Cache of that type, nothing about the request is malformed — the addressed Cache simply is not there. Live on dev,GET /layer/TEST_LAYER-Orthophoto/s3returned 400 for a Layer whose Caches arefileandredis.Every "the addressed Cache is not there" case now answers alike — Layer absent, no Cache under the resolved name, and a Cache Type that cannot be confirmed. A Cache Type outside the enum still answers 400, from request validation, before the manager runs, so the endpoint keeps both statuses and the openapi document needs no change.
The unreadable entry follows the mismatch to 404 rather than keeping a 400 of its own: telling "the Cache Type is confirmably something else" from "the Cache Type could not be confirmed" would reintroduce the malformedness concept this design deliberately does not carry, and a 400 would blame the caller for the server's own corrupt configuration. The operator's signal stays the warn log carrying the offending entry.
A consumer branching on 400 for this endpoint will see 404 instead. Risk is low — the premise of this ticket is that consumers were pushed to the deprecated
GET /layer/{name}— but it is a real contract change and should be called out at release.Nine cases now covered at the HTTP seam, each asserting
toSatisfyApiSpecalongside the body: s3 whole Cache; redis resolving to the-redisCache and legitimately lackingupscale_tiles; unmodelled mapproxy options surviving verbatim at both levels; an entry that is not an object → 400; an entry with no Cache Source → 400; a genuine Cache Type mismatch → 400; an unknown Layer → 404; a Layer with no Cache under the resolved name → 404; acacheTypeoutside the enum → 400.The deprecated
GET /layer/{name}is not touched, and neither is the Layer-name validation it shares with the insert and update flows.Geopackage fixture and coverage follow in a second PR on this ticket.