Skip to content

Commit 3c93b0c

Browse files
authored
Implement configuration flatbuffer parsing strategy (#1270)
Add ConfigurationFlatbufferParsingStrategy + Parse mw_com config from a FlatBuffers binary into the impl::Configuration model + Declare optional/@required scalars as `= null` in mw_com_config.fbs so the parser can tell a missing value from a default + Move the generated fbs types into the configuration::fbs namespace + Add unit tests and an optional-scalars example config + Patch the flatc-generated header to call `this->B()` so it compiles: flatc's `B` template parameter hides the field accessor `B()` in AllowedConsumer/AllowedProvider Common test to run on json configs + If flatbuffers flag is enabled, json_to_flatbuffer converter is generating config bins for all the possible testcases, and the common test is done on both + Ensures the integrity of the json_to_flatbuffer converter and the same results from both parsing mechanisms + Redundant tests removed - Edge cases covered
2 parents b936b36 + ac9253a commit 3c93b0c

129 files changed

Lines changed: 7576 additions & 5360 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎score/mw/com/impl/configuration/BUILD‎

Lines changed: 122 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@ load("@rules_cc//cc:defs.bzl", "cc_library")
1515
load("@rules_python//python:defs.bzl", "py_binary", "py_test")
1616
load("@score_baselibs//score/flatbuffers/bazel:codegen.bzl", "generate_cpp")
1717
load("@score_baselibs//score/flatbuffers/bazel:tools.bzl", "generate_json_schema")
18+
load("//bazel/rules:expand_template.bzl", "expand_template")
1819
load("//bazel/tools:json_schema_validator.bzl", "validate_json_schema_test")
1920
load("//quality/unit_testing:unit_testing.bzl", "cc_unit_test")
2021
load("//score/mw:common_features.bzl", "COMPILER_WARNING_FEATURES")
22+
load("//score/mw/com/impl/configuration/converter:json_to_flatbuffer_rule.bzl", "json_to_flatbuffer")
2123

2224
exports_files(
2325
[
@@ -43,6 +45,26 @@ generate_cpp(
4345
visibility = ["//score/mw/com/impl/configuration:__subpackages__"],
4446
)
4547

48+
# flatc's C++ generator hard-codes the bool verifier flag of every generated Verify() method as the
49+
# template parameter `B` (see @flatbuffers//:flatc). AllowedConsumer/AllowedProvider each declare a
50+
# scalar field literally named `B` (the ASIL-B branch of the allow-/deny-list), so the unqualified
51+
# call `B()` inside their generated Verify() resolves to that template parameter instead of the
52+
# accessor and fails to compile ("expression cannot be used as a function"). Qualifying with `this->`
53+
# forces member lookup and fixes it without touching semantics. This hermetic expand_template action
54+
# patches only that exact, narrow pattern in the generated header; consumers include this
55+
# file's output instead of :mw_com_config_fbs's directly.
56+
expand_template(
57+
name = "mw_com_config_generated_h",
58+
out = "mw_com_config_generated_patched.h",
59+
substitutions = {"VerifyVector(B())": "VerifyVector(this->B())"},
60+
target_compatible_with = select({
61+
"//score/mw/com/flags:flatbuffers_enabled": [],
62+
"//conditions:default": ["@platforms//:incompatible"],
63+
}),
64+
template = ":mw_com_config_fbs",
65+
visibility = ["//score/mw/com/impl/configuration:__subpackages__"],
66+
)
67+
4668
# Post-processed (but not yet hyphen-restored) rich JSON schema: baselibs' generate_json_schema
4769
# runs flatc --jsonschema and its schema_generator. generate_schema.py then only restores
4870
# the fbs "_" -> "-" mapping on this output to produce the checked-in mw_com_config_schema.json.
@@ -99,6 +121,16 @@ validate_json_schema_test(
99121
target_compatible_with = ["@platforms//os:linux"],
100122
)
101123

124+
# FlatBuffer binary fixture for configuration_flatbuffer_parsing_strategy_test's optional-scalar
125+
# coverage: an instance without instanceId (must parse to std::nullopt) and one with instanceId: 0
126+
# (a legal value, must parse to a present LolaServiceInstanceId{0}) -- the exact "absent" vs
127+
# "defaulted" distinction mw_com_config.fbs's "= null" fields (see the .fbs authoring rules) exist
128+
# for.
129+
json_to_flatbuffer(
130+
name = "mw_com_config_optional_scalars",
131+
json = "example/mw_com_config_optional_scalars.json",
132+
)
133+
102134
filegroup(
103135
name = "mw_com_config_schema",
104136
srcs = ["mw_com_config_schema.json"],
@@ -160,6 +192,50 @@ cc_library(
160192
],
161193
)
162194

195+
# ConfigurationFlatbufferParsingStrategy parses FlatBuffer-encoded (see mw_com_config.fbs) configuration files.
196+
# It is unconditionally buildable, but only functional when the (experimental) FlatBuffers configuration flag is
197+
# enabled: with the flag off, its Parse() overloads log a fatal error and terminate, and no FlatBuffers dependency
198+
# is linked in (see the two selects below).
199+
cc_library(
200+
name = "configuration_flatbuffer_parsing_strategy",
201+
srcs = ["configuration_flatbuffer_parsing_strategy.cpp"] + select({
202+
"//score/mw/com/flags:flatbuffers_enabled": [":mw_com_config_generated_h"],
203+
"//conditions:default": [],
204+
}),
205+
hdrs = ["configuration_flatbuffer_parsing_strategy.h"],
206+
features = COMPILER_WARNING_FEATURES,
207+
implementation_deps = [
208+
":config_validate",
209+
":lola_service_instance_deployment",
210+
":quality_type",
211+
":service_type_deployment",
212+
":someip_service_instance_deployment",
213+
":someip_service_type_deployment",
214+
"@score_baselibs//score/mw/log",
215+
"@score_baselibs//score/quality/compiler_warnings",
216+
] + select({
217+
"//score/mw/com/flags:flatbuffers_enabled": [
218+
"@score_baselibs//score/filesystem",
219+
"@score_baselibs//score/flatbuffers:flatbufferscpp",
220+
"@score_baselibs//score/flatbuffers:flatbufferutils",
221+
],
222+
"//conditions:default": [],
223+
}),
224+
local_defines = select({
225+
"//score/mw/com/flags:flatbuffers_enabled": ["SCORE_MW_COM_FLATBUFFERS_CONFIGURATION_ENABLED"],
226+
"//conditions:default": [],
227+
}),
228+
tags = ["FFI"],
229+
visibility = [
230+
"//score/mw/com/impl/configuration:__subpackages__",
231+
],
232+
deps = [
233+
":configuration_local",
234+
":i_configuration_parsing_strategy",
235+
"@score_baselibs//score/language/futurecpp",
236+
],
237+
)
238+
163239
cc_library(
164240
name = "config_validate",
165241
srcs = ["config_validate.cpp"],
@@ -681,20 +757,63 @@ cc_unit_test(
681757
],
682758
)
683759

760+
# Runs the shared tests with ConfigurationJsonParsingStrategy and, if the FlatBuffers configuration flag is enabled,
761+
# additionally with ConfigurationFlatbufferParsingStrategy.
684762
cc_unit_test(
685-
name = "configuration_json_parsing_strategy_test",
686-
srcs = ["configuration_json_parsing_strategy_test.cpp"],
687-
data = ["example/mw_com_config.json"],
763+
name = "configuration_parsing_strategy_test",
764+
srcs = ["configuration_parsing_strategy_test.cpp"],
765+
data = [
766+
"example/mw_com_config.json",
767+
"//score/mw/com/impl/configuration/test/parsing_strategy_configs:test_configs_json",
768+
] + select({
769+
"//score/mw/com/flags:flatbuffers_enabled": [
770+
"//score/mw/com/impl/configuration/converter:mw_com_config",
771+
"//score/mw/com/impl/configuration/test/parsing_strategy_configs:test_configs_bin",
772+
],
773+
"//conditions:default": [],
774+
}),
688775
features = COMPILER_WARNING_FEATURES + [
689776
# These tests catch exceptions instead of using gtest EXPECT_DEATH so we disable aborts_upon_exception.
690777
"-aborts_upon_exception",
691778
],
779+
local_defines = select({
780+
"//score/mw/com/flags:flatbuffers_enabled": ["SCORE_MW_COM_FLATBUFFERS_CONFIGURATION_ENABLED"],
781+
"//conditions:default": [],
782+
}),
692783
deps = [
784+
":configuration_flatbuffer_parsing_strategy",
693785
":configuration_json_parsing_strategy",
694786
"@score_baselibs//score/language/futurecpp:futurecpp_test_support",
695787
],
696788
)
697789

790+
# Only buildable when the FlatBuffers configuration flag is enabled: ConfigurationFlatbufferParsingStrategy is
791+
# unimplemented (LogFatal-and-abort) otherwise, so there is nothing meaningful to test.
792+
cc_unit_test(
793+
name = "configuration_flatbuffer_parsing_strategy_test",
794+
srcs = [
795+
"configuration_flatbuffer_parsing_strategy_test.cpp",
796+
":mw_com_config_generated_h",
797+
],
798+
data = [
799+
":mw_com_config_optional_scalars",
800+
"//score/mw/com/impl/configuration/converter:mw_com_config",
801+
],
802+
features = COMPILER_WARNING_FEATURES + [
803+
# These tests catch exceptions instead of using gtest EXPECT_DEATH so we disable aborts_upon_exception.
804+
"-aborts_upon_exception",
805+
],
806+
target_compatible_with = select({
807+
"//score/mw/com/flags:flatbuffers_enabled": [],
808+
"//conditions:default": ["@platforms//:incompatible"],
809+
}),
810+
deps = [
811+
":configuration_flatbuffer_parsing_strategy",
812+
"@score_baselibs//score/flatbuffers:flatbufferscpp",
813+
"@score_baselibs//score/language/futurecpp:futurecpp_test_support",
814+
],
815+
)
816+
698817
cc_unit_test(
699818
name = "config_validate_test",
700819
srcs = ["config_validate_test.cpp"],

‎score/mw/com/impl/configuration/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## mw::com Configuration Concept
44

55
Following document contains a short documentation, how to configure a `mw::com` application.
6+
If you want to extend or change the configuration schema itself, see [SCHEMA_UPDATE.md](./SCHEMA_UPDATE.md).
67

78
### Central JSON File per App
89

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
# Extending the mw::com configuration schema
2+
3+
This document explains how to add or change a configuration property. For what the properties *mean*, see
4+
[README.md](./README.md).
5+
6+
## Single source of truth
7+
8+
[mw_com_config.fbs](./mw_com_config.fbs) is the single source of truth for the configuration schema. Everything else
9+
is derived from it:
10+
11+
| Artifact | Derived how |
12+
|-------------------------------------------------------------|-----------------------------------------------------------------------------|
13+
| [mw_com_config_schema.json](./mw_com_config_schema.json) | `flatc --jsonschema` + baselibs' `schema_generator` + [generate_schema.py](./generate_schema.py) (checked in) |
14+
| `mw_com_config_generated(_patched).h` | `flatc --cpp` at build time (only with `--config=flatbuffers`) |
15+
| `<config>.bin` FlatBuffer binaries | [converter/json_to_flatbuffer.py](./converter/json_to_flatbuffer.py) + `flatc --binary` at build time |
16+
17+
Never edit `mw_com_config_schema.json` by hand. `schema_drift_test` fails if it is not byte-identical to what is
18+
generated from the `.fbs`.
19+
20+
## Workflow
21+
22+
1. **Edit `mw_com_config.fbs`.** Follow the authoring rules in its header and the rules below.
23+
2. **Regenerate the JSON schema** and commit it together with the `.fbs`:
24+
```sh
25+
bazel run //score/mw/com/impl/configuration:generate_schema
26+
```
27+
Check the `git diff` of `mw_com_config_schema.json`: it should contain exactly your change.
28+
3. **Extend both parsing strategies.** They are independent implementations producing the same `Configuration`:
29+
- [configuration_json_parsing_strategy.cpp](./configuration_json_parsing_strategy.cpp) reads the public JSON
30+
(hyphenated keys, defaults applied in C++).
31+
- [configuration_flatbuffer_parsing_strategy.cpp](./configuration_flatbuffer_parsing_strategy.cpp) reads the
32+
generated FlatBuffer accessors (underscore names). Use `ToOptional`, `RequireValue`, `RequireNotNull` and
33+
`NarrowOrFatal` for optional, required and narrowed values.
34+
4. **Add tests** (see [Tests](#tests)) and, if useful, extend [example/mw_com_config.json](./example/mw_com_config.json).
35+
5. **Document the property** in [README.md](./README.md).
36+
6. **Run the tests in both build configurations** (see [Build flag](#build-flag)):
37+
```sh
38+
bazel test //score/mw/com/impl/configuration/...
39+
bazel test --config=flatbuffers //score/mw/com/impl/configuration/...
40+
```
41+
42+
## Rules for the `.fbs`
43+
44+
### `-` vs. `_` in names
45+
46+
FlatBuffers identifiers cannot contain `-`, so a hyphenated public JSON key or enum value is written with `_` in the
47+
`.fbs` (`asil-level` -> `asil_level`, `file-permissions-on-empty` -> `file_permissions_on_empty`). The tooling maps
48+
this generically, no name is hardcoded:
49+
50+
- `generate_schema.py` turns **every** `_` back into `-` in property keys, `required` entries, enum values and
51+
enum-valued defaults.
52+
- `json_to_flatbuffer.py` turns every `-` into `_` in JSON keys, and in string values whose underscore form is an enum
53+
symbol, before handing the JSON to `flatc --binary`.
54+
55+
Consequences:
56+
57+
- A public JSON key or enum value **cannot contain `_`**. It would come out with `-` in the schema.
58+
- Keep camelCase keys exactly as they are in the JSON (`instanceSpecifier`, `eventId`, ...). Renaming one in the
59+
`.fbs` renames the public JSON key.
60+
- Field names `B`/`QM` (in `AllowedConsumer`/`AllowedProvider`) are why the generated header is patched
61+
(`VerifyVector(B())` -> `VerifyVector(this->B())`, see the `mw_com_config_generated_h` target in [BUILD](./BUILD)).
62+
Avoid adding further fields with names that clash with template parameters of the generated code.
63+
64+
> **Outlook:** A future version 2 of the configuration format will drop hyphens and camelCase from the JSON keys.
65+
> The keys will then match the `.fbs` field names directly, and the `-` <-> `_` conversion steps
66+
> (`generate_schema.py` hyphen restoration, `json_to_flatbuffer.py` normalization) will no longer be needed.
67+
68+
### Schema metadata via doc comments
69+
70+
What `flatc --jsonschema` cannot derive is written as `///` tokens **before** the description lines:
71+
72+
```
73+
/// @title: Service Field Getter
74+
/// @default: false
75+
/// @min: 0
76+
/// @max: 255
77+
/// @required
78+
/// @shared: serviceVersion (table-level: emitted once in $defs and referenced)
79+
/// Free-form description, one or more lines.
80+
```
81+
82+
`@default` is schema metadata only. If a property has a default, keep three places consistent: the `@default` token,
83+
the FlatBuffers default (`field: bool = true;`) and the default the JSON strategy applies in C++.
84+
85+
### Optional and required scalars (`= null`)
86+
87+
A plain FlatBuffers scalar can't distinguish "absent" from "default value": an omitted key reads back as the default
88+
(`0`, `false`, the first enum value). Therefore:
89+
90+
- **Optional by design** scalars, where absent and `0` mean different things (e.g. `instanceId` may be `0` or missing),
91+
are declared `field: T = null;`. flatc then generates a `flatbuffers::Optional<T>` accessor; convert it with
92+
`ToOptional`.
93+
- **`@required`** scalars must also be declared `= null`. FlatBuffers can't mark scalars as required, so without
94+
`= null` a missing key would silently read back as the default. With `= null` the parser detects the missing key via
95+
`RequireValue` and terminates.
96+
- Scalars with a real default (e.g. `enforceMaxSamples: bool = true;`) do **not** use `= null`.
97+
98+
`= null` doesn't show up in the JSON schema, because an omitted JSON key already means "absent".
99+
`generate_json_schema` strips it before running `flatc --jsonschema`.
100+
The optional-scalar behaviour of the FlatBuffer strategy is covered by `configuration_flatbuffer_parsing_strategy_test`
101+
with [example/mw_com_config_optional_scalars.json](./example/mw_com_config_optional_scalars.json).
102+
103+
### Unknown keys
104+
105+
`flatc --binary` rejects JSON keys that are not in the `.fbs`. A new property must therefore be added to the `.fbs`
106+
before any configuration that converts to a FlatBuffer can use it.
107+
108+
## Build flag
109+
110+
The FlatBuffer configuration path is experimental and gated by
111+
`//score/mw/com/flags:experimental_enable_flatbuffers_configuration` (default `False`). Enable it with
112+
`--config=flatbuffers` (see `.bazelrc`).
113+
114+
| Without the flag (default) | With `--config=flatbuffers` |
115+
|----------------------------------------------------------------------------------|-------------------------------------------------------------------|
116+
| No FlatBuffers dependency is linked; `ConfigurationFlatbufferParsingStrategy::Parse()` logs a fatal error and terminates | `SCORE_MW_COM_FLATBUFFERS_CONFIGURATION_ENABLED` is defined and the strategy is implemented |
117+
| `configuration_parsing_strategy_test` runs the JSON strategy only | It runs every parametrised test for JSON **and** FlatBuffer |
118+
| FlatBuffer-only targets (`mw_com_config_fbs`, `configuration_flatbuffer_parsing_strategy_test`, ...) are incompatible and skipped | They are built and run |
119+
120+
Not gated by the flag: `generate_schema`, `schema_drift_test` and the `json_to_flatbuffer` conversion of the test
121+
configurations. These rules only need `flatc` as a build tool.
122+
123+
Because a plain `bazel test` silently skips the FlatBuffer side, always run the tests **with and without**
124+
`--config=flatbuffers` after a schema change.
125+
126+
## Tests
127+
128+
Parser test configurations live in [test/parsing_strategy_configs](./test/parsing_strategy_configs), one file per
129+
case, and are registered in its [BUILD](./test/parsing_strategy_configs/BUILD) file in one of two lists.
130+
131+
### `SHARED_CONFIGS`: tested with both strategies
132+
133+
Every `<name>.json` is also converted to `<name>.bin` at build time via `json_to_flatbuffer`. The parametrised suites
134+
in [configuration_parsing_strategy_test.cpp](./configuration_parsing_strategy_test.cpp)
135+
(`ConfigurationParsingStrategyTest` & co., instantiated with `kStrategies`) parse the `.json` with the JSON strategy
136+
and the `.bin` with the FlatBuffer strategy, then check that both produce the same result or both terminate. This is
137+
the default: one input, two parsers, one expectation.
138+
139+
Use it for every configuration that flatc can convert. This includes *semantically* invalid configurations (missing
140+
required keys, duplicates, inconsistent values), because those must be rejected by both parsers.
141+
142+
### `FLATC_REJECTED_CONFIGS`: tested with the JSON strategy only
143+
144+
Some invalid configurations can't be converted by flatc at all: unknown keys, wrong JSON types, unknown enum
145+
symbols, numbers outside the type's range. No `.bin` can be built for them, so the FlatBuffer strategy never sees such
146+
input. They are covered as follows:
147+
148+
- **JSON path:** the JSON strategy must reject them at runtime. They are tested in
149+
`ConfigurationJsonOnlyParsingTest` (plain `TEST_F`, no strategy parameter, using `ParseJson()`).
150+
- **FlatBuffer path:** a `<name>_schema_rejects` test (`validate_json_schema_test` with `expected_failure = True`)
151+
checks that the JSON schema generated from the `.fbs` rejects the config.
152+
153+
The schema test stands in for "flatc rejects it" because flatc is only used through baselibs *build* rules. A
154+
rejected input fails the build action, and Bazel can't turn a failing action into a passing test.
155+
`rules_build_error` only supports C++ targets. A TODO in the BUILD file tracks replacing these schema tests with
156+
build-failure tests once that is possible.
157+
158+
The schema is **stricter** than flatc (required keys, `@min`/`@max`). The schema rejecting a config therefore
159+
doesn't mean flatc rejects it. Before putting a config into `FLATC_REJECTED_CONFIGS`, make sure `json_to_flatbuffer`
160+
really fails to build it. Otherwise it belongs in `SHARED_CONFIGS`, so that the FlatBuffer strategy's runtime
161+
checks are tested too.
162+
163+
### Adding a test
164+
165+
1. Add `test/parsing_strategy_configs/<name>.json`.
166+
2. Add `<name>` to `SHARED_CONFIGS` or, only if flatc can't convert it, to `FLATC_REJECTED_CONFIGS`.
167+
3. Add a `TEST_P(ConfigurationParsingStrategyTest, ...)` using `Parse(GetParam(), "<name>")`, or for
168+
flatc-rejected configs a `TEST_F(ConfigurationJsonOnlyParsingTest, ...)` using `ParseJson("<name>")`.

0 commit comments

Comments
 (0)