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

Add an errorprone check and typed annotation for Javax -> Jakarta #2366

Merged
merged 13 commits into from
Aug 25, 2022

Conversation

mglazer
Copy link
Contributor

@mglazer mglazer commented Aug 24, 2022

Before this PR

Legacy javax.ws.rs types could be submitted to methods which only accept jakarta.ws.rs types at runtime.

After this PR

==COMMIT_MSG==
Add an errorprone check and typed annotation for Javax -> Jakarta

There is a certain class of very problematic cases whereby if you have
a method such as the following:

myJerseyResource.register(/* this is of type Object */ object);

Then if you supply a resource which includes any javax.ws.rs
annotations on it, then those will not be registered if your Jersey
version is 3.x or later (and you'll only find this out at runtime).

The opposite is also true if you try to supply resources annotated
with jakarta.ws.rs to Jersey 2.x.

To address this, this commit attempts to add an errorprone check
which lets implementors add an annotation @ForbidJavax to methods
which have been knowingly migrated to Jakarta EE9 and cannot
accept legacy javax types.
==COMMIT_MSG==

Possible downsides?

There is a certain class of very problematic cases whereby if you have
a method such as the following:

```
myJerseyResource.register(/* this is of type Object */ object);
```

Then if you supply a resource which includes any `javax.ws.rs`
annotations on it, then those will not be registered if your Jersey
version is 3.x or later (and you'll only find this out at runtime).

The opposite is also true if you try to supply resources annotated
with `jakarta.ws.rs` to Jersey 2.x.

To address this, this commit attempts to add an errorprone check
which lets implementors add an annotation `@ForbidJavax` to methods
which have been knowingly migrated to Jakarta EE9 and cannot
accept legacy javax types.
@changelog-app
Copy link

changelog-app bot commented Aug 24, 2022

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Add an errorprone check and typed annotation for Javax -> Jakarta

There is a certain class of very problematic cases whereby if you have
a method such as the following:

myJerseyResource.register(/* this is of type Object */ object);

Then if you supply a resource which includes any javax.ws.rs
annotations on it, then those will not be registered if your Jersey
version is 3.x or later (and you'll only find this out at runtime).

The opposite is also true if you try to supply resources annotated
with jakarta.ws.rs to Jersey 2.x.

To address this, this commit attempts to add an errorprone check
which lets implementors add an annotation @ForbidJavax to methods
which have been knowingly migrated to Jakarta EE9 and cannot
accept legacy javax types.

Check the box to generate changelog(s)

  • Generate changelog entry

svc-changelog and others added 4 commits August 24, 2022 03:18
…seline into mglazer/require-jaxrs

* 'mglazer/require-jaxrs' of github.com:palantir/gradle-baseline:
  Add generated changelog entries
mglazer pushed a commit to palantir/conjure-java-runtime that referenced this pull request Aug 24, 2022
@mglazer
Copy link
Contributor Author

mglazer commented Aug 24, 2022

When the CJR change goes through it should also resolve the error that I'm currently getting where I'm not publishing the annotations correctly somehow.

bulldozer-bot bot pushed a commit to palantir/conjure-java-runtime that referenced this pull request Aug 24, 2022
@mglazer
Copy link
Contributor Author

mglazer commented Aug 24, 2022

Added a couple of more tests, one to handle the case where you're supplying a resource which extends from a JaxRS annotated interface, and another to assert that this errorprone check is not perfect, and can't handle the case where there is no type information at all on the ingress types.

Copy link
Contributor

@carterkozak carterkozak left a comment

Choose a reason for hiding this comment

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

Thanks!

@mglazer
Copy link
Contributor Author

mglazer commented Aug 25, 2022

👍

bulldozer-bot bot pushed a commit to palantir/witchcraft-api that referenced this pull request Sep 2, 2022
###### _excavator_ is a bot for automating changes across repositories.

Changes produced by the roomba/latest-baseline-oss check.

# Release Notes
## 4.154.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Improvement | The JUnits reports plugin is no longer applied by default. Test reports now use the standard output locations from Gradle conventions. | palantir/gradle-baseline#2355 |


## 4.155.0
_Automated release, no documented user facing changes_

## 4.156.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Fix | Fix BaselineJavaVersion checkstyle configuration on gradle < 7.5 | palantir/gradle-baseline#2360 |


## 4.157.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Improvement | Make task initialization lazier in the `junit-reports` plugin. | palantir/gradle-baseline#2364 |


## 4.158.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Fix | Make the `checkUnusedDependencies` tasks added by `baseline-exact-dependencies` compatible with Gradle's configure-on-demand feature. | palantir/gradle-baseline#2363 |


## 4.159.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Improvement | Add an errorprone check and typed annotation for Javax -> Jakarta<br><br>There is a certain class of very problematic cases whereby if you have<br>a method such as the following:<br><br>```<br>myJerseyResource.register(/* this is of type Object */ object);<br>```<br><br>Then if you supply a resource which includes any `javax.ws.rs`<br>annotations on it, then those will not be registered if your Jersey<br>version is 3.x or later (and you'll only find this out at runtime).<br><br>The opposite is also true if you try to supply resources annotated<br>with `jakarta.ws.rs` to Jersey 2.x.<br><br>To address this, this commit attempts to add an errorprone check<br>which lets implementors add an annotation `@ForbidJavax` to methods<br>which have been knowingly migrated to Jakarta EE9 and cannot<br>accept legacy javax types. | palantir/gradle-baseline#2366 |


## 4.160.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Fix | Workaround to IDEA-301084 | palantir/gradle-baseline#2368 |


## 4.161.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Fix | Reverts a change introduced to baseline-java-version 4.160.0, which was causing failures on multi-project builds. | palantir/gradle-baseline#2369 |



To enable or disable this check, please contact the maintainers of Excavator.
This was referenced Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants