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 fixes for spec compatibility #30509

Merged
merged 5 commits into from
Jan 25, 2023
Merged

ArC fixes for spec compatibility #30509

merged 5 commits into from
Jan 25, 2023

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Jan 20, 2023

Related to #28558

  • scan package-info.class for @Vetoed
    • including a small fix to ArcTestContainer to also index package-info.class of all classes in the test bean archive
  • fix dynamic lookup of array types
    • applies when required array type during dynamic lookup is represented as a Class, while the bean array types are instantiated as GenericArrayType
    • this scenario was already handled in the Weld codebase, so this is just a direct copy
  • fix BeanManager.getReference() implementation
    • was missing one check, which is copied directly from Weld again
  • fix treatment of primitive wrapper type producers that produce null
    • the zero value must be injected into primitive injection points
  • fix treatment of injection points with @Named without value
    • field injection points with @Named without value should use @Named(fieldName) as the qualifier
    • other such injection points are a definition error

To review, it's probably best to look at each commit in isolation. Tests are added, too.

@Ladicek Ladicek requested review from mkouba and manovotn January 20, 2023 14:28
@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Jan 20, 2023
@quarkus-bot

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Jan 23, 2023

The failures are not related.

Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

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

Cool! Thanks!

Copy link
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

The additions in this commit are taken directly from the Weld codebase.
They apply in the situation when the required type during dynamic lookup
is represented using a `Class` object, while the bean type is represented
using `GenericArrayType`.
The additions in this commit are taken directly from the Weld codebase.
In such case, the corresponding zero value must be injected instead.
The specification is clear that field injection points with `@Named`
without value should use `@Named(fieldName)` as the qualifier. Other
such injection points lead to a definition error.
@Ladicek
Copy link
Contributor Author

Ladicek commented Jan 24, 2023

Rebased (and addressed the nitpick 😆).

@quarkus-bot
Copy link

quarkus-bot bot commented Jan 24, 2023

Failing Jobs - Building b5d825a

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 11 Windows Build Failures Logs Raw logs
✔️ JVM Tests - JDK 17
✔️ JVM Tests - JDK 18

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/grpc/deployment 
! Skipped: extensions/micrometer-registry-prometheus/deployment extensions/micrometer/deployment extensions/opentelemetry/deployment and 41 more

📦 extensions/grpc/deployment

io.quarkus.grpc.server.devmode.GrpcDevModeTest.testEchoStreamReload - More details - Source on GitHub

java.util.concurrent.RejectedExecutionException
	at org.jboss.threads.RejectingExecutor.execute(RejectingExecutor.java:38)
	at org.jboss.threads.EnhancedQueueExecutor.rejectShutdown(EnhancedQueueExecutor.java:2076)

@gsmet gsmet merged commit 6faf038 into quarkusio:main Jan 25, 2023
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jan 25, 2023
@quarkus-bot quarkus-bot bot added this to the 2.17 - main milestone Jan 25, 2023
@Ladicek Ladicek deleted the arc-fixes branch January 25, 2023 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants