[jwrosewell]

Problem

The specification's Translation Engine has a configuration file set the options that take a single value, being SourceElementDataKey, FixedLanguage and MissingTranslationBehavior with the values Original, EmptyString and FlowError, while sources and translations are added in code (translation-engine.md, Configuration, and Processing for the values).

.NET does this by taking the builder from its service collection when one is supplied (PipelineBuilder.cs lines 411 to 414), so code can register a TranslationEngineBuilder that already holds its sources and translations, and the file sets the rest. That matters because the engine refuses to build without at least one source and one translation, in .NET (TranslationEngineBase.cs lines 188 to 204) and in Java (TranslationEngineBase lines 143 to 155).

Java could not do any of this on rc (2299507).

1. PipelineBuilder always constructed a new builder for a configured element (line 250), so code had no way to give it sources and translations.
2. TranslationEngineBuilder had no @ElementBuilder annotation (line 45), and the configuration route only finds annotated builders (lines 187 to 189), so no file could name it.
3. MissingTranslationBehavior had no static parse method, which the configuration route needs to turn text into any type other than a string (lines 953 to 989), so a file could not set the behavior. When a parse method did refuse a value, the error dropped the parse method's own message.

Change

This follows .NET and the specification's Configuration section, with code adding sources and translations and the file setting the single-value options.

• PipelineBuilder.addElementBuilder(Object builder), a new public method in pipeline.core. It gives the configuration route a builder prepared in code, which buildFromConfiguration uses for any element whose builder name resolves to that builder's type, in place of constructing one. The builder's type must still be one a configuration can name, being annotated with @ElementBuilder. This is the one new public surface in pipeline.core, and its name and shape are a choice for James to review.
• Two prepared builders of the same type. The first added is used, and every element of that type in the configuration uses that one instance. This matches .NET's own FiftyOneServiceProvider, which returns the first service of a matching type (FiftyOneServiceProvider.cs line 58), and the existing getService in this class, which does the same for pipeline services. In .NET the rule belongs to whichever service provider the application passes in, and this is the one 51Degrees supplies.
• parseToType now keeps the parse method's own message in the error it throws, and passes the parse method's exception as the cause.
• TranslationEngineBuilder is annotated with @ElementBuilder, so a file names it as TranslationEngine or TranslationEngineBuilder. No other builder has either name, so the IP Intelligence country builders resolve as before. Its existing setters already carry the specification's names, setSourceElementDataKey, setFixedLanguage and setMissingTranslationBehavior, and no older names are in use, so there are no deprecated aliases. When either sources or translations are missing, build now says they are added in code and how to use addElementBuilder. Before, the engine's message was "At least one property translation must be configured.", which did not say where they come from. The exception type is unchanged.
• MissingTranslationBehavior.parse(String) accepts Original, EmptyString and FlowError and the constant names ORIGINAL, EMPTY_STRING and FLOW_ERROR, ignoring case and any white space around the text. Anything else is refused with "'Sometimes' is not a missing translation behavior. Use Original, EmptyString or FlowError."

An example configuration, with the builder prepared in code:

TranslationEngineBuilder translation = new TranslationEngineBuilder(loggerFactory)
    .addSource("countries/*.yml")
    .addTranslation("Country", "CountryTranslated");
Pipeline pipeline = new PipelineBuilder(loggerFactory)
    .addElementBuilder(translation)
    .buildFromConfiguration(options);

<Element>
    <BuilderName>TranslationEngine</BuilderName>
    <BuildParameters>
        <SourceElementDataKey>ip</SourceElementDataKey>
        <FixedLanguage>en_GB</FixedLanguage>
        <MissingTranslationBehavior>Original</MissingTranslationBehavior>
    </BuildParameters>
</Element>

Options considered and not taken

• Letting a configuration file set sources and translations, as the draft specifications#100 would allow. Java's configuration route cannot pass a pair of values, and XML build parameters with the same name overwrite each other, so translations would need a new text form, such as Country=CountryTranslated with commas between pairs, chosen before the specification settles it. It would also need a fix so that AddSource, which has two overloads, is matched reliably. That is a public format I did not count as simple and clearly safe.
• Only annotating the builder and adding the parse method. A file could then name the builder and set its options, but nothing could give it sources and translations, so building it from a file would always fail.

Tests

• pipeline.core, PipelineBuilderPreparedBuilderTest, using the existing ListSplitterElementBuilder test builder:
  • A prepared builder keeps the options code gave it and takes the ones the configuration sets.
  • With two prepared builders of the same type, the first is used.
  • A prepared builder of another type leaves the configured element to a new builder.
  • A null builder is refused.
  • A value the parse method refuses keeps the parse method's message in the error.
• pipeline.translation, TranslationEngineConfigurationTests. Each case reads an XML configuration file that puts a test source element (CountrySourceElement) in front of TranslationEngine and sets SourceElementDataKey, FixedLanguage and MissingTranslationBehavior. The TranslationEngineBuilder is prepared in code with one source and one translation.
  • England translates to Angleterre with no language in the evidence, which shows both the source element data key and the fixed language came from the file.
  • Original, original, ORIGINAL and Original keep an untranslated value. The prepared builder is set to EmptyString first, so this shows the file's value replaced it.
  • EmptyString, emptystring, EMPTY_STRING and empty_string give an empty string.
  • FlowError, flowerror, FLOW_ERROR and flow_error add the flow error for the untranslated value.
  • Sometimes is refused with the accepted values in the message.
  • The same configuration with no prepared builder fails with the message that sources and translations are added in code.
• pipeline.translation, MissingTranslationBehaviorTests, covering parse directly: every accepted spelling, white space around the text, and null, empty, Sometimes, Empty String and Flow-Error refused with the exact message.
• ip-intelligence.translation, CountryBuildersConfigurationTest. One configuration names CountryCodeTranslationEngine, CountriesTranslationEngine and TranslationEngine, with a prepared general builder given. It checks that the country engines are still built by their own builders, and the general one by the prepared builder.

Checks

From the repository root, with Maven 3.9.16 on Java 21:

mvn -B -pl pipeline/pipeline.core,pipeline/pipeline.translation,ip-intelligence/ip-intelligence.translation -am test -DskipNativeBuild=true "-Dtest=fiftyone/pipeline/core/**/*Test*,fiftyone/pipeline/util/**/*Test*,fiftyone/pipeline/translation/**/*Test*,fiftyone/ipintelligence/translation/**/*Test*" -Dsurefire.failIfNoSpecifiedTests=false

Module	rc at 2299507	This branch at eb7842b
pipeline.core	173 run, 0 failed	178 run, 0 failed
pipeline.translation	23 run, 0 failed	53 run, 0 failed
ip-intelligence.translation	22 run, 0 failed, 1 skipped	23 run, 0 failed, 1 skipped

• The skipped test is CountryTranslationIntegrationTest, which needs an IP Intelligence data file and the native engine. It is skipped the same way on both.
• The IP Intelligence translation tests read the country translation files from the cxx/data submodule, so that submodule has to be checked out for them to pass, as CI does.
• The new tests do not compile against rc, because addElementBuilder and MissingTranslationBehavior.parse do not exist there.

What remains

• A configuration file still cannot add sources or translations, as the specification on rc says. A file that names AddTranslation is refused, because that method takes two values. AddSource has two overloads, and the configuration route uses whichever one reflection lists first, so a file that names it may or may not work. That is unchanged by this pull request.
• Every element of one builder type in a configuration uses the same prepared instance, as in .NET. Two general Translation Engines with different sources therefore cannot both be built from one configuration, unless one of them uses a builder of its own type, as the IP Intelligence country builders do.

For the release notes

The general Translation Engine can be built from a configuration file once code has given it its sources and translations, through the new PipelineBuilder.addElementBuilder. The file sets SourceElementDataKey, FixedLanguage and MissingTranslationBehavior, which accepts Original, EmptyString or FlowError.