FISH-12987 Remove deprecated properties#7990
Conversation
…ssLoggingEnabled and sso-enabled from virtual-server
appserver/deployment/dtds/src/main/resources/glassfish/lib/dtds/glassfish-ejb-jar_3_1-1.dtd
Outdated
Show resolved
Hide resolved
Pandrex247
left a comment
Pandrex247
left a comment
There was a problem hiding this comment.
Review still in progress, but spotted this
|
|
||
| @LogMessageInfo(message = "No Endpoints selected in com.sun.appserv.iiop.endpoints property. Using JNDI Provider URL {0} instead") | ||
| @LogMessageInfo(message = "Using JNDI Provider URL {0}") | ||
| public static final String NO_ENDPOINTS_SELECTED_PROVIDER = "AS-ORB-00006"; |
There was a problem hiding this comment.
It might be worth dropping the AS-ORB code?
This aligned to a documented error message, which this isn't anymore.
https://glassfish.org/docs/5.1.0/error-messages-reference/error-messages.html
| // new property | ||
| if ((System.getProperty(SO_KEEPALIVE) == null && Boolean.getBoolean(SO_KEEPALIVE_DEPRECATED)) | ||
| || Boolean.getBoolean(SO_KEEPALIVE)) { | ||
| if (System.getProperty(SO_KEEPALIVE) == null || Boolean.getBoolean(SO_KEEPALIVE)) { |
There was a problem hiding this comment.
Not sure this is right?
The original behaviour was:
if SO_KEEPALIVE is null and SO_KEEPALIVE_DEPRECATED is true or SO_KEEPALIVE is true
Which unless I'm wrong means: evaluate to true if the new property isn't defined and the old one has been set to true, or if the new property has been defined and happens to be true
What you've got now is:
Evaluate to true if the property has not been defined or has been set to true
So there's a change in behaviour: it now evaluates as true if the new property hasn't been defined.
| if ((iiopListener.getPropertyValue(SO_KEEPALIVE) == null | ||
| && Boolean.valueOf(iiopListener.getPropertyValue(SO_KEEPALIVE_DEPRECATED))) | ||
| || Boolean.valueOf(iiopListener.getPropertyValue(SO_KEEPALIVE))) { | ||
| if (iiopListener.getPropertyValue(SO_KEEPALIVE) == null || Boolean.valueOf(iiopListener.getPropertyValue(SO_KEEPALIVE))) { |
There was a problem hiding this comment.
Same here, you need to lose the null check to retain the original behaviour
Also it would be better if you updated/removed the comment
2 participants
Description
See commits for specific removals.
Important Info
Blockers
n/a
Testing
New tests
n/a
Testing Performed
https://jenkins.payara.fish/view/TCKs/job/TCKs/job/JakartaEE-11-TCK/
https://jenkins.payara.fish/view/TCKs/job/TCKs/job/JakartaEE-11-TCK/105/
https://jenkins.payara.fish/view/TCKs/job/TCKs/job/JakartaEE-11-TCK/106/
Testing Environment
Apache Maven 3.9.8 (36645f6c9b5079805ea5009217e36f2cffd34256)
Maven home: /Users/kalinchan/Library/apache-maven-3.9.8
Java version: 21.0.9, vendor: Azul Systems, Inc., runtime: /Users/kalinchan/.sdkman/candidates/java/21.0.9-zulu/zulu-21.jdk/Contents/Home
Default locale: en_GB, platform encoding: UTF-8
OS name: "mac os x", version: "15.7.3", arch: "aarch64", family: "mac"
Documentation
pending
Notes for Reviewers