Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding metadata to application CRUD group #705

Merged
merged 1 commit into from
Nov 17, 2022
Merged

Conversation

svix-gabriel
Copy link
Contributor

@svix-gabriel svix-gabriel commented Nov 16, 2022

This adds the metadata field to the application CRUD group.

Motivation

The metadata field is just a JSON dictionary with string only values. This field was added to our public API, but never to our open source solution. This PR corrects that.

Solution

We add the field to our /apps CRUD group. Metadata itself is a separate joined table, instead of a column on the apps field, because metadata can be quite large (up to 4096 bytes).

Because it's a separate model, the sea_orm queries end up being pretty tedious. I tried to keep things efficient and query for both records in a single round trip where possible, but in some places we needed two roundtrips.

A side effect is that the endpoints were starting to get fairly complicated, so I tried to offload a lot of the logic to the relevant db::models where appropriate to offset this.

Finally, the metadata field is entirely optional, but we default to rendering {} if it's unspecified. Additionally, we don't create empty metadata records. It ended up being much simpler to just pass around an applicationmetadata::[Model|ActiveModel] around, and intelligently upsert only IFF the underlying data wasn't empty.

@svix-gabriel svix-gabriel force-pushed the gabriel/add-metadata branch 3 times, most recently from 7393b91 to 558b2ab Compare November 16, 2022 18:57
@svix-gabriel svix-gabriel marked this pull request as ready for review November 16, 2022 19:12
@svix-gabriel svix-gabriel force-pushed the gabriel/add-metadata branch 6 times, most recently from f836f4c to 0678eff Compare November 17, 2022 14:29
@svix-gabriel svix-gabriel merged commit f4961a5 into main Nov 17, 2022
@svix-gabriel svix-gabriel deleted the gabriel/add-metadata branch November 17, 2022 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants