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

Jump to bottom

CAMEL-10918 create sjms2 component to add support for JMS 2.0 #1514

Closed
wants to merge 1 commit into from
Closed

CAMEL-10918 create sjms2 component to add support for JMS 2.0 #1514

wants to merge 1 commit into from

Conversation

ryeats
Copy link
Contributor

@ryeats ryeats commented Mar 8, 2017

This PR adds a SJMS2 component that can support shared subscriptions along with some modifications to the SJMS component so that it could be extended by the new SJMS2 component.

I created the branch off of 2.18.x let me know if it should be off master.

@davsclaus
Copy link
Contributor

Yes master this will only be included in 2.19 onwards

@ryeats ryeats changed the base branch from camel-2.18.x to master March 8, 2017 19:00
@ryeats ryeats force-pushed the sjms-sharedconsumer branch 2 times, most recently from d878f6b to 4d1464f Compare March 8, 2017 19:17
davsclaus
davsclaus requested changes Mar 8, 2017
View reviewed changes
components/camel-sjms/src/main/java/org/apache/camel/component/sjms/SjmsEndpoint.java Outdated
@@ -98,6 +99,9 @@
private boolean persistent = true;
@UriParam(label = "consumer")
private String durableSubscriptionId;
private String subscriptionId;
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to add UriParam for these new options as well. For the new options that are on for jms2 you can move to it instead of keeping it here on the old component

components/camel-sjms2/src/main/docs/sjms2-component.adoc Outdated


// component options: START
[NOTE]
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be auto generated so no NOTE here

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 pushed up what I have but I am having trouble with the docs it doesn't look like its generating the content and they are failing validation, I am probably missing something obvious.

components/camel-sjms2/src/main/java/org/apache/camel/component/sjms2/Sjms2Component.java
/**
* The <a href="http://camel.apache.org/sjms">Simple JMS2</a> component.
*/
public class Sjms2Component extends SjmsComponent {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need a Sjms2Endpoint class as well with @UriEndpoint and all that stuff

components/camel-sjms2/src/main/java/org/apache/camel/component/sjms2/Sjms2Component.java Outdated
}

@Override
protected Endpoint createEndpoint(String uri, String remaining, Map<String, Object> parameters) throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a afterConfiguration method you can use instead to change the jms object factory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to go a different route but good to know for the future.

components/camel-sjms2/src/main/resources/META-INF/services/org/apache/camel/component/sjms2
@@ -0,0 +1 @@
class=org.apache.camel.component.sjms2.Sjms2Component
Copy link
Contributor

Choose a reason for hiding this comment

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

Need ASF license headers like the other file has

components/camel-sjms2/src/test/resources/log4j2.properties
@@ -0,0 +1,7 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

Need ASF license header

Copy link
Contributor

Choose a reason for hiding this comment

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

And log to file like the other does

@ryeats
Copy link
Contributor Author

ryeats commented Mar 9, 2017

I am having an issue getting the generated portion of the docs to work and getting it to pass validation but I pushed up what I had while I try and figure that out.

@ryeats ryeats force-pushed the sjms-sharedconsumer branch 2 times, most recently from 44f3b63 to c9f1596 Compare March 9, 2017 19:49
ryeats
ryeats commented Mar 10, 2017
View reviewed changes
tooling/apt/src/main/java/org/apache/camel/tools/apt/DocumentationHelper.java
@@ -110,6 +110,8 @@ private static File jsonFile(String scheme, String extendsScheme) {
return new File("../camel-ftp/target/classes/org/apache/camel/component/file/remote/ftp.json");
} else if ("jms".equals(extendsScheme)) {
return new File("../camel-jms/target/classes/org/apache/camel/component/jms/jms.json");
} else if ("sjms".equals(extendsScheme)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This didn't seem to resolve my docs issue either... still looking.

davsclaus
davsclaus reviewed Mar 10, 2017
View reviewed changes
components/camel-sjms2/src/main/java/org/apache/camel/component/sjms2/Sjms2Component.java Outdated
@Override
protected Endpoint createEndpoint(String uri, String remaining, Map<String, Object> parameters) throws Exception {
validateMepAndReplyTo(parameters);
Sjms2Endpoint endpoint = new Sjms2Endpoint(uri, this, remaining);
Copy link
Contributor

Choose a reason for hiding this comment

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

It would likely be easier to create a protected method in the camel-sjms component to create the endpoint class, we do that for some other components we extend. Then this component just need to create a new Sjms2Endpoint and set the jms2 object factory and all the other configuration below is the same and done on the base class

@davsclaus
Copy link
Contributor

For the documentation to work you would have to add the documentation to every @UriParam in the camel-sjms endpoint class.

And for the component you must do the same for every @metadata

This has been done on the other JMS component, eg camel-jms such as
https://github.com/apache/camel/blob/master/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsEndpoint.java#L84

https://github.com/apache/camel/blob/master/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsConfiguration.java#L89

https://github.com/apache/camel/blob/master/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java#L66

You can copy the documentation from the setter methods.

The reason is that the apt plugin cannot read the source code from external JARs and therefore need access to the documentation using annotations.

@ryeats ryeats force-pushed the sjms-sharedconsumer branch 3 times, most recently from b2e2442 to 3b4f352 Compare March 13, 2017 14:04
@davsclaus
Copy link
Contributor

Thanks for the PR it has been merged. Do you mind closing this?

@ryeats ryeats closed this Mar 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davsclaus davsclaus davsclaus requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@ryeats @davsclaus