dcapwell commented on a change in pull request #1335:
URL: https://github.com/apache/cassandra/pull/1335#discussion_r804192649



##########
File path: src/java/org/apache/cassandra/db/virtual/SettingsTable.java
##########
@@ -97,123 +66,91 @@
         this.config = config;
     }
 
-    @VisibleForTesting
-    Object getValue(Field f)
-    {
-        Object value;
-        try
-        {
-            value = f.get(config);
-        }
-        catch (IllegalAccessException | IllegalArgumentException e)
-        {
-            throw new ServerError(e);
-        }
-        return value;
-    }
-
-    private void addValue(SimpleDataSet result, Field f)
-    {
-        Object value = getValue(f);
-        if (value == null)
-        {
-            result.row(f.getName());
-        }
-        else if (overrides.containsKey(f.getName()))
-        {
-            overrides.get(f.getName()).accept(result, f);
-        }
-        else
-        {
-            if (value.getClass().isArray())
-                value = Arrays.toString((Object[]) value);
-            result.row(f.getName()).column(VALUE, value.toString());
-
-            if (ANNOTATED_FIELDS.containsKey(f.getName()) && 
!EXCLUDED_CONFIG.contains(f.getName()))
-            {
-                Replaces annotation = f.getAnnotation(Replaces.class);
-                result.row(annotation.oldName())
-                      .column(VALUE, annotation.converter()
-                                               .deconvert(value)
-                                               .toString());
-            }
-        }
-    }
-
     @Override
     public DataSet data(DecoratedKey partitionKey)
     {
         SimpleDataSet result = new SimpleDataSet(metadata());
         String name = UTF8Type.instance.compose(partitionKey.getKey());
-        Field field = FIELDS.get(name);
-        if (field != null)
-        {
-            addValue(result, field);
-        }
-        else
-        {
-            // rows created by overrides might be directly queried so include 
them in result to be possibly filtered
-            for (String override : overrides.keySet())
-                if (name.startsWith(override))
-                    addValue(result, FIELDS.get(override));
-        }
+        if (BACKWARDS_COMPATABLE_NAMES.containsKey(name))
+            ClientWarn.instance.warn("key '" + name + "' is deprecated; should 
switch to '" + BACKWARDS_COMPATABLE_NAMES.get(name) + "'");
+        if (PROPERTIES.containsKey(name))
+            result.row(name).column(VALUE, getValue(PROPERTIES.get(name)));
         return result;
     }
 
     @Override
     public DataSet data()
     {
         SimpleDataSet result = new SimpleDataSet(metadata());
-        for (Field setting : FIELDS.values())
-            addValue(result, setting);
+        for (Map.Entry<String, Property> e : PROPERTIES.entrySet())
+            result.row(e.getKey()).column(VALUE, getValue(e.getValue()));
         return result;
     }
 
-    private void addAuditLoggingOptions(SimpleDataSet result, Field f)
+    private String getValue(Property prop)
     {
-        
Preconditions.checkArgument(AuditLogOptions.class.isAssignableFrom(f.getType()));
-
-        AuditLogOptions value = (AuditLogOptions) getValue(f);
-        result.row(f.getName() + "_enabled").column(VALUE, 
Boolean.toString(value.enabled));
-        result.row(f.getName() + "_logger").column(VALUE, 
value.logger.class_name);
-        result.row(f.getName() + "_audit_logs_dir").column(VALUE, 
value.audit_logs_dir);
-        result.row(f.getName() + "_included_keyspaces").column(VALUE, 
value.included_keyspaces);
-        result.row(f.getName() + "_excluded_keyspaces").column(VALUE, 
value.excluded_keyspaces);
-        result.row(f.getName() + "_included_categories").column(VALUE, 
value.included_categories);
-        result.row(f.getName() + "_excluded_categories").column(VALUE, 
value.excluded_categories);
-        result.row(f.getName() + "_included_users").column(VALUE, 
value.included_users);
-        result.row(f.getName() + "_excluded_users").column(VALUE, 
value.excluded_users);
+        Object value = prop.get(config);
+        return value == null ? null : value.toString();
     }
 
-    private void addEncryptionOptions(SimpleDataSet result, Field f)
+    private static Map<String, Property> getProperties()
     {
-        
Preconditions.checkArgument(EncryptionOptions.class.isAssignableFrom(f.getType()));
-
-        EncryptionOptions value = (EncryptionOptions) getValue(f);
-        result.row(f.getName() + "_enabled").column(VALUE, 
Boolean.toString(value.isEnabled()));
-        result.row(f.getName() + "_algorithm").column(VALUE, value.algorithm);
-        result.row(f.getName() + "_protocol").column(VALUE, 
Objects.toString(value.acceptedProtocols(), null));
-        result.row(f.getName() + "_cipher_suites").column(VALUE, 
Objects.toString(value.cipher_suites, null));
-        result.row(f.getName() + "_client_auth").column(VALUE, 
Boolean.toString(value.require_client_auth));
-        result.row(f.getName() + "_endpoint_verification").column(VALUE, 
Boolean.toString(value.require_endpoint_verification));
-        result.row(f.getName() + "_optional").column(VALUE, 
Boolean.toString(value.isOptional()));
-
-        if (value instanceof EncryptionOptions.ServerEncryptionOptions)
+        Loader loader = Properties.defaultLoader();
+        Map<String, Property> properties = loader.flatten(Config.class);
+        // only handling top-level replacements for now, previous logic was 
only top level so not a regression
+        Map<String, Replacement> replacements = 
Replacements.getNameReplacements(Config.class).get(Config.class);
+        if (replacements != null)
         {
-            EncryptionOptions.ServerEncryptionOptions server = 
(EncryptionOptions.ServerEncryptionOptions) value;
-            result.row(f.getName() + "_internode_encryption").column(VALUE, 
server.internode_encryption.toString());
-            result.row(f.getName() + "_legacy_ssl_storage_port").column(VALUE, 
Boolean.toString(server.legacy_ssl_storage_port_enabled));
+            for (Replacement r : replacements.values())
+            {
+                Property latest = properties.get(r.newName);
+                assert latest != null : String.format("Unable to find 
replacement new name: " + r.newName);
+                Property conflict = properties.put(r.oldName, 
r.toProperty(latest));
+                // some configs kept the same name, but changed the type, if 
this is detected then rely on the replaced property
+                assert conflict == null || r.oldName.equals(r.newName) : 
String.format("Multiple properties exist for " + r.oldName);
+            }
+        }
+        for (Map.Entry<String, String> e : 
BACKWARDS_COMPATABLE_NAMES.entrySet())
+        {
+            String oldName = e.getKey();
+            if (properties.containsKey(oldName))
+                throw new AssertionError("Name " + oldName + " is present in 
Config, this adds a conflict as this name had a different meaning in " + 
SettingsTable.class.getSimpleName());
+            String newName = e.getValue();
+            Property prop = Objects.requireNonNull(properties.get(newName), 
newName + " cant be found for " + oldName);
+            properties.put(oldName, Properties.rename(oldName, prop));
         }
+        return properties;
     }
 
-    private void addTransparentEncryptionOptions(SimpleDataSet result, Field f)
+    private static Map<String, String> getBackwardsCompatableNames()
     {
-        
Preconditions.checkArgument(TransparentDataEncryptionOptions.class.isAssignableFrom(f.getType()));
-
-        TransparentDataEncryptionOptions value = 
(TransparentDataEncryptionOptions) getValue(f);
-        result.row(f.getName() + "_enabled").column(VALUE, 
Boolean.toString(value.enabled));
-        result.row(f.getName() + "_cipher").column(VALUE, value.cipher);
-        result.row(f.getName() + "_chunk_length_kb").column(VALUE, 
Integer.toString(value.chunk_length_kb));
-        result.row(f.getName() + "_iv_length").column(VALUE, 
Integer.toString(value.iv_length));
+        Map<String, String> names = new HashMap<>();
+        // Names that dont match yaml
+        names.put("audit_logging_options_logger", 
"audit_logging_options.logger.class_name");
+        names.put("server_encryption_options_client_auth", 
"server_encryption_options.require_client_auth");
+        names.put("server_encryption_options_endpoint_verification", 
"server_encryption_options.require_endpoint_verification");
+        names.put("server_encryption_options_legacy_ssl_storage_port", 
"server_encryption_options.legacy_ssl_storage_port_enabled");
+        names.put("server_encryption_options_protocols", 
"server_encryption_options.accepted_protocols");
+
+        // matching names
+        names.put("audit_logging_options_audit_logs_dir", 
"audit_logging_options.audit_logs_dir");
+        names.put("audit_logging_options_enabled", 
"audit_logging_options.enabled");
+        names.put("audit_logging_options_excluded_categories", 
"audit_logging_options.excluded_categories");
+        names.put("audit_logging_options_excluded_keyspaces", 
"audit_logging_options.excluded_keyspaces");
+        names.put("audit_logging_options_excluded_users", 
"audit_logging_options.excluded_users");
+        names.put("audit_logging_options_included_categories", 
"audit_logging_options.included_categories");
+        names.put("audit_logging_options_included_keyspaces", 
"audit_logging_options.included_keyspaces");
+        names.put("audit_logging_options_included_users", 
"audit_logging_options.included_users");
+        names.put("server_encryption_options_algorithm", 
"server_encryption_options.algorithm");
+        names.put("server_encryption_options_cipher_suites", 
"server_encryption_options.cipher_suites");
+        names.put("server_encryption_options_enabled", 
"server_encryption_options.enabled");
+        names.put("server_encryption_options_internode_encryption", 
"server_encryption_options.internode_encryption");
+        names.put("server_encryption_options_optional", 
"server_encryption_options.optional");
+        names.put("transparent_data_encryption_options_chunk_length_kb", 
"transparent_data_encryption_options.chunk_length_kb");
+        names.put("transparent_data_encryption_options_cipher", 
"transparent_data_encryption_options.cipher");
+        names.put("transparent_data_encryption_options_enabled", 
"transparent_data_encryption_options.enabled");
+        names.put("transparent_data_encryption_options_iv_length", 
"transparent_data_encryption_options.iv_length");

Review comment:
       this makes sure the old `_` prefix is still present even though the new 
`.` is there; in case anyone was using before




-- 
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]

Reply via email to