Make AmqpAppender.Event public

[Romster]

In the official documentation it is recommended to override AmqpAppender#postProcessMessageBeforeSend to customise the message.
But the second parameter of this method is an AmqpAppender.Event, which is declared as protected static.
It seems strange to use a class with that declaration in a public method, which is designed to be overridden in client code, don't you think so?

Also I faced the problem when I tried to implement my AmqpAppender in Kotlin.

[garyrussell]

It seems strange to use a class with that declaration in a public method, which is designed to be overridden in client code, don't you think so?

Why would it seem "strange" when it works fine in java?

@SpringBootApplication
public class Rgh722Application {

	public static void main(String[] args) {
		SpringApplication.run(Rgh722Application.class, args);
	}

	public static class MyAppender extends AmqpAppender {

		@Override
		public Message postProcessMessageBeforeSend(Message message, Event event) {
			message.getMessageProperties().setHeader("foo", "foobar");
			return message;
		}

	}

}

The class wasn't designed for use in Kotlin; we can certainly open it up to make it usable in Kotlin, given that it has different semantics to Java. Contributions are welcome.

[Romster]

@garyrussell Well.
Maybe it seems strange to me because I haven't written in Java for some time.
But it's great if you are ready to open for Kotlin interoperability. And I would be glad to contribute this change, even if it looks insignificant.

[artembilan]

But it's great if you are ready to open for Kotlin interoperability

Well, @Romster , I would suggest you to ask Kotlin guys on the matter to determine why it isn't interoperable with Java. This is JVM language, so what is the point to fix foundation to tolerate a flaw in the target implementation?

IMO, the problem is in Kotlin if it isn't compatible with Java.

[garyrussell]

I agree that it seems to be a problem with Kotlin's inconsistency, but I don't think it's a big deal to make that class public.

[Areks]

@artembilan

IMO, the problem is in Kotlin if it isn't compatible with Java.

Yes, you are right, kotlin should support interop with java if it declare this.
But i see some design problem in this solution: why you use protected nested class in public api?
I think all objects for external api should be public, isn't it?

[garyrussell]

It's not really a "public" API, it's just for subclasses to override; the method should really be protected too.

[artembilan]

H-m-m. I wonder if Checkstyle can have such a rule to disallow similar public-protected design...

However I agree that the postProcessMessageBeforeSend method must not be public.

So, let's consider it as a bug and even back-port to 1.7.x!

[Romster]

Yes, to make this method protected is a great option!
I've opened a ticket:
https://jira.spring.io/browse/AMQP-805