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 - validate Event#select and Event#fire methods for type variables #30917

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

manovotn
Copy link
Contributor

@manovotn manovotn commented Feb 6, 2023

This PR aligns event payload validation to that of TCK and Weld.
Related to #28558

From the specification:

(in the desc of Event#select() method)
If the specified type contains a type variable, an IllegalArgumentException is thrown.

The methods fire() and fireAsync() fire an event with the specified qualifiers and notify observers, as defined by Observer notification. If the container is unable to resolve the parameterized type of the event object, it uses the specified type to infer the parameterized type of the event types.

If the runtime type of the event object contains an unresolvable type variable, an IllegalArgumentException is thrown.

An event type may not contain an unresolvable type variable. A wildcard type is not considered an unresolvable type variable.

@mkouba @Ladicek this is a follow up on my Fri Zulip rambling, curious to hear WDYT :)

@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Feb 6, 2023
@manovotn manovotn force-pushed the eventPayloadValidation branch 2 times, most recently from 4653929 to 29293cd Compare February 6, 2023 12:46
// expected
}
SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get();
TypeLiteral<SecurityEvent_Illegal<T>> typeLiteral = new TypeLiteral<SecurityEvent_Illegal<T>>() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A beer to whoever can explain to me why declaring this TypeLiteral directly inside the lambda leads to the type variable not being recognized correctly :)

Copy link
Contributor

Choose a reason for hiding this comment

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

🤷

Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. No beer for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

I even tried to reproduce this in a minimal single-file Java program, but couldn't 🤷

@manovotn manovotn requested review from Ladicek and mkouba February 6, 2023 12:51
@manovotn manovotn marked this pull request as ready for review February 6, 2023 12:51
@manovotn manovotn force-pushed the eventPayloadValidation branch from 29293cd to 709478c Compare February 6, 2023 14:42
@manovotn manovotn requested a review from mkouba February 6, 2023 14:42
@quarkus-bot

This comment has been minimized.

@Ladicek
Copy link
Contributor

Ladicek commented Feb 7, 2023

Otherwise LGTM.

@manovotn manovotn force-pushed the eventPayloadValidation branch from 709478c to e62c88d Compare February 7, 2023 12:44
@manovotn manovotn added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 7, 2023
@mkouba mkouba removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 7, 2023
@manovotn manovotn force-pushed the eventPayloadValidation branch from e62c88d to eb0bd36 Compare February 7, 2023 13:39
@mkouba mkouba added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 7, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 7, 2023

Failing Jobs - Building eb0bd36

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 Build ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18
✔️ Maven Tests - JDK 11
Maven Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: integration-tests/mongodb-rest-data-panache 

📦 integration-tests/mongodb-rest-data-panache

io.quarkus.it.mongodb.rest.data.panache.MongoDbRestDataPanacheTest.shouldNotCreateOrDeleteAuthor - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.mongodb.deployment.DevServicesMongoProcessor#startMongo threw an exception: java.lang.RuntimeException: java.lang.IllegalStateException: Previous attempts to find a Docker environment failed. Will not retry. Please see logs and check configuration

@manovotn
Copy link
Contributor Author

manovotn commented Feb 8, 2023

CI failures are unrelated

@manovotn manovotn merged commit e6760e8 into quarkusio:main Feb 8, 2023
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 8, 2023
@quarkus-bot quarkus-bot bot added this to the 3.0 - main milestone Feb 8, 2023
@manovotn manovotn deleted the eventPayloadValidation branch February 8, 2023 10:52
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.

3 participants