[WFCORE-6981] fix incorrect alternatives in reply-properties inherited from basis attribute

[soul2zimate]

Issue: https://issues.redhat.com/browse/WFCORE-6981

[WFCORE-6981] fix incorrect alternatives in reply-properties inherited from basis attribute

[wildfly-ci]

Core -> WildFly Preview Integration Build 14084 outcome was FAILURE using a merge of f29af68
Summary: Tests failed: 1 (1 new), passed: 5001, ignored: 85 Build time: 02:51:52

Failed tests

org.wildfly.test.integration.microprofile.reactive.messaging.kafka.ssl.ReactiveMessagingKafkaSslConfiguredOnConnectionTestCase(layers-test): org.wildfly.test.integration.microprofile.reactive.messaging.kafka.ssl.ReactiveMessagingKafkaSslConfiguredOnConnectionTestCase(layers-test).org.wildfly.test.integration.microprofile.reactive.messaging.kafka.ssl.ReactiveMessagingKafkaSslConfiguredOnConnectionTestCase: java.lang.IllegalArgumentException: requirement failed: Each listener must have a different port, listeners: PLAINTEXT://localhost:41327,EXTERNAL://:9092,CONTROLLER://localhost:41327,INTERNAL://:19002
	at scala.Predef$.require(Predef.scala:337)
	at kafka.utils.CoreUtils$.checkDuplicateListenerPorts(CoreUtils.scala:181)
	at kafka.utils.CoreUtils$.$anonfun$listenerListToEndPoints$8(CoreUtils.scala:212)
	at kafka.utils.CoreUtils$.$anonfun$listenerListToEndPoints$8$adapted(CoreUtils.scala:209)
	at scala.collection.immutable.HashMap.foreach(HashMap.scala:1115)
	at kafka.utils.CoreUtils$.validate$1(CoreUtils.scala:209)
	at kafka.utils.CoreUtils$.listenerListToEndPoints(CoreUtils.scala:245)
	at kafka.server.KafkaConfig.listeners(KafkaConfig.scala:2177)
	at kafka.server.KafkaConfig.<init>(KafkaConfig.scala:1845)
	at kafka.server.KafkaConfig.<init>(KafkaConfig.scala:1639)
	at kafka.server.KafkaConfig.fromProps(KafkaConfig.scala:1561)
	at io.smallrye.reactive.messaging.kafka.companion.test.EmbeddedKafkaBroker.formatStorageFromConfig(EmbeddedKafkaBroker.java:361)
	at io.smallrye.reactive.messaging.kafka.companion.test.EmbeddedKafkaBroker.start(EmbeddedKafkaBroker.java:202)
	at org.wildfly.test.integration.microprofile.reactive.messaging.kafka.ssl.RunKafkaWithSslSetupTask.setup(RunKafkaWithSslSetupTask.java:54)
	at org.jboss.as.arquillian.container.ServerSetupObserver$ServerSetupTaskHolder.setup(ServerSetupObserver.java:259)
	at org.jboss.as.arquillian.container.ServerSetupObserver.executeSetup(ServerSetupObserver.java:186)
	at org.jboss.as.arquillian.container.ServerSetupObserver.handleBeforeDeployment(ServerSetupObserver.java:119)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)

[yersan]

Hello @soul2zimate

This is a partial fix, there are several operations that have this issue. We are getting these alternatives because the CONTENT_HASH attribute is already built with those alternatives in place:

wildfly-core/server/src/main/java/org/jboss/as/server/controller/resources/DeploymentAttributes.java

Lines 155 to 159 in e4bedf5

	public static final SimpleAttributeDefinition CONTENT_HASH =
	createContentValueTypeAttribute(ModelDescriptionConstants.HASH, ModelType.BYTES, new HashValidator(true), false,
	ModelDescriptionConstants.INPUT_STREAM_INDEX, ModelDescriptionConstants.BYTES, ModelDescriptionConstants.URL,
	ModelDescriptionConstants.PATH, ModelDescriptionConstants.RELATIVE_TO, ModelDescriptionConstants.EMPTY)
	.build();

If we look at that file, we can see several attributes that are created with those alternatives in place. So, this PR is a partial fix only for the usage of CONTENT_HASH in the UPLOAD_HASH_REPLY operation only/

The question is more why those attributes were created with all of those alternatives if they can end up as a single attribute in one operation. I don't know, but in such a case, the alternatives should be removed. Maybe @ehsavoie could remember.

So, I guess we need to perform a full check of those attributes and remove the alternatives when required in the Operation handlers where they are used, or, maybe fix their definition by adding them without alternatives and adding them when more than one is added to the same Operation.

[soul2zimate]

So, this PR is a partial fix only for the usage of CONTENT_HASH in the UPLOAD_HASH_REPLY operation only/

Hello @yersan, UPLOAD_HASH_REPLY is attribute, the fix applies to all related operations UPLOAD_BYTES_DEFINITION, DOMAIN_UPLOAD_BYTES_DEFINITION, UPLOAD_URL_DEFINITION, DOMAIN_UPLOAD_URL_DEFINITION, UPLOAD_STREAM_ATTACHMENT_DEFINITION and DOMAIN_UPLOAD_STREAM_ATTACHMENT_DEFINITION as UPLOAD_HASH_REPLY is the reply parameter. It doesn't make sense to provide alternatives for reply-properties.

AFAIU, I need to check the usage of this create method and make sure there are no incorrect alternatives inherited from basis attributes. In these places, where the basic attribute defines alternatives, it has already been taken care of.

e.g. https://github.com/wildfly/wildfly-core/blob/27.0.0.Beta1/logging/src/main/java/org/jboss/as/logging/handlers/SocketHandlerResourceDefinition.java#L84-L89 and https://github.com/wildfly/wildfly-core/blob/27.0.0.Beta1/logging/src/main/java/org/jboss/as/logging/handlers/SyslogHandlerResourceDefinition.java#L116

[ehsavoie]

You need all the alternative in the operation parameters but I think the issue was in the reply attribute that was created based on the parameter instead of the parameter name + type. I think it is getting more details from the parameter which shouldn't be defined there.

[yersan]

I see it now @soul2zimate , the UPLOAD_HASH_REPLY is used as a common structure for the reply parameters of all the upload_XXXX operations, but it was inheriting the alternative attributes from the CONTENT_HASH, hence the problem with the alternatives being leaked into the reply parameters

[yersan]

Thanks @soul2zimate and @ehsavoie