Script resolution depends on order of element in config file

[ppkarwasz]

The resolution of ScriptRef elements depends on the order of elements in the configuration file: the Scripts element must appear before any ScriptRef element.

[Suvrat1629]

(I am new to open source contribution and trying to understand the code)
Are the changes mentioned in the issue to be made in the log4j2.xml file or the log4j2.json?

[ppkarwasz]

The fix must be done in code.

The basic problem is that the resolution of script references is done in constructors or factory methods of script-enabled components. For example in ScriptFilter we have this snippet in the createFilter method:

logging-log4j2/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/ScriptFilter.java

Lines 148 to 157 in 5496c72

	if (script instanceof ScriptRef) {
	if (configuration.getScriptManager().getScript(script.getName()) == null) {
	logger.error("No script with name {} has been declared.", script.getName());
	return null;
	}
	} else {
	if (!configuration.getScriptManager().addScript(script)) {
	return null;
	}
	}

Since the constructors of Log4j components are called in the order the components are declared in the configuration file, something like this will fail:

<ScriptFilter>
  <ScriptRef ref="GLOBAL_FILTER"/>
</ScriptFilter>
<Scripts>
  <ScriptFile name="GLOBAL_FILTER"
              language="groovy"
              path="global-filter.groovy"/>
</Scripts>

will fail, since at the moment ScriptRef is resolved, the ScriptFile element has not been processed yet. To solve this problem, the resolution of ScriptRefs must be delayed. For example you can perform the resolution in the start() method of the LifeCycle interface.

[Suvrat1629]

Ok, I did make the changes as you suggested by using the start() method. What more changes do i need to make now? Do i need to change the testcases or anything?

[ppkarwasz]

@Suvrat1629,

Yes, if you want to contribute the fix, you should add some tests. See the PR template for more details.

Should I assign the issue to you?

[Suvrat1629]

Yes you can assign it to me, also can you guide me with the test cases changes as the ScriptFilterTest.java seems to be consistent with my code is there any file that i need to look into?