Skip to content

Commit ec5b8cb

Browse files
cassavacassava
committed
fable: Fix incorrect JSON schema output in some edge cases (WIP)
1 parent 3f0a62c commit ec5b8cb

14 files changed

+113
-25
lines changed

‎engine/src/stack.hpp

+1-15
Original file line numberDiff line numberDiff line change
@@ -635,32 +635,18 @@ struct ComponentConf : public Confable {
635635
{"binding", make_const_str(binding, "name of binding").require()},
636636
{"name", make_schema(&name, id_prototype(), "globally unique identifier for component")},
637637
{"from", Variant{
638+
make_schema(&from, "component inputs for binding"),
638639
CustomDeserializer(
639640
make_prototype<std::string>("component input for binding"),
640641
[this](CustomDeserializer*, const Conf& c) {
641642
this->from.push_back(c.get<std::string>());
642643
}
643644
),
644-
CustomDeserializer(
645-
make_prototype<std::vector<std::string>>("component inputs for binding"),
646-
[this](CustomDeserializer*, const Conf& c) {
647-
this->from = c.get<std::vector<std::string>>();
648-
}
649-
),
650645
}},
651646
{"args", make_schema(&args, factory->schema(), "factory-specific args")},
652647
};
653648
// clang-format on
654649
}
655-
656-
void to_json(Json& j) const override {
657-
j = Json{
658-
{"binding", binding},
659-
{"name", name},
660-
{"from", from},
661-
{"args", args},
662-
};
663-
}
664650
};
665651

666652
class ComponentSchema : public FactoryPlugin<ComponentConf, ComponentFactory> {

‎engine/tests/test_engine_json_schema.json

+15-3
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,11 @@
173173
{
174174
"additionalProperties": false,
175175
"properties": {
176-
"args": null,
176+
"args": {
177+
"additionalProperties": false,
178+
"properties": {},
179+
"type": "object"
180+
},
177181
"binding": {
178182
"const": "demo_printer",
179183
"description": "name of factory"
@@ -236,7 +240,11 @@
236240
{
237241
"additionalProperties": false,
238242
"properties": {
239-
"args": null,
243+
"args": {
244+
"additionalProperties": false,
245+
"properties": {},
246+
"type": "object"
247+
},
240248
"binding": {
241249
"const": "nop",
242250
"description": "name of factory"
@@ -1381,7 +1389,11 @@
13811389
{
13821390
"additionalProperties": false,
13831391
"properties": {
1384-
"args": null,
1392+
"args": {
1393+
"additionalProperties": false,
1394+
"properties": {},
1395+
"type": "object"
1396+
},
13851397
"binding": {
13861398
"const": "speedometer",
13871399
"description": "name of factory"

‎engine/tests/test_engine_nop_smoketest_dump.json

-1
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,6 @@
188188
"name": "noisy_object_sensor"
189189
},
190190
"cloe::speedometer": {
191-
"args": null,
192191
"binding": "speedometer",
193192
"from": [
194193
"cloe::gndtruth_ego_sensor"

‎engine/tests/test_engine_nop_smoketest_dump_with_vtd.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@
191191
"name": "noisy_object_sensor"
192192
},
193193
"cloe::speedometer": {
194-
"args": null,
194+
"args": {},
195195
"binding": "speedometer",
196196
"from": [
197197
"cloe::gndtruth_ego_sensor"

‎fable/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ if(BuildTests)
6161
src/fable/schema/enum_test.cpp
6262
src/fable/schema/factory_test.cpp
6363
src/fable/schema/factory_advanced_test.cpp
64+
src/fable/schema/ignore_test.cpp
6465
src/fable/schema/number_test.cpp
6566
src/fable/schema/optional_test.cpp
6667
src/fable/schema/struct_test.cpp

‎fable/include/fable/confable.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ class Confable {
144144
* object is created or moved, since Schema contains references to fields
145145
* that (should be) in the object.
146146
*/
147-
virtual Schema schema_impl() { return Schema(); }
147+
virtual Schema schema_impl() { return schema::Struct(); }
148148

149149
private:
150150
mutable std::unique_ptr<Schema> schema_{nullptr};

‎fable/include/fable/schema/ignore.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class Ignore : public Base<Ignore> {
5050

5151
public: // Overrides
5252
Json json_schema() const override {
53-
Json j;
53+
Json j = Json::object({});
5454
this->augment_schema(j);
5555
return j;
5656
}

‎fable/include/fable/utility/gtest.hpp

+8
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,18 @@ inline void assert_eq(const Json& j, const Json& k) {
3838
ASSERT_EQ(std::string(j.dump(2)), std::string(k.dump(2)));
3939
}
4040

41+
inline void assert_eq(const Json& j, const char expect[]) {
42+
assert_eq(j, parse_json(expect));
43+
}
44+
4145
inline void assert_ne(const Json& j, const Json& k) {
4246
ASSERT_NE(std::string(j.dump(2)), std::string(k.dump(2)));
4347
}
4448

49+
inline void assert_ne(const Json& j, const char expect[]) {
50+
assert_ne(j, parse_json(expect));
51+
}
52+
4553
inline void assert_schema_eq(const Confable& x, const Json& expect) {
4654
assert_eq(x.schema().json_schema(), expect);
4755
}

‎fable/src/fable/schema/factory_test.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ TEST(fable_schema_factory, schema) {
8585
"properties": {
8686
"args": {
8787
"additionalProperties": false,
88-
"properties": null,
88+
"properties": {},
8989
"type": "object"
9090
},
9191
"factory": {

‎fable/src/fable/schema/ignore_test.cpp

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
* Copyright 2022 Robert Bosch GmbH
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
*/
18+
/**
19+
* \file fable/schema/ignore_test.cpp
20+
* \see fable/schema/ignore.hpp
21+
*/
22+
23+
#include <gtest/gtest.h> // for TEST
24+
25+
#include <fable/confable.hpp> // for Confable
26+
#include <fable/schema.hpp> // for Ignore
27+
#include <fable/utility/gtest.hpp> // for assert_validate
28+
29+
namespace {
30+
31+
struct MyIgnoreStruct : public fable::Confable {
32+
CONFABLE_SCHEMA(MyIgnoreStruct) {
33+
using namespace fable::schema; // NOLINT(build/namespaces)
34+
return Struct{
35+
{"args", Ignore("validates with anything")},
36+
{"default", Ignore()},
37+
{"silent", Ignore("")},
38+
};
39+
}
40+
};
41+
42+
} // anonymous namespace
43+
44+
TEST(fable_schema_ignore, schema) {
45+
MyIgnoreStruct tmp;
46+
fable::assert_schema_eq(tmp, R"({
47+
"type": "object",
48+
"properties": {
49+
"args": {
50+
"description": "validates with anything"
51+
},
52+
"default": {
53+
"description": "ignored"
54+
},
55+
"silent": {}
56+
},
57+
"additionalProperties": false
58+
})");
59+
}

‎fable/src/fable/schema/struct.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ Json Struct::usage() const {
9090
}
9191

9292
Json Struct::json_schema() const {
93-
Json props;
93+
Json props = Json::object();
9494
for (const auto& kv : properties_) {
9595
props[kv.first] = kv.second.json_schema();
9696
}

‎fable/src/fable/schema/struct_test.cpp

+11
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,17 @@ struct StructFoo : public fable::Confable {
6363

6464
} // anonymous namespace
6565

66+
TEST(fable_schema_struct, empty_schema) {
67+
auto s = fable::schema::Struct();
68+
fable::assert_eq(s.json_schema(), R"({
69+
"additionalProperties": false,
70+
"properties": {},
71+
"type": "object"
72+
})");
73+
74+
fable::assert_eq(s.to_json(), "{}");
75+
}
76+
6677
TEST(fable_schema_struct, foo_schema) {
6778
StructFoo tmp;
6879
fable::assert_schema_eq(tmp, R"({

‎fable/src/fable/schema_test.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,16 @@ TEST(fable_schema, schema_wrapper) {
153153
fable::assert_eq(s1.json_schema(), s2.schema().json_schema());
154154
}
155155

156+
TEST(fable_schema, empty_schema) {
157+
using namespace fable; // NOLINT(build/namespaces)
158+
159+
auto s1 = Schema();
160+
auto s2 = Schema{};
161+
162+
fable::assert_eq(s1.json_schema(), "{}");
163+
fable::assert_eq(s2.json_schema(), "{}");
164+
}
165+
156166
TEST(fable_schema, json_schema) {
157167
MyStruct tmp;
158168
ASSERT_FALSE(tmp.my_required);

‎plugins/speedometer/src/speedometer.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@
2727
#include <cloe/vehicle.hpp> // for Vehicle
2828
using namespace cloe;
2929

30-
struct SpeedometerConf : public Confable {};
30+
struct SpeedometerConf : public Confable {
31+
CONFABLE_FRIENDS(SpeedometerConf)
32+
};
3133

3234
class Speedometer : public Component {
3335
public:

0 commit comments

Comments
 (0)