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

OAUTH2-135 OAuth2ApplicationScopeAliases scopeAliases column is too short for most usages #59368

Closed
wants to merge 12 commits into from

Conversation

topolik
Copy link

@topolik topolik commented May 31, 2018

@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering "ci:test:sf" and "ci:test:relevant" for this pull to run Source Formatter and relevant tests.

Comment "ci:test" to run the full PR Tester for this pull.

@topolik topolik changed the title pull-request-525 OAUTH2-135 OAuth2ApplicationScopeAliases scopeAliases column is too short for most usages May 31, 2018
@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 2 minutes 313 ms

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 82d5f0c7f86cf643ac63b509f9e72e1f9c9a48f9

Sender Branch:

Branch Name: pull-request-525
Branch GIT ID: ab2b54b4540e2d815b69035723768ef97f509e87

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@brianchandotcom
Copy link
Owner

Merged. Thank you.
View total diff: 0f3509a...abc7dd0

@brianchandotcom
Copy link
Owner

@topolik @csierra this is a very interesting fix/hack. Good job.

Can you send a follow up pull? I don't like using the method "hashCode" to generate the hash.

I'd rather you add a new method, called "predictableHashCode", or "deterministicHashCode" or, "oAuthHashCode", or something else. Why? I think hashCode is JVM dependent. And even if we override it in BaseModel, that implementation may one day change.

And if it changed, our DB logic is now useless.

We need an implementation of hash code that we control, that no one else can change, that is always predictable.

Please send a follow up pull or we'll end up losing data when a server switches JVMs.

@csierra
Copy link

csierra commented May 31, 2018

Hi Brian,
String.hashcode() is part of String API and is stable across JVM's and versions. It is true that we might risk using hashcode unknowingly for other objects and then it would vary.

Where would you suggest us to add the new function? in the very oauth2-provider-service module?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants