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

Move candidate io.jsonwebtoken.lang.* classes to the impl module #525

Open
lhazlewood opened this issue Oct 24, 2019 · 3 comments
Open

Move candidate io.jsonwebtoken.lang.* classes to the impl module #525

lhazlewood opened this issue Oct 24, 2019 · 3 comments
Milestone

Comments

@lhazlewood
Copy link
Contributor

lhazlewood commented Oct 24, 2019

Due to old implementations (before JJWT was modularized), the io.jsonwebtoken.lang package in the api module contains a lot of classes and methods that aren't needed by JJWT users.

For 1.0, as many of these classes and methods as possible should be moved to the impl module since they're almost exclusively used by JJWT implementation classes.

Note that anything used by any of the extensions or in the api module itself would need to remain in the api module. But this number should be relatively minor.

If this opens a can of worms, we can leave these classes alone, but per the open-closed principle, it'd be best to not expose anything that's not absolutely critical for using the api.

Setting for the 1.0 milestone as semver dictates this can't be done without incrementing the major version number.

@lhazlewood lhazlewood added this to the 1.0 milestone Oct 24, 2019
@lhazlewood lhazlewood changed the title Move all io.jsonwebtoken.lang.* Classes to the impl module as possible Move candidate io.jsonwebtoken.lang.* classes to the impl module Oct 24, 2019
@dogeared
Copy link
Contributor

dogeared commented Oct 26, 2019

As a first pass, I wanted to see what the impact would be of moving the lang package into impl. After getting compile time errors due to usages within the api modules, I started moving things back. Turns out, most everything was needed just to get back to compiling properly:

api:
image

impl:
image

How about we make a jjwt-lang module, move everything in there, and make both jjwt-api and jjwt-impl depend on it? @lhazlewood

@lhazlewood
Copy link
Contributor Author

Bummer. So the goal was to hide whatever we could from projects using JJWT that wasn't necessary in creating and parsing JWTs.

If we have a jjwt-lang module that jjwt-api depends on, when a user declares compile scope for jjwt-api, they'll still have compile scope for jjwt-lang as well, defeating the purpose of multiple modules.

That said, for 1.0, there are a lot of internal implementation details w.r.t. to making SignatureAlgorithm an interface (#493) that will result in us no longer needing many of the lang.* classes in the api module.

Along those same lines, the same is true of the Encoders and Decoders (Base64/Base64URL implementations) and probably a few others.

Ideally, for 1.0, by the time we're done with everything related to api vs impl migration, the only things that should be in the api module would be interfaces and exceptions and maybe a couple of static classes/helper methods and that's it. Everything else should be pushed down to the impl module so that only the things needed for JJWT usage should be exposed and all other implementation details remain hidden and out of compile scope.

@lhazlewood
Copy link
Contributor Author

This is probably going to be related to and impact #519

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

No branches or pull requests

2 participants