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.

Sign up for GitHub

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

fix(vertx): do not log a warning about missing json support when vertx extension is present #5623

Merged
merged 1 commit into from
Nov 20, 2019
Merged

fix(vertx): do not log a warning about missing json support when vertx extension is present #5623

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions extensions/vertx/deployment/src/main/java/io/quarkus/vertx/deployment/VertxProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import io.quarkus.arc.processor.AnnotationsTransformer;
import io.quarkus.arc.processor.BeanInfo;
import io.quarkus.arc.processor.BuildExtension;
import io.quarkus.deployment.Capabilities;
import io.quarkus.deployment.GizmoAdaptor;
import io.quarkus.deployment.annotations.BuildProducer;
import io.quarkus.deployment.annotations.BuildStep;
Expand All @@ -51,6 +52,11 @@ class VertxProcessor {
@Inject
BuildProducer<ReflectiveClassBuildItem> reflectiveClass;

@BuildStep(providesCapabilities = Capabilities.RESTEASY_JSON_EXTENSION)
Copy link
Member

Choose a reason for hiding this comment

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

It's not used for anything else?

Copy link
Contributor

Choose a reason for hiding this comment

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

Which part do you mean? Sorry for my thick headedness :)

Copy link
Member Author

Choose a reason for hiding this comment

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

The RESTEASY_JSON_EXTENSION Capability is used in jackson and jsonb extension to indicate json be support in ResteasyCommonProcessor. It was added for that case IIRC.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes it already exists which is I guess what @gsmet was asking :)

Copy link
Member

Choose a reason for hiding this comment

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

Well, I'm wondering if it's a good idea to reuse that one or if we should add a VERTX capability and alter the condition in the processor itself.

This capability was used to basically say "we have an extension to provide proper JSON support for RESTEasy" and this one is not exactly saying that. It's saying "Vert.x provides some sort of JSON support for its own types".

Maybe I'm overthinking it, I don't know.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are OK with that @machi1990 added for the time being. My view is that if we need something more complicated later, we can always improve things

Copy link
Member Author

Choose a reason for hiding this comment

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

+1 @geoand

WDYT @gsmet ?

Copy link
Member

Choose a reason for hiding this comment

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

I think I agree with myself but let's do it your way and I'll keep my "I told you so" for later :).

Copy link
Contributor

Choose a reason for hiding this comment

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

It wouldn't be the first time 😁

Copy link
Member Author

Choose a reason for hiding this comment

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

It wouldn't be the first time 😁

ha ha indeed :-)

FeatureBuildItem feature() {
return new FeatureBuildItem(FeatureBuildItem.VERTX);
}

@BuildStep
AdditionalBeanBuildItem registerBean() {
return AdditionalBeanBuildItem.unremovableOf(VertxProducer.class);
Expand All @@ -59,13 +65,11 @@ AdditionalBeanBuildItem registerBean() {
@BuildStep
@Record(ExecutionTime.RUNTIME_INIT)
VertxBuildItem build(CoreVertxBuildItem internalVertx, VertxRecorder recorder, BeanContainerBuildItem beanContainer,
BuildProducer<FeatureBuildItem> feature,
List<EventConsumerBusinessMethodItem> messageConsumerBusinessMethods,
BuildProducer<GeneratedClassBuildItem> generatedClass,
AnnotationProxyBuildItem annotationProxy, LaunchModeBuildItem launchMode, ShutdownContextBuildItem shutdown,
BuildProducer<ServiceStartBuildItem> serviceStart,
List<MessageCodecBuildItem> codecs, RecorderContext recorderContext) {
feature.produce(new FeatureBuildItem(FeatureBuildItem.VERTX));
Map<String, ConsumeEvent> messageConsumerConfigurations = new HashMap<>();
ClassOutput classOutput = new GizmoAdaptor(generatedClass, true);
for (EventConsumerBusinessMethodItem businessMethod : messageConsumerBusinessMethods) {
Expand Down