Skip to content

Commit de9d324

Browse files
cassavacassava
committed
fable: Fix unorthogonal interface of Struct schema
1 parent 902dfc9 commit de9d324

File tree

10 files changed

+611
-134
lines changed

10 files changed

+611
-134
lines changed

‎fable/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ if(BuildTests)
5353
src/fable/schema/enum_test.cpp
5454
src/fable/schema/number_test.cpp
5555
src/fable/schema/optional_test.cpp
56+
src/fable/schema/struct_test.cpp
5657
src/fable/schema_test.cpp
5758
)
5859
set_target_properties(test-fable PROPERTIES

‎fable/include/fable/schema.hpp

+24-11
Original file line numberDiff line numberDiff line change
@@ -157,33 +157,46 @@ using schema::make_const_str;
157157
using schema::make_prototype;
158158
using schema::make_schema;
159159

160+
/**
161+
* Define the automatically deduced schema class of a given type.
162+
*
163+
* \example
164+
*
165+
* using VecSchema = schema_type<std::vector<int64_t>>::type;
166+
*/
160167
template <typename T>
161168
struct schema_type {
162169
using type = decltype(make_schema(static_cast<T*>(nullptr), ""));
163170
};
164171

172+
/**
173+
* Schema is a wrapper class for fable schemas that automatically
174+
* chooses the correct underlying schema type.
175+
*
176+
* That is, this class provides an interface that doesn't require you to know
177+
* which class to use, but it doesn't cover all use-cases.
178+
*/
165179
class Schema : public schema::Interface {
166180
public:
167-
using SchemaMap = std::map<std::string, Schema>;
168-
using SchemaVec = std::vector<Schema>;
169-
170181
// Operators
171182
Schema(const Schema&) = default;
172183
Schema(Schema&&) = default;
173184
Schema& operator=(const Schema&) = default;
174185

175186
// Struct
176-
Schema(const SchemaMap& props); // NOLINT(runtime/explicit)
177-
Schema(std::string&& desc, const SchemaMap& props);
187+
Schema(std::string&& desc, schema::PropertyList<> props)
188+
: impl_(new schema::Struct(std::move(desc), props)) {}
189+
190+
Schema(schema::PropertyList<> props) : Schema("", props) {}
191+
192+
Schema(std::string&& desc, const Schema& base, schema::PropertyList<> props)
193+
: impl_(new schema::Struct(std::move(desc), base, props)) {}
178194

179-
Schema(schema::BoxPairList props); // NOLINT(runtime/explicit)
180-
Schema(std::string&& desc, schema::BoxPairList props);
181-
Schema(const Schema& base, schema::BoxPairList props);
182-
Schema(std::string&& desc, const Schema& base, schema::BoxPairList props);
195+
Schema(const Schema& base, schema::PropertyList<> props) : Schema("", base, props) {}
183196

184197
// Variant
185-
Schema(const SchemaVec& xs); // NOLINT(runtime/explicit)
186-
Schema(std::string&& desc, const SchemaVec& xs);
198+
Schema(const std::vector<Schema>& xs); // NOLINT(runtime/explicit)
199+
Schema(std::string&& desc, const std::vector<Schema>& xs);
187200

188201
Schema(schema::BoxList props); // NOLINT(runtime/explicit)
189202
Schema(std::string&& desc, schema::BoxList props);

‎fable/include/fable/schema/interface.hpp

+42-1
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,49 @@ class Box : public Interface {
259259
Box(std::shared_ptr<Interface> i) : impl_(std::move(i)) { assert(impl_); } // NOLINT
260260

261261
public: // Special
262+
/**
263+
* Return this type as a pointer to T.
264+
*
265+
* This method can be used like so:
266+
*
267+
* // In this example, we want a Struct.
268+
* auto ptr = s.template as<Struct>();
269+
*
270+
*/
271+
template <typename T>
272+
std::shared_ptr<T> as() const {
273+
assert(impl_ != nullptr);
274+
auto downcast_ptr = std::dynamic_pointer_cast<T>(impl_);
275+
if (downcast_ptr == nullptr) {
276+
// If you can't figure out the type of this even from this error, set
277+
// a breakpoint here in gdb and run:
278+
//
279+
// p dynamic_cast<Interface*>(impl_.get())
280+
//
281+
// That will give you something like:
282+
//
283+
// $8 = (fable::schema::Interface *) 0x5555559dfd30 <
284+
// vtable for fable::schema::FromConfable<
285+
// (anonymous namespace)::NormalDistribution<double>, 0
286+
// >
287+
// +16>
288+
//
289+
// Which is much more specific, since the output of FromConfable and
290+
// Struct can be equivalent, for example.
291+
throw SchemaError{Conf{}, this->json_schema(),
292+
"cannot dynamic_pointer_cast to type T, got nullptr"};
293+
}
294+
return downcast_ptr;
295+
}
296+
297+
/**
298+
* Return this type as a pointer to T.
299+
*
300+
* This is unsafe in that you need to check the resulting shared pointer
301+
* yourself for whether the dynamic cast was successful or not.
302+
*/
262303
template <typename T>
263-
std::shared_ptr<const T> as() const {
304+
std::shared_ptr<T> as_unsafe() const {
264305
return std::dynamic_pointer_cast<T>(impl_);
265306
}
266307

‎fable/include/fable/schema/struct.hpp

+117-36
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,27 @@
3737
namespace fable {
3838
namespace schema {
3939

40-
using BoxPairList = std::initializer_list<std::pair<std::string const, Box>>;
41-
using BoxMap = std::map<std::string, Box>;
40+
/**
41+
* PropertyList is mainly used in constructors to enable the use of initializer
42+
* list.
43+
*
44+
* \example
45+
* Given the following constructor declaration:
46+
*
47+
* Struct(PropertyList<> props);
48+
*
49+
* Struct can be instantiated then like so:
50+
*
51+
* return Struct{
52+
* {"prop_a", make_schema(...)},
53+
* {"prop_b", make_schema(...)},
54+
* };
55+
*/
56+
template <typename S = Box>
57+
using PropertyList = std::initializer_list<std::pair<std::string const, S>>;
4258

43-
template <typename T>
44-
using is_properties_t =
45-
std::enable_if_t<std::is_same<BoxPairList, T>::value || std::is_same<BoxMap, T>::value, int>;
59+
template <typename T, typename S = Box>
60+
using enable_if_property_list_t = std::enable_if_t<std::is_same<PropertyList<S>, T>::value>;
4661

4762
/**
4863
* Struct maintains a key-value mapping, where the list of keys is usually
@@ -53,53 +68,111 @@ using is_properties_t =
5368
*
5469
* This should not be confused with the Map type.
5570
*
56-
* \see fable/schema/map.hpp
71+
* \see fable/schema/map.hpp
5772
*/
5873
class Struct : public Base<Struct> {
5974
public: // Constructors
6075
explicit Struct(std::string&& desc = "") : Base(JsonType::object, std::move(desc)) {}
6176

62-
Struct(std::string&& desc, BoxPairList props);
63-
Struct(std::string&& desc, BoxMap&& props);
64-
Struct(std::string&& desc, const Struct& base, BoxPairList props);
65-
Struct(std::string&& desc, const Struct& base, BoxMap&& props);
66-
Struct(std::string&& desc, const Box& base, BoxPairList props);
67-
Struct(std::string&& desc, const Box& base, BoxMap&& props);
77+
Struct(std::string&& desc, PropertyList<Box> props) : Base(JsonType::object, std::move(desc)) {
78+
set_properties(props);
79+
}
6880

69-
Struct(BoxPairList props) : Struct("", std::move(props)) {} // NOLINT
70-
Struct(BoxMap&& props) : Struct("", std::move(props)) {} // NOLINT
71-
Struct(const Struct& base, BoxPairList props) : Struct("", base, std::move(props)) {}
72-
Struct(const Struct& base, BoxMap&& props) : Struct("", base, std::move(props)) {}
73-
Struct(const Box& base, BoxPairList props) : Struct("", base, std::move(props)) {}
74-
Struct(const Box& base, BoxMap&& props) : Struct("", base, std::move(props)) {}
81+
Struct(PropertyList<Box> props) : Struct("", props) {} // NOLINT
82+
83+
// Inheriting constructors
84+
/**
85+
* Instantiate a Struct with a base Schema, which should also be a Struct,
86+
* then extend it with the property list.
87+
*
88+
* This is particularly useful when the Confable is inheriting from a base
89+
* class:
90+
*
91+
* struct Sub : public Base {
92+
* std::string member;
93+
* CONFABLE_SCHEMA(Sub) {
94+
* return Struct{
95+
* Base::schema_impl(),
96+
* {
97+
* {"member", make_schema(&member, "important addition")},
98+
* }
99+
* };
100+
* }
101+
* };
102+
*
103+
* Warning: When implementing schema_impl, for example with CONFABLE_SCHEMA,
104+
* it is absolutely important that you _not_ call Base::schema(), as this
105+
* will internally call this->schema_impl(), which will lead to an
106+
* infinite recursion! Instead, call Base::schema_impl().
107+
*/
108+
Struct(std::string&& desc, const Box& base, PropertyList<Box> props)
109+
: Struct(*base.template as<Struct>()) {
110+
desc_ = std::move(desc);
111+
set_properties(props);
112+
}
113+
114+
Struct(const Box& base, PropertyList<Box> props) : Struct("", base, props) {}
75115

76116
public: // Special
77117
/**
78118
* Set the property to this schema.
79119
*
80120
* - This overwrites any already existing field of the same key.
81-
* - Meant to be used during construction.
82121
*/
83-
Struct property(std::string&& key, Box&& s) &&;
84-
Struct property(const std::string& key, Box&& s) &&;
85-
void set_property(const std::string& key, const Box& s);
122+
void set_property(const std::string& key, Box&& s);
123+
124+
Struct property(const std::string& key, Box&& s) && {
125+
set_property(key, std::move(s));
126+
return std::move(*this);
127+
}
86128

87129
/**
88-
* Set whether all entries are required.
130+
* Set all properties.
89131
*
90-
* - The default is false.
91-
* - Meant to be used during construction.
132+
* - This overwrites any already existing property of the same key.
92133
*/
93-
Struct require_all() &&;
134+
void set_properties(PropertyList<Box> props);
94135

95136
/**
96-
* Set which entries are required.
137+
* Add the properties from s to this schema.
138+
*
139+
* - This will overwrite any existing properties.
97140
*/
98-
Struct require(std::initializer_list<std::string> init) {
99-
properties_required_ = init;
141+
void set_properties_from(const Struct& s) { set_properties(s.properties_); }
142+
143+
void set_properties_from(const Box& s) { set_properties_from(*s.template as<Struct>()); }
144+
145+
template <typename T, typename = enable_if_confable_t<T>>
146+
void set_properties_from(const T* x) {
147+
set_properties_from(x->schema());
148+
}
149+
150+
template <typename T>
151+
Struct properties_from(const T x) {
152+
set_properties_from(x);
100153
return std::move(*this);
101154
}
102155

156+
/**
157+
* Set which entries are required.
158+
*
159+
* - Complexity: O(n*m) with n the number of current properties and
160+
* m the number of properties in init.
161+
*/
162+
void set_require(std::initializer_list<std::string> init);
163+
Struct require(std::initializer_list<std::string> init) &&;
164+
165+
/**
166+
* Set whether all entries are required.
167+
*
168+
* - The default is false.
169+
* - Meant to be used during construction.
170+
* - This will only act on properties that exist at the time that this is
171+
* called.
172+
*/
173+
void set_require_all();
174+
Struct require_all() &&;
175+
103176
/**
104177
* Set whether to tolerate unknown fields in this entry.
105178
*
@@ -111,11 +184,16 @@ class Struct : public Base<Struct> {
111184
return std::move(*this);
112185
}
113186

114-
template <typename T, std::enable_if_t<std::is_base_of<Interface, T>::value, int> = 0>
115-
Struct additional_properties(const T& s) && {
187+
template <typename S, typename = enable_if_schema_t<S>>
188+
void set_additional_properties(const S& s) {
116189
additional_properties_ = true;
117-
additional_prototype_ = s.clone();
190+
additional_prototype_.reset(s.clone());
118191
additional_prototype_->reset_ptr();
192+
}
193+
194+
template <typename S, typename = enable_if_schema_t<S>>
195+
Struct additional_properties(const S& s) && {
196+
set_additional_properties(s);
119197
return std::move(*this);
120198
}
121199

@@ -130,29 +208,32 @@ class Struct : public Base<Struct> {
130208
void to_json(Json& j) const override;
131209
void from_conf(const Conf& c) override;
132210

211+
private:
212+
void set_properties(const std::map<std::string, Box>& props);
213+
133214
private:
134215
std::map<std::string, Box> properties_{};
135216
std::vector<std::string> properties_required_{};
136217
std::shared_ptr<Interface> additional_prototype_{};
137218
bool additional_properties_{false};
138219
};
139220

140-
template <typename T, is_properties_t<T> = 0>
221+
template <typename T, typename = enable_if_property_list_t<T>>
141222
inline Struct make_schema(T&& props) {
142223
return Struct(std::forward<T>(props));
143224
}
144225

145-
template <typename T, is_properties_t<T> = 0>
226+
template <typename T, typename = enable_if_property_list_t<T>>
146227
inline Struct make_schema(std::string&& desc, T&& props) {
147228
return Struct(std::move(desc), std::forward<T>(props));
148229
}
149230

150-
template <typename T, is_properties_t<T> = 0>
231+
template <typename T, typename = enable_if_property_list_t<T>>
151232
inline Struct make_schema(std::string&& desc, const Box& base, T&& props) {
152233
return Struct(std::move(desc), base, std::forward<T>(props));
153234
}
154235

155-
template <typename T, is_properties_t<T> = 0>
236+
template <typename T, typename = enable_if_property_list_t<T>>
156237
inline Struct make_schema(std::string&& desc, const Struct& base, T&& props) {
157238
return Struct(std::move(desc), base, std::forward<T>(props));
158239
}

‎fable/src/fable/schema.cpp

+3-29
Original file line numberDiff line numberDiff line change
@@ -26,28 +26,13 @@
2626

2727
namespace fable {
2828

29-
using schema::BoxMap;
30-
using schema::BoxPairList;
31-
using schema::Struct;
32-
3329
using schema::BoxList;
3430
using schema::BoxVec;
3531
using schema::Variant;
3632

37-
using SchemaMap = Schema::SchemaMap;
38-
using SchemaVec = Schema::SchemaVec;
39-
4033
namespace {
4134

42-
BoxMap to_box_map(const SchemaMap& m) {
43-
BoxMap out;
44-
for (const auto& kv : m) {
45-
out.insert(std::make_pair(kv.first, kv.second));
46-
}
47-
return out;
48-
}
49-
50-
BoxVec to_box_vec(const SchemaVec& xs) {
35+
BoxVec to_box_vec(const std::vector<Schema>& xs) {
5136
BoxVec out;
5237
out.reserve(xs.size());
5338
for (const auto& x : xs) {
@@ -58,20 +43,9 @@ BoxVec to_box_vec(const SchemaVec& xs) {
5843

5944
} // anonymous namespace
6045

61-
// Struct constructions:
62-
Schema::Schema(const SchemaMap& props) : Schema("", std::move(props)) {}
63-
Schema::Schema(std::string&& desc, const SchemaMap& props)
64-
: impl_(new Struct(std::move(desc), to_box_map(props))) {}
65-
Schema::Schema(BoxPairList props) : Schema("", std::move(props)) {}
66-
Schema::Schema(std::string&& desc, BoxPairList props)
67-
: impl_(new Struct(std::move(desc), std::move(props))) {}
68-
Schema::Schema(const Schema& base, BoxPairList props) : Schema("", base, std::move(props)) {}
69-
Schema::Schema(std::string&& desc, const Schema& base, BoxPairList props)
70-
: impl_(new Struct(std::move(desc), base, std::move(props))) {}
71-
7246
// Variant constructions:
73-
Schema::Schema(const SchemaVec& xs) : impl_(new Variant(to_box_vec(xs))) {}
74-
Schema::Schema(std::string&& desc, const SchemaVec& xs)
47+
Schema::Schema(const std::vector<Schema>& xs) : impl_(new Variant(to_box_vec(xs))) {}
48+
Schema::Schema(std::string&& desc, const std::vector<Schema>& xs)
7549
: impl_(new Variant(std::move(desc), to_box_vec(xs))) {}
7650
Schema::Schema(schema::BoxList props) : impl_(new Variant(props)) {}
7751
Schema::Schema(std::string&& desc, schema::BoxList props)

0 commit comments

Comments
 (0)