X Tutup
Skip to content

Refactoring #565

Closed
iarshtejay wants to merge 10 commits intoauth0:masterfrom
iarshtejay:master
Closed

Refactoring #565
iarshtejay wants to merge 10 commits intoauth0:masterfrom
iarshtejay:master

Conversation

@iarshtejay
Copy link
Copy Markdown

@iarshtejay iarshtejay commented Mar 25, 2022

Changes

Hi! Did a refactoring of some of the code. Please have a look and accept my pull request. If you'd like some more changes, I am happy to incorporate them. This is my first contribution to open source. Appreciate your feedback.

An overview of the changes I did:

  • Rename a variable "parser" -> "jwtParser" in JWT.java
  • Extracted a method from a long code block, and created a new method named "isAValidClaimsObject()"
  • Introduced an interface "ExpectedClaimType" and implemented "Factory Pattern" to replace the switch case in JWTVerifier.java
  • Added new abstractions for the new switch case classes I introduced

References

Testing

No new functionality was introduced. Only refactoring, so no new test cases were introduced. All the existing test cases are passing.

Checklist

iarshtejay added 10 commits March 25, 2022 11:29
@iarshtejay
Rename variable "parser" -> "jwtParser"
2654046
@iarshtejay
Extract method "isAValidClaimsObject()" from code block in "assertVal…
d8769c5 
…idClaim()"
@iarshtejay
Switch Case Polymorphism: Create an interface
94931a8
@iarshtejay
Switch Case Polymorphism: Extend the interface // Possible errors nee…
8ba2316 
…d to be resolved
@iarshtejay
Move method: Move methods from JWTVerifier to classes implemented fro…
f92ca84 
…m interface
@iarshtejay
Switch Case Polymorphism: Implement a factory class
9519e59
@iarshtejay
Resolve method compatibility errors after "Move method refactoring"
b8d56bc
@iarshtejay
Resolve "Switch Case Polymorphism" with "Interface" and "Abstract Fac…
7a34a2c 
…tory"
@iarshtejay
Introduce a superclass and pull up methods from sub-classes to resolv…
f93cb6e 
…e unutilized abstraction
@iarshtejay
Introduce a superclass and pull up methods from sub-classes to resolv…
67352d2 
…e unutilized abstraction
@iarshtejay iarshtejay requested a review from a team as a code owner March 25, 2022 18:32
@poovamraj
Copy link
Copy Markdown
Contributor

poovamraj commented Mar 28, 2022

Hi @iarshtejay! We really value the effort you have put in for us. Unfortunately, as you can see in our branch v4-dev, We are working on a new major where many of the code you have refactored here is completely revamped. Since merging this not be suitable right now (There will be a lot of conflicts), we might have to close this PR. We will welcome any feedback you can provide to us on v4-dev branch, and we can guide you on creating a PR for them as well.

Hope this doesn't discourage you from contributing to open-source which is very valuable work you can do for the community. Looking forward to more contributions from you.

@poovamraj poovamraj closed this Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@iarshtejay @poovamraj
X Tutup