fix: add asset upload medium test

[jrasm91]

• Add medium test for asset upload service method
• Make metadata optional in open api, to match the runtime checks

Follow up to #25143

[timonrieger]

perfect, i just had issue with this being marked required in the spec 😄

[jrasm91]

@timonrieger what use case are you using metadata for?

[timonrieger]

btw. this duplicates the guard set in https://github.com/immich-app/immich/pull/25143/files#diff-c80282da76df83dbb605192d511162ba1a48cca7c9726828dd1d9297c3a438d2R436

[jrasm91]

True but this repo method might be used in other places in the future. The typing for the method itself doesn't preclude sending empty values but the database will throw an error so it's seems like an appropriate place to include it.

You could revert the length change but a check is still needed either way because it can be undefined so let's just leave it as is.

[timonrieger]

@timonrieger what use case are you using metadata for?

i am building https://github.com/timonrieger/immich-python-client, and currently dealing with the upload endpoint to make it accessible. It's auto generated from the openapi spec, so since metadata is marked as required i faced issues. I manually patched the following error, but server-side is the correct place obv.

passing metadata=[] gave me this error, removing it entirely works

{"message":["metadata must be valid JSON"],"error":"Bad Request","statusCode":400,"correlationId":"8gtc2p8v"}

The server does not handle empty values correctly. to reproduce could you test this on the latest main build (i don't have a machine available rn)

printf '\xFF\xD8\xFF\xD9' > /tmp/minimal.jpg
curl -X POST "http://localhost:2283/api/assets" \
  -H "x-api-key: api-key" \
  -F "assetData=@/tmp/minimal.jpg" \
  -F "deviceAssetId=test-fails" \
  -F "deviceId=test-client" \
  -F "fileCreatedAt=2024-01-15T10:30:00.000Z" \
  -F "fileModifiedAt=2024-01-15T10:30:00.000Z" \
  -F 'metadata=[]'

Note

Your PR should fix this insofar as the client would know that this is an optional field now and can omit the metadata field entirly. for the sake of completeness, commenting here nevertheless

will file a PR nevertheless

[jrasm91]

This is bringing up some old memories. I think that because this endpoint uses form data to accept the upload file, you have to send the other data as form fields as well. And, I don't think it's actually possible to natively send a list of json objects in a single form field. You would need to manually stringify it or something.

[timonrieger]

This is bringing up some old memories. I think that because this endpoint uses form data to accept the upload file, you have to send the other data as form fields as well. And, I don't think it's actually possible to natively send a list of json objects in a single form field. You would need to manually stringify it or something.

yes, form data is always stringified. Sending patch

[jrasm91]

Oh, I already fixed this in #21594. If you stringify the metadata before sending it, it should just work.

[timonrieger]

Oh, I already fixed this in #21594. If you stringify the metadata before sending it, it should just work.

just figured, my patch is unnecessary, handling weird cases assuming the client is dumb 🤣 . stopping here now…thanks for the fix

[timonrieger]

btw. i recognized that server-side optional properties are often typed with !. Why do you do that?
e.g. https://github.com/immich-app/immich/blob/main/server/src/dtos/time-bucket.dto.ts#L195:L210

runtime tells me that these field sometimes don't exists (as expected from optional)

# Get time bucket assets with coordinates (to check if latitude/longitude are present)
curl -X GET "http://localhost:2283/api/timeline/bucket?timeBucket=2024-01-01&withCoordinates=true" \
                          -H "x-api-key: key" \
                          -H "Accept: application/json"
# contains coordinates data
{..., "latitude": [], "longitude": []}


curl -X GET "http://localhost:2283/api/timeline/bucket?timeBucket=2024-01-01" \
                          -H "x-api-key: key" \
                          -H "Accept: application/json"
# no coordinates data
{...}

Should these be changed to latitude?: number[] and longitude?: number[] to match the spec and runtime, or is there a reason to keep !?
Low risk as of know since no server code uses this DTO internally. But would make types match runtime behavior.

[jrasm91]

I think those might be dynamic based on that query parameter. Time buckets specifically are a very specialized thing that's only meant to be used with immich-web so I wouldn't rely on them not changing in the future.