JdbcOAuth2AuthorizationService supports clob and text datatype for token columns

[ghost]

Use CLOB instead of BLOB. With CLOB type the underlying schema can store varchars and clobs as well (if this is supported by the database)

Closes gh-480

[jgrandja]

Thanks for the PR @ovidiupopa91.

The changes do work for Postgres but doesn't work anymore for oauth2_authorization tables that have already defined blob. The changes will force applications to update the schema.

We'll need to come up with a backwards compatible strategy. For now, I'm going to push this to the 0.2.2 release to allow for more time.

[jgrandja]

Hi @ovidiupopa91 and Happy New year! I hope you had a relaxing break :)

Just touching base to see how the updates are coming along?

[ghost]

hi @jgrandja . Happy New Year to you too! Actually, this is my first day back. I will check today if anything else needs to be done (as far as I can remember the implementation is done, and all tests are passing except one or two).

[jgrandja]

Welcome back @ovidiupopa91 !

Just a heads up that more changes will be required. My suggestion to change the column types to varchar will not work. Please see gh-550 and gh-557 for additional details.

We'll need to ensure that data items that have variable length data (e.g. access_token_value, access_token_metadata, attributes, etc.) are defined with a standard sql type that will allow for variable length without restricting size, e.g clob or text.

Can you look into which standard sql type would work for at least mysql and postgres and we can decide from there. Thanks!

[ghost]

hi @jgrandja . Based on postgresql datatypes and mysql datatypes text sql data type should work for mysql and postgresql.

I will change the schema to use text instead of clob for all columns that might have variable length data.

[jgrandja]

Thanks for looking into this @ovidiupopa91. Although text is not a standard sql type, it seems to be used by a few well-known vendors so let's go with it.

I will change the schema to use text instead of clob for all columns that might have variable length data.

Only change the columns that have blob defined to text in this PR. For the other columns, e.g. access_token_metadata, attributes, etc. can you submit a new PR?

[ghost]

hi @jgrandja . I've pushed the updated implementation. Do you think it's still necessary to have a different file where the token columns are defined as text?

[jgrandja]

Thanks for the updates @ovidiupopa91. Please see review comments.

[jgrandja]

@ovidiupopa91 Will you be able to submit the updates by tomorrow? I'd like to get this into the release this Wed. If you don't have time, no worries, I can submit it. Please let me know.

[ghost]

@ovidiupopa91 Will you be able to submit the updates by tomorrow? I'd like to get this into the release this Wed. If you don't have time, no worries, I can submit it. Please let me know.

hi @jgrandja . I've just pushed the changes. Sorry for the delay but I was quite backlogged in the last couple of weeks.
FYI, I've decided to keep oauth2-authorization-schema-postgres.sql because HSQL is not supporting text data type. The alternative would be to create the type TEXT before the tests are running.

[jgrandja]

Thanks @ovidiupopa91 ! I'll look at this tomorrow and get it merged.

[jgrandja]

Thanks for all the updates @ovidiupopa91.

FYI, I added a polish commit and just merged this into main.

[ghost]

Great @jgrandja . Let me know when you have anything else that I can take a look at. Thanks!

[jgrandja]

For sure @ovidiupopa91. I'll get back to you soon.

[jgrandja]

Hi @ovidiupopa91. Would you be interested in gh-417 or gh-616?

[ghost]

Hi @ovidiupopa91. Would you be interested in gh-417 or gh-616?

Hi @jgrandja . I can take a look and fix both of them if that's ok with you.

[jgrandja]

Sounds good @ovidiupopa91 ! Please respond on both issues and I'll assign to you.