Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 162 additions & 2 deletions src/lisflood/settings_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@
from __future__ import absolute_import

import argparse
import re
import sys

import yaml
from lxml import etree

# Built-in path variables that LISFLOOD provides automatically.
_BUILTIN_VARS = frozenset({
"SettingsPath", "SettingsDir", "ProjectPath", "ProjectDir",
})


class SettingsToolError(Exception):
"""Raised when the settings tool cannot complete requested operations."""
Expand Down Expand Up @@ -75,6 +81,14 @@ def _flatten_pairs(raw_values):
return flattened


_VAR_RE = re.compile(r"\$\(([^)]+)\)")


def _find_refs(value):
"""Return set of variable names referenced via $(Name) in *value*."""
return set(_VAR_RE.findall(value))


def run_tool(input_path, output_path=None, file_path=None, lfoptions_values=None, lfuser_values=None, check=False):
parser = etree.XMLParser(remove_blank_text=True, remove_comments=False)
tree = etree.parse(input_path, parser)
Expand All @@ -87,7 +101,7 @@ def run_tool(input_path, output_path=None, file_path=None, lfoptions_values=None
_ = _get_required_child(root, ("lfbinding", "lfbindings"))

if check:
return 0
return _validate_refs(lfuser_elem, root)

if not output_path:
raise SettingsToolError("Output path is required for the 'set' subcommand.")
Expand Down Expand Up @@ -129,6 +143,141 @@ def run_tool(input_path, output_path=None, file_path=None, lfoptions_values=None
return 0


def _validate_refs(lfuser_elem, root):
"""Check that every $(Var) reference resolves to a defined lfuser name or a builtin."""
user_index = _index_textvars(lfuser_elem)
defined = set(user_index.keys()) | _BUILTIN_VARS
errors = []

lfbinding_elem = root.find("lfbinding")
if lfbinding_elem is None:
lfbinding_elem = root.find("lfbindings")

for section_tag, elem in [("lfuser", lfuser_elem), ("lfbinding", lfbinding_elem)]:
if elem is None:
continue
for node in elem.findall(".//textvar"):
name = node.get("name", "?")
value = node.get("value", "")
for ref in _find_refs(value):
if ref not in defined:
errors.append(
"{}: '{}' references undefined variable '$({})'.".format(
section_tag, name, ref
)
)

# Also scan lfoptions — they shouldn't contain $(refs) but flag if they do
lfoptions_elem = root.find("lfoptions")
if lfoptions_elem is not None:
for node in lfoptions_elem.findall(".//setoption"):
choice = node.get("choice", "")
for ref in _find_refs(choice):
errors.append(
"lfoptions: '{}' choice references variable '$({})'.".format(
node.get("name", "?"), ref
)
)

if errors:
for err in errors:
print(" " + err, file=sys.stderr)
raise SettingsToolError("{} undefined variable reference(s) found.".format(len(errors)))
return 0


def _parse_sections(path):
"""Parse an XML file and return (tree, root, lfoptions_elem, lfuser_elem, lfbinding_elem)."""
parser = etree.XMLParser(remove_blank_text=True, remove_comments=False)
tree = etree.parse(path, parser)
root = tree.getroot()
if root.tag != "lfsettings":
raise SettingsToolError("Root element must be <lfsettings>, found <{}>.".format(root.tag))
lfoptions_elem = _get_required_child(root, ("lfoptions",))
lfuser_elem = _get_required_child(root, ("lfuser",))
lfbinding_elem = _get_required_child(root, ("lfbinding", "lfbindings"))
return tree, root, lfoptions_elem, lfuser_elem, lfbinding_elem


def _shell_quote(value):
"""Wrap *value* in single quotes so it is safe to paste into a shell command."""
return "'" + value.replace("'", "'\"'\"'") + "'"


def _copy_path(path):
"""Return *path* with '_copy' inserted before the extension."""
import os
base, ext = os.path.splitext(path)
return base + "_copy" + ext


def run_diff(input_path, output_path):
"""Compare two LISFLOOD settings XMLs and print differences."""
_, _, opts_a, user_a, _ = _parse_sections(input_path)
_, _, opts_b, user_b, _ = _parse_sections(output_path)

idx_opts_a = {n.get("name"): n.get("choice") for n in opts_a.findall(".//setoption") if n.get("name")}
idx_opts_b = {n.get("name"): n.get("choice") for n in opts_b.findall(".//setoption") if n.get("name")}
idx_user_a = {n.get("name"): n.get("value") for n in user_a.findall(".//textvar") if n.get("name")}
idx_user_b = {n.get("name"): n.get("value") for n in user_b.findall(".//textvar") if n.get("name")}

opt_diffs = []
user_diffs = []

all_opt_keys = sorted(set(idx_opts_a) | set(idx_opts_b))
for key in all_opt_keys:
va = idx_opts_a.get(key)
vb = idx_opts_b.get(key)
if va != vb:
opt_diffs.append((key, va, vb))

all_user_keys = sorted(set(idx_user_a) | set(idx_user_b))
for key in all_user_keys:
va = idx_user_a.get(key)
vb = idx_user_b.get(key)
if va != vb:
user_diffs.append((key, va, vb))

if not opt_diffs and not user_diffs:
print("No differences found.")
return 0

if opt_diffs:
print("lfoptions:")
for name, va, vb in opt_diffs:
la = " (absent)" if va is None else va
lb = " (absent)" if vb is None else vb
print(" {}: {} -> {}".format(name, la, lb))

if user_diffs:
print("lfuser:")
for name, va, vb in user_diffs:
la = " (absent)" if va is None else va
lb = " (absent)" if vb is None else vb
print(" {}: {} -> {}".format(name, la, lb))

# Build the set command to go from input (-i) to output (-o)
out_copy = _copy_path(output_path)
cmd_parts = ["lisflood-settings set -i {} -o {}".format(
_shell_quote(input_path), _shell_quote(out_copy)
)]
set_opt_parts = []
set_user_parts = []
for name, _va, vb in opt_diffs:
if vb is not None:
set_opt_parts.append(_shell_quote("{}={}".format(name, vb)))
for name, _va, vb in user_diffs:
if vb is not None:
set_user_parts.append(_shell_quote("{}={}".format(name, vb)))
if set_opt_parts:
cmd_parts.append("--lfoptions " + " ".join(set_opt_parts))
if set_user_parts:
cmd_parts.append("--lfuser " + " ".join(set_user_parts))
print("\nEquivalent command (-i -> -o):")
print(" " + " ".join(cmd_parts))
return 0


def build_parser():
parser = argparse.ArgumentParser(
description="Parse, validate, lint and update LISFLOOD settings XML files."
Expand All @@ -137,10 +286,17 @@ def build_parser():

check_parser = subparsers.add_parser(
"check",
help="Validate XML structure/sections without writing output.",
help="Validate XML structure, sections, and variable references.",
)
check_parser.add_argument("-i", "--input", required=True, help="Input LISFLOOD settings XML file.")

diff_parser = subparsers.add_parser(
"diff",
help="Show differences between two settings XMLs.",
)
diff_parser.add_argument("-i", "--input", required=True, help="First (base) LISFLOOD settings XML file.")
diff_parser.add_argument("-o", "--output", required=True, help="Second (target) LISFLOOD settings XML file.")

set_parser = subparsers.add_parser(
"set",
help="Apply updates and write output XML.",
Expand Down Expand Up @@ -182,6 +338,10 @@ def main(argv=None):
run_tool(input_path=args.input, check=True)
return 0

if args.command == "diff":
run_diff(args.input, args.output)
return 0

lfoptions_values = _flatten_pairs(args.lfoptions)
lfuser_values = _flatten_pairs(args.lfuser)
run_tool(
Expand Down
133 changes: 132 additions & 1 deletion tests/test_settings_tool.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest
from lxml import etree

from lisflood.settings_tool import main
from lisflood.settings_tool import main, SettingsToolError


SAMPLE_XML = """\
Expand Down Expand Up @@ -30,6 +30,65 @@
</lfsettings>
"""

SAMPLE_BAD_REF_XML = """\
<lfsettings>
<lfoptions>
<setoption choice="0" name="wateruse"/>
</lfoptions>
<lfuser>
<group>
<textvar name="PathRoot" value="/data"/>
<textvar name="MaskMap" value="$(UndefinedVar)/mask.nc"/>
</group>
</lfuser>
<lfbinding>
<group>
<textvar name="MaskMap" value="$(MaskMap)"/>
</group>
</lfbinding>
</lfsettings>
"""

SAMPLE_BAD_BINDING_REF_XML = """\
<lfsettings>
<lfoptions>
<setoption choice="0" name="wateruse"/>
</lfoptions>
<lfuser>
<group>
<textvar name="PathRoot" value="/data"/>
</group>
</lfuser>
<lfbinding>
<group>
<textvar name="MaskMap" value="$(NoSuchVar)/mask.nc"/>
</group>
</lfbinding>
</lfsettings>
"""

SAMPLE_XML_B = """\
<lfsettings>
<lfoptions>
<setoption choice="1" name="TemperatureInKelvin"/>
<setoption choice="0" name="wateruse"/>
</lfoptions>
<lfuser>
<group>
<textvar name="PathRoot" value="/data/root"/>
<textvar name="SharedVar" value="/tmp/project/shared"/>
<textvar name="SomeUserVar" value="xyz"/>
</group>
</lfuser>
<lfbinding>
<group>
<textvar name="SharedVar" value="$(PathRoot)/shared"/>
<textvar name="MaskMap" value="$(PathRoot)/mask.nc"/>
</group>
</lfbinding>
</lfsettings>
"""


def _write(path, content):
path.write_text(content, encoding="utf-8")
Expand Down Expand Up @@ -164,3 +223,75 @@ def test_lfoptions_rejects_non_key_value_token(self, tmp_path):

with pytest.raises(SystemExit):
main(["set", "-i", str(src), "-o", str(dst), "--lfoptions", "wateruse=1", "not-a-pair"])

def test_check_validates_refs_ok(self, tmp_path):
src = tmp_path / "settings.xml"
_write(src, SAMPLE_XML)

rc = main(["check", "-i", str(src)])
assert rc == 0

def test_check_catches_undefined_lfuser_ref(self, tmp_path):
src = tmp_path / "bad.xml"
_write(src, SAMPLE_BAD_REF_XML)

rc = main(["check", "-i", str(src)])
assert rc == 1

def test_check_catches_undefined_lfbinding_ref(self, tmp_path):
src = tmp_path / "bad.xml"
_write(src, SAMPLE_BAD_BINDING_REF_XML)

rc = main(["check", "-i", str(src)])
assert rc == 1

def test_check_allows_builtin_vars(self, tmp_path):
xml = """\
<lfsettings>
<lfoptions/>
<lfuser>
<group>
<textvar name="PathRoot" value="$(SettingsPath)/../../root"/>
</group>
</lfuser>
<lfbinding>
<group>
<textvar name="MaskMap" value="$(PathRoot)/mask.nc"/>
</group>
</lfbinding>
</lfsettings>
"""
src = tmp_path / "builtin.xml"
_write(src, xml)

rc = main(["check", "-i", str(src)])
assert rc == 0

def test_diff_identical_files(self, tmp_path, capsys):
src = tmp_path / "a.xml"
_write(src, SAMPLE_XML)

rc = main(["diff", "-i", str(src), "-o", str(src)])
assert rc == 0
assert "No differences" in capsys.readouterr().out

def test_diff_shows_changes(self, tmp_path, capsys):
a = tmp_path / "a.xml"
b = tmp_path / "b.xml"
_write(a, SAMPLE_XML)
_write(b, SAMPLE_XML_B)

rc = main(["diff", "-i", str(a), "-o", str(b)])
assert rc == 0

out = capsys.readouterr().out
assert "TemperatureInKelvin" in out
assert "wateruse" in out
assert "PathRoot" in out
assert "SomeUserVar" in out
assert "Equivalent command" in out
# Generated command uses real paths with _copy suffix
assert str(a) in out
assert "b_copy.xml" in out
# Values are single-quoted for shell safety
assert "'PathRoot=/data/root'" in out