Skip to content

Add support for include-file-directive in config files. - #3771

Merged
t2gran merged 4 commits into
opentripplanner:dev-2.xfrom
entur:otp2_include_file_directive
Dec 16, 2021
Merged

Add support for include-file-directive in config files.#3771
t2gran merged 4 commits into
opentripplanner:dev-2.xfrom
entur:otp2_include_file_directive

Conversation

@t2gran

@t2gran t2gran commented Dec 9, 2021

Copy link
Copy Markdown
Member

Summary

This feature make it possible to inject the content of another file into a configuration file. It make it possible to keep part of the configuration in separate files. To include the content of a file use ${includeFile:FILE_NAME}. The FILE_NAME must be the name of a file in the configuration directory. Relative paths are not supported.

To allow both files (the configuration file and the injected file) to be valid JSON files a special case is supported; If the include file directive is quoted, then the quotes are removed if the text inserted is valid JSON. We check that the text to inject start with a `{` and ends with `}`.

Variable substitution is performed on the configuration file after the include file directive; Hence
variable substitution is also performed on the text in the injected file, but not inside the ${includeFile:FILE_NAME}.

Here is an example including variable substitution, assuming version 2.1.0 of OTP:

// build-config.json
{
  "storage" : "${includeFile:storage.json}"
} 
// storage.json
{
  "streetGraph": "street-graph-v${maven.version}.obj"
}

The result will look like this:

{
  "storage" : {
    "streetGraph": "street-graph-v2.1.0.obj"
  }
} 

Issue

There is no issue for this, but this feature where requested as part of the #3760.

Unit tests

Code style

Documentation

@t2gran
t2gran requested a review from a team as a code owner December 9, 2021 16:26
@t2gran t2gran added this to the 2.1 milestone Dec 9, 2021
@t2gran t2gran added the !Improvement A functional improvement or micro feature label Dec 9, 2021
@t2gran

t2gran commented Dec 9, 2021

Copy link
Copy Markdown
Member Author

In the dev-meeting today, I got some feedback that the "preview version" of this did not worh, due to white-space before/after the {/}. I have updated the unit test to cover this case and fixed it.

@leonardehrenfried leonardehrenfried left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the code and it does what it should. The code also looks good.

Comment thread docs/Configuration.md Outdated
Comment thread docs/Configuration.md Outdated
Comment thread src/main/java/org/opentripplanner/standalone/config/IncludeFileDirective.java Outdated
Co-authored-by: Hannes Junnila <hannes.junnila@gmail.com>
t2gran and others added 2 commits December 15, 2021 19:27
Co-authored-by: Hannes Junnila <hannes.junnila@gmail.com>
Allow the filename to contain dash as well
@t2gran
t2gran merged commit a759e17 into opentripplanner:dev-2.x Dec 16, 2021
@t2gran
t2gran deleted the otp2_include_file_directive branch December 16, 2021 15:53
t2gran pushed a commit that referenced this pull request Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Improvement A functional improvement or micro feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants