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

Arc - improve injection point transformation API #40841

Merged
merged 3 commits into from
Jun 6, 2024

Conversation

manovotn
Copy link
Contributor

Fixes #38856

First commit changes Arc's injection point transformation API. ATM the injection point AnnotationTarget is either FIELD or METHOD, making it difficult to alter any annotation for single or multiple METHOD_PARAMETER targets underneath.
Changes I introduce allow to operate directly on method parameters but in order to stay backward compatible, this requires new methods. The old ones are still in place, but I marked them as @Deprecated(forRemoval=true)

Second commit changes all internal usages of the newly deprecated methods to their new variants.

@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/context-propagation area/grpc gRPC area/qute The template engine area/reactive-messaging area/smallrye labels May 25, 2024
Copy link

quarkus-bot bot commented May 25, 2024

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

@manovotn manovotn force-pushed the issue38856 branch 3 times, most recently from 0f31863 to 06cdf54 Compare May 25, 2024 21:13

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@Ladicek
Copy link
Contributor

Ladicek commented Jun 4, 2024

LGTM, but I agree with @mkouba's comments.

This comment has been minimized.

@manovotn manovotn force-pushed the issue38856 branch 2 times, most recently from 2a3a41f to 8213641 Compare June 4, 2024 12:49

This comment has been minimized.

@manovotn
Copy link
Contributor Author

manovotn commented Jun 5, 2024

I've changed the third commit and TransformedAnnotationsBuildItem now handles method param. annotation targets with filtering. Since you couldn't query method params even before, we don't need to deprecate those methods and can instead add special case handling directly into them. This allows us to mask the Jandex behavior for users which can use the API regardless of the annotation target they have - which is visible in the change to WiringHelper that now has much nicer/simpler code.

Let's see what CI has to say...

Copy link

quarkus-bot bot commented Jun 5, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 1b3b935.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-reactive-messaging-kafka/deployment

io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase.sseStream - History

  • Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase Expecting size of: [] to be greater than or equal to 2 but was 0 within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase 
Expecting size of:
  []
to be greater than or equal to 2 but was 0 within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

@manovotn manovotn requested review from mkouba and gsmet June 6, 2024 09:43
@manovotn
Copy link
Contributor Author

manovotn commented Jun 6, 2024

@mkouba @gsmet neither of you has further comments, we should be good to go with this PR

@gsmet
Copy link
Member

gsmet commented Jun 6, 2024

Mine was just a question, I will let @mkouba approve and merge this one.

@mkouba mkouba merged commit 3fdff36 into quarkusio:main Jun 6, 2024
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.12 - main milestone Jun 6, 2024
@manovotn manovotn deleted the issue38856 branch June 6, 2024 14:29
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.

Arc - improve injection point transformation API
4 participants