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

[EJBCLIENT-419] Implement EE interoperability (disabled by default, enabled via org.wildfly.ee.interoperable sys prop) #557

Merged
merged 3 commits into from
Sep 21, 2022

Conversation

fl4via
Copy link
Contributor

@fl4via fl4via commented Sep 15, 2022

…ios.

This introduces new EJB protocol version number 4.
Protocol versions 1 2 and 3 mean JBoss EJB Client runs in JavaEE environment.
Protocol versions 4 and above mean JBoss EJB Client runs in JakartaEE environment.
@fl4via
Copy link
Contributor Author

fl4via commented Sep 15, 2022

@ropalka please review as well

* Indicates if EE interoperable mode is enabled.
*/
public static final boolean EE_INTEROPERABLE_MODE = Boolean.parseBoolean(
org.wildfly.security.manager.WildFlySecurityManager.getPropertyPrivileged("org.wildfly.ee.interoperable", "false"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for now I'm keeping this property duplicated across the affected projects (but as Richard pointed, maybe we should move this to marshalling?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with splitting it into multiple components - this is something we can fix later if needed.

@fl4via
Copy link
Contributor Author

fl4via commented Sep 15, 2022

@tadamski should we enable testing with the system property enabled? Notice that the test will still ignore the ee interoperability because both ends will be Jakarta. On the other hand, at least this small bit will be test. Just let me know what you prefer.

Copy link
Contributor

@ropalka ropalka left a comment

Choose a reason for hiding this comment

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

Just few minor issues to be resolved.
Otherwise PR looks very good to me.

* @author Flavia Rainone
* @author Richard Opalka
*/
class EEInteroperability {
Copy link
Contributor

Choose a reason for hiding this comment

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

Class should be final.

class EEInteroperability {
// Batavia transformer sensible constant - it can start with either "javax." or "jakarta." if transformation was performed
private static final String VARIABLE_CONSTANT = "javax.ejb.FAKE_STRING";
public static final boolean JAKARTAEE_ENVIRONMENT = VARIABLE_CONSTANT.startsWith("jakarta");
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for public classifier for package private class

/**
* Indicates if EE interoperable mode is enabled.
*/
public static final boolean EE_INTEROPERABLE_MODE = Boolean.parseBoolean(
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for public classifier for package private class

Copy link
Contributor

@ropalka ropalka Sep 15, 2022

Choose a reason for hiding this comment

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

If we would reorder JAKARTAEE_ENVIRONMENT and Boolean.parseBoolean() in the condition then we wouldn't need to read the system property (access the file system) when JavaEE environment is detected.

* Indicates if EE interoperable mode is enabled.
*/
public static final boolean EE_INTEROPERABLE_MODE = Boolean.parseBoolean(
org.wildfly.security.manager.WildFlySecurityManager.getPropertyPrivileged("org.wildfly.ee.interoperable", "false"))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with splitting it into multiple components - this is something we can fix later if needed.

* @param marshallingConfiguration the marshalling configuration that will be used by the endpoint
* @param channelProtocolVersion the channel protocol version used by the endpoint
*/
public static void handleInteroperability(MarshallingConfiguration marshallingConfiguration, int channelProtocolVersion) {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for public classifier for package private class

}
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add private constructor to forbid instantiating this utility class.

… enable it only when org.wildfly.ee.namespace.interop system property is true

Signed-off-by: Flavia Rainone <frainone@redhat.com>
@chengfang
Copy link
Contributor

@fl4via I think it will be good to enable this EE Interop for testing in this project, say, in surefire config.

@chengfang chengfang merged commit 67b957f into wildfly:4.0 Sep 21, 2022
@fl4via fl4via deleted the EJBCLIENT-419_Final branch September 22, 2022 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants