dsmiley commented on code in PR #4893:
URL: https://github.com/apache/solr/pull/4893#discussion_r3969285070
##########
solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java:
##########
@@ -47,8 +47,14 @@ public class EnvUtils {
/** Maps ENV keys to sys prop keys for special/custom mappings */
private static final Map<String, String> CUSTOM_MAPPINGS = new HashMap<>();
+ /**
+ * Maps a legacy/deprecated sys prop key (dot-separated form) to the current
property it was
+ * replaced by, and whether the replacement is boolean-inverted relative to
the legacy property.
+ */
+ private record DeprecatedMapping(String currentName, boolean inverted) {}
Review Comment:
okay/fine but merely for a boolean you could simply have a parallel set of
inverted names. That's what I'd do.
##########
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java:
##########
@@ -28,9 +29,10 @@
public class ResponseUtils {
private ResponseUtils() {}
- // System property to use if the Solr core does not exist or
solr.hideStackTrace is not
- // configured. (i.e.: a lot of unit test).
- private static final boolean SYSTEM_HIDE_STACK_TRACES =
Boolean.getBoolean("solr.hideStackTrace");
+ // System property to use if the Solr core does not exist or
solr.responses.stacktrace.enabled
Review Comment:
"solr core does not exist" seems very out of place, looking at this
property's intuitive purpose
##########
solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java:
##########
@@ -108,24 +120,52 @@ public static String getProperty(String key) {
* @param defaultValue fallback value if property is not found
*/
public static String getProperty(String key, String defaultValue) {
- String value = getPropertyWithCamelCaseFallback(key);
+ String value = resolvePropertyValue(key);
return value != null ? value : defaultValue;
}
/**
- * Get a property from given key or an alias key converted from CamelCase to
dot separated.
+ * Resolves {@code key} by trying, in order: the key as given; the key
converted from CamelCase to
+ * dot-separated; and -- if {@code key} is itself a legacy/deprecated
property name -- the current
+ * property it was replaced by. See {@link #resolveViaDeprecatedMapping} for
why that last
+ * fallback is needed.
*
- * @return property value or value of dot-separated alias key or null if not
found
+ * @return the resolved value, or null if none of the above are found
*/
- private static String getPropertyWithCamelCaseFallback(String key) {
+ private static String resolvePropertyValue(String key) {
String value = System.getProperty(key);
if (value != null) {
return value;
- } else {
- // Figure out if string is CamelCase and convert to dot separated
- String altKey = camelCaseToDotSeparated(key);
- return System.getProperty(altKey);
}
+ // Figure out if string is CamelCase and convert to dot separated
+ String altKey = camelCaseToDotSeparated(key);
+ value = System.getProperty(altKey);
+ if (value != null) {
+ return value;
+ }
+ return resolveViaDeprecatedMapping(altKey);
+ }
+
+ /**
+ * If {@code dotKey} is a known legacy/deprecated property name, returns the
value of the current
+ * property it was replaced by (inverted, if the mapping is
boolean-inverted), or null if that
+ * current property hasn't been set either.
+ *
+ * <p>This matters for config files (e.g. {@code solr.xml}) that still
contain a {@code
+ * ${legacyName:default}} substitution token from before a property was
renamed: without this,
+ * setting only the new property name would silently have no effect on that
token, since nothing
+ * else ever rewrites the token's text. See SOLR-17864.
+ */
+ private static String resolveViaDeprecatedMapping(String dotKey) {
+ DeprecatedMapping mapping = DEPRECATED_MAPPINGS.get(dotKey);
+ if (mapping == null) {
+ return null;
+ }
+ String newValue = System.getProperty(mapping.currentName());
Review Comment:
IMO at this juncture, we should log a warning to alert the user to use the
new name. See the DeprecationLog to do this elegantly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
