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

set library id lowercase make more compatible #3600

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

unl0ck
Copy link

@unl0ck unl0ck commented Nov 10, 2024

iOS swift will send with openapi generated client lib the UUID to string not lowercased and so it is not possible the find the correct library

@nichwall
Copy link
Contributor

nichwall commented Nov 10, 2024

Huh, I didn't realize that parsers are required to accept uppercase UUID, but software is required to generate UUID lowercase. Looks like it's a thing of Apple not conforming to the spec, shortest summary here: https://stackoverflow.com/a/48333432/3422368

Anyway, I think we want to modify the Sequelize models to just handle the lowercasing of the UUID field automatically instead of manually changing everywhere a UUID is used, since this will affect every model, not just libraries.

@unl0ck
Copy link
Author

unl0ck commented Nov 10, 2024

So I should change it everywhere ?

@unl0ck
Copy link
Author

unl0ck commented Nov 12, 2024

@nichwall soll ich die Models alle anpassen ? Beziehungsweise darf ich die anpassen ?

@nichwall
Copy link
Contributor

I'm not sure. I haven't had time to look into it more. Some of my reading made it sound like Postgres uses uppercase UUID, but they use a 128-bit binary representation instead of the string representation for SQLite. We are not currently supporting other database backends, but that is planned eventually and why we are using Sequelize.

If you can do some more research to show that using lowercase everywhere is better than just updating the model to do it automatically, that would be good.

@nichwall
Copy link
Contributor

To clarify, I think changing all of the middleware to convert to lowercase is fine, but we want to make sure this won't cause a different problem somewhere.

It may not be simple to change the model to handle the lowercase logic, and there might be other server logic which expects it to be a lowercase UUID, in which case we do want to convert it to lowercase like you've done here.

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