[WIP] Feature/move settings ruby to java

[andsel]

What does this PR do?

Why is it important/What is the impact to the user?

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files (and/or docker env variables)
• I have added tests that prove my fix is effective or that my feature works

Author's Checklist

• [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

[yaauie]

could this be pulled up into the specific SettingOption implementation referenced above? I'd like to avoid an ever-growing if chain. If we do leave the implementation here, perhaps a switch statement would be more efficient and read more clearly?

Suggested change

	if ("String".equals(rclass.getName())) {
	return String.class;
	}
	if ("Array".equals(rclass.getName())) {
	return ArrayList.class;
	}
	if ("Java::OrgLogstashUtil::ModulesSettingArray".equals(rclass.getName())) {
	return ModulesSettingArray.class;
	}
	if ("Java::OrgLogstashUtil::CloudSettingId".equals(rclass.getName())) {
	return CloudSettingId.class;
	}
	if ("Java::OrgLogstashUtil::CloudSettingAuth".equals(rclass.getName())) {
	return CloudSettingAuth.class;
	}
	if ("Java::OrgLogstashUtil::TimeValue".equals(rclass.getName())) {
	return TimeValue.class;
	}
	// this cover Boolean (Ruby doesn't have a class for it) and StringCoercible
	if ("Object".equals(rclass.getName())) {
	return Object.class;
	}
	if ("Integer".equals(rclass.getName())) {
	return Integer.class;
	}
	throw new IllegalArgumentException("Cannot find matching Java class for: " + rclass.getName());
	switch (rclass.getName()) {
	case "String":
	return String.class;
	case "Array":
	return ArrayList.class;
	case "Java::OrgLogstashUtil::ModulesSettingArray":
	return org.logstash.util.ModulesSettingArray.class;
	case "Java::OrgLogstashUtil::CloudSettingId":
	return org.logstash.util.CloudSettingId.class;
	case "Java::OrgLogstashUtil::CloudSettingAuth":
	return org.logstash.util.CloudSettingAuth.class;
	case "Java::OrgLogstashUtil::TimeValue":
	return org.logstash.util.TimeValue.class;
	case "Object":
	return Object.class;
	case "Integer":
	return Integer.class;
	default:
	throw new IllegalArgumentException("Cannot find matching Java class for: " + rclass.getName());
	}

[andsel]

Moved to switch, thanks.

[yaauie]

Thoughts:

While it might be a little more straight-forward here to just mirror our ruby setting in java-land, I think we have the opportunity to introduce a path toward safety.

In the end, I would prefer to have a collection of Setting objects that are immutable at runtime so that we can ensure that once they are initialized they are not modified. This will be especially useful as it will reduce the surface-area for bugs caused by plugins that reside in other code-bases modifying settings that they do not wholly own.

To achieve this, I think we need a few tweaks to the abstraction.

• Introduce java SettingOption, an immutable object representing a single configurable setting name along with its defaults, validators, etc., but not the value.
• Introduce java Setting, an immutable object representing the current effective value of the setting and the information about whether that value was explicitly given or implicit. Implementation will likely carry a reference to a specific SettingOption instance.
• Ruby Setting remains mutable, but as a wrapper for the immutable java Setting.
  • Its mutation methods (e.g., Setting#set(value), Setting#reset(value)) replace the reference to the new java Setting.
  • Its mutation methods emit a deprecation log once the settings have been parsed from config and/or command-line. This will likely be implemented as a global variable that gets set once the config has been loaded.
  • This bit could be implemented in vanilla ruby or in a jruby java extension.

This will enable us to independently move setting initialization into java at a later time, and to subsequently remove the mutation methods from the ruby shim in a future major release.

[andsel]

This PR is POC to move the head of settings hierarchy (the Setting class) from Ruby to Java. The idea is to create an isolation level (SettingExt + ProxyJavaSetting classes) to keep the 2 world separated to that we can try to improve the implementation on the Java side without affecting too much the Ruby one. Your idea to use value objects is great this saves from various side effects, however do we really have plugins that manipulate core settings?

[andsel]

@yaauie I've a couple of questions.

• what do you mean for "whether that value was explicitly given or implicit."? Is implicit the value of the setting passed at construction time while the explicit is the one provided to the set method?
• if we think to use the existing Ruby Setting class as a container of the Java Setting (+ SettingOption) then we don't have any reason to create a JRuby extension for the root class Setting like this PR is doing, right?

[yaauie]

nit: debuging to stdout