DDMAL / CantusDB

A new site for Cantus Database running under Django.
https://cantusdatabase.org
MIT License
5 stars 6 forks source link

Create JSON APIs for all models in the database #564

Open jacobdgm opened 1 year ago

jacobdgm commented 1 year ago

In doing some research for #563, I discovered a situation where Drupal and Django handle their databases differently, and this will cause a problem for our json-node API.

OldCantus appears to guarantee that all objects, whether they are sequences, chants, sources, etc., are all assigned a unique ID when they are created. In NewCantus, however, all sources have unique IDs relative to other sources, but there's nothing stopping a new source from being assigned same ID as an existing chant, say.

Currently, in our json_node_export view (code), we naively assume a given ID will match only a single object in the database - we check if it matches an existing chant, then sequence, then source. As soon as we find a match, we return information on it (it's worth noting that OldCantus's version of the API can also return information on indexers and news articles, which we don't currently do).

This will work fine up until people start adding new objects to the database in NewCantus. As people begin creating new sources, chants, etc., these ID duplications will become more and more common.

I can think of a few possible approaches to resolving this:

jacobdgm commented 1 year ago

@dchiller and I just realized that CU needs to get at provenances' names, so while resolving this issue, we can also set up a /provenance/123456/json/ path to facilitate this (the current API doesn't support returning information on Provenances at all).

jacobdgm commented 1 year ago

Our current solution for preventing /node/ URL collisions solves this issue.

I still like the idea of changing the paths of the URLs to function better as URIs. If we choose to do this, we can probably keep both URL paths available, or redirect requests to the new URL, similar to what we do for /node/ URLs currently. Marking this as an enhancement.

jacobdgm commented 9 months ago

Increasing the priority to medium because of https://github.com/DDMAL/cantus/issues/808#issuecomment-1883989468.

jacobdgm commented 9 months ago

UPDATE: What happened here was I was trying to be too fancy in the tests. By simply examining BaseModel and Provenance/Notation and manually writing out a list of expected keys, the test works as expected and is clearer to boot. And we get our FKs, in case they're needed. Feel free to ignore the text below!

ORIGINAL COMMENT: @lucasmarchd01 @dchiller - wondering if you have a sense of this:

I started writing a Chant Export API, and realized the approach we're using with our JSON Node Export view (taking a queryset of a single item, using queryset.values(), and returning the results) only returns fields that aren't FK fields (i.e., it excludes all authoring/editing information, as well as Source/Office/Genre/etc. information.) Do we think this is fine?

Also, this approach includes all the non-FK fields, including json_info, search_vector, both of c_sequence and s_sequence, and so on. Do we think this is fine, or should we exclude some extraneous fields?

For the time being, I'll set aside the Chant API, and focus on implementing the Notation and Provenance APIs, which should not be affected by the two issues outlined in the paragraphs above.

dchiller commented 9 months ago

Also, this approach includes all the non-FK fields, including json_info, search_vector, both of c_sequence and s_sequence, and so on. Do we think this is fine, or should we exclude some extraneous fields?

I could see a case for excluding these extraneous fields for clarity...for example, json_info could contain out-of-date information and we wouldn't want someone to inadvertently use it... I guess the main thing would be to make sure documentation stays up to date.

RE: FK fields

This may be what you are saying, but my impression was that it included these fields, but just as keys/ID's. So, for example the Source API returns the ID of the provenance, but not its string name. Is that right?

jacobdgm commented 9 months ago

This may be what you are saying, but my impression was that it included these fields, but just as keys/ID's. So, for example the Source API returns the ID of the provenance, but not its string name. Is that right?

I'll have to check again to be sure, but I'm pretty sure that with the current approach, we'll get "feast_id": some_int instead of "feast": something

for example, json_info could contain out-of-date information and we wouldn't want someone to inadvertently use it... I guess the main thing would be to make sure documentation stays up to date.

Okay - I agree. I'll remove these from the APIs I just implemented, and make sure they're also removed in the rest of the APIs.

dchiller commented 9 months ago

I'll have to check again to be sure, but I'm pretty sure that with the current approach, we'll get "feast_id": some_int instead of "feast": something

Yes, this is what I'm seeing, at least in the case of the provenance field in the response of json-node for a source.