Conversation
gunnarvelle
force-pushed
the
content-type-filter
branch
from
9a28313 to
1b19916
Compare
gunnarvelle
force-pushed
the
content-type-filter
branch
from
1b19916 to
c6f8f40
Compare
| case object Png extends ImageContentType("image/png", List(".png")) | ||
| case object PngXToken extends ImageContentType("image/x-png", List(".png")) | ||
| case object Gif extends ImageContentType("image/gif", List(".gif")) | ||
| case object Webp extends ImageContentType("image/webp", List(".webp")) |
There was a problem hiding this comment.
Vi har ikkje skrudd på opplasting av disse, men siden vi genererer varianter på formatet går det kanskje bra? Eller må det gjøres endringer der om opplasting av slike skal gå an? Skal eg fjerne denne @amatho ?
There was a problem hiding this comment.
Kunne kanskje også hatt et processable-flagg på enumen for å unngå to enumer som gjør det samme.
There was a problem hiding this comment.
Vi har ikkje skrudd på opplasting av disse, men siden vi genererer varianter på formatet går det kanskje bra? Eller må det gjøres endringer der om opplasting av slike skal gå an? Skal eg fjerne denne @amatho ?
Scrimage har bare PNG, GIF, JPEG og WebP definert som formater i koden deres, så tipper det er der det går galt i så fall
There was a problem hiding this comment.
Kunne kanskje også hatt et processable-flagg på enumen for å unngå to enumer som gjør det samme.
Litt usikker, fordi ProcessableImageFormat kommer av et Scrimage format, hvor vi vet det ekte formatet på bildet. Mens Content-Type kan man egentlig ikke stole på at stemmer (vi gjør det likevel for SVGer da)
There was a problem hiding this comment.
Men betyr det at opplasting av Bmp vil feile siden Scrimage ikkje takler det? Eller blir det berre ikkje lasta opp varianter?
gunnarvelle
force-pushed
the
content-type-filter
branch
from
48c06fe to
0bc1a85
Compare
| s3Object.stream, | ||
| s3Object.key, | ||
| s3Object.contentLength, | ||
| ImageContentType.valueOf(s3Object.contentType).getOrElse(ImageContentType.Binary), |
There was a problem hiding this comment.
Tror egentlig det er bedre å feile hvis vi ikke har noen kjent Content-Type, så sørger vi for at vi ikke ender opp med bilder vi ikke støtter. Så kan vi heller fjerne Binary fra ImageContentType, slik at den bare inneholder typer vi ønsker å støtte
image-api/src/main/scala/no/ndla/imageapi/service/ImageStorageService.scala
Outdated
Show resolved
Hide resolved
| ) | ||
| ) | ||
| case Some(contentType) => | ||
| // Verify that the file extension matches the content type |
There was a problem hiding this comment.
Litt åpent spørsmål: trenger vi egentlig å bry oss om filendelse? Det har jo egentlig ikke noe å si, i hvert fall ikke når vi genererer filnavn. Kunne jo f.eks. brukt Content-Type til å sette filendelse på det genererte filnavnet
There was a problem hiding this comment.
Tenker uansett det er greit at filnavn og content-type henger sammen.
gunnarvelle
force-pushed
the
content-type-filter
branch
from
62a7ff9 to
9ac09da
Compare
2 participants
Lar deg filtrere bilder på contentype