Skip to content

Introduce Interfaces to Final Classes#581

Merged
evansims merged 3 commits intoauth0:mainfrom
jobcloud:feat/update-version-8-add-interfaces
Oct 27, 2021
Merged

Introduce Interfaces to Final Classes#581
evansims merged 3 commits intoauth0:mainfrom
jobcloud:feat/update-version-8-add-interfaces

Conversation

@komando82
Copy link
Copy Markdown
Contributor

@komando82 komando82 commented Oct 21, 2021

This Pull Request intent is to add Interfaces to Final Classes. It is not a good pattern for public SDK to have final Classes without implemented Interfaces. I didn't add all Interfaces before check with you, since that requires more time for me to invest.

Changes

Things changed:

  1. Interfaces added to existing Contract folder: Auth0Interface.php, TokenInterface.php, Token\ValidatorInterface.php, Api\AuthenticationInterface.php, Api\ManagementInterface.php and all Api\Management/{Class}Interface.php .
  2. Original Final Classes are changed to implement this Interfaces.
  3. Management.php class magic method __call is removed and concrete methods added.
  4. ManagementTest.php changed

Benefits for users (developers in my case) are big ones from my standpoint:

  1. Dependency injection : we can now depend upon interfaces, instead of concrete classes
  2. Writing tests would be much easier

Introducing interfaces will not break any existing functionality since implementation is not changed.

References

This is referring to closed Issue : #575

Resolves #

Contributor Checklist

@komando82 komando82 requested a review from a team as a code owner October 21, 2021 09:45
@komando82 komando82 changed the title Introduce Interfaces to Classes Introduce Interfaces to Final Classes Oct 21, 2021
@evansims
Copy link
Copy Markdown
Contributor

Hey @komando82, thanks for your hard work on this! This was the same approach I had been working on, but you beat me to the punch at getting this far. Let me give this a thorough look over and get back to you, but I like what I'm seeing. 👍

@evansims evansims self-assigned this Oct 22, 2021
Copy link
Copy Markdown
Contributor

@evansims evansims left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @komando82 👋 Thanks for your patience while I reviewed this. Everything looks solid to me, and I appreciate the contribution!

@evansims evansims merged commit 9995020 into auth0:main Oct 27, 2021
@healerz
Copy link
Copy Markdown

healerz commented Nov 1, 2021

Hi @evansims any chance of releasing this soon? 🙂 (And thx for accepting our PR)

@evansims evansims added this to the 8.0.3 milestone Nov 1, 2021
@evansims
Copy link
Copy Markdown
Contributor

evansims commented Nov 2, 2021

Hi @evansims any chance of releasing this soon? 🙂 (And thx for accepting our PR)

@healerz No worries! Just cut a new release containing these changes. Thanks for your contributions!

@healerz
Copy link
Copy Markdown

healerz commented Nov 2, 2021

@evansims thx a lot

@shadowhand
Copy link
Copy Markdown
Contributor

@komando82 this is amazing work, thank you!

@github-actions
Copy link
Copy Markdown
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants