Skip to content

Commit 570e5a6

Browse files
brbzull0brbzull0
authored
records: Replace record registration macros with non macro solution. (#12636)
* records: Replace record registration macros with template tag dispatch Replaces REC_REGISTER_STAT_XXX and REC_REGISTER_CONFIG_XXX macros with template implementations using tag dispatch to properly distinguish RecInt from RecCounter (both int64_t aliases).
1 parent 1949b3b commit 570e5a6

File tree

5 files changed

+201
-93
lines changed

5 files changed

+201
-93
lines changed

‎src/records/CMakeLists.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ target_link_libraries(
4040
)
4141

4242
if(BUILD_TESTING)
43-
add_executable(test_records unit_tests/unit_test_main.cc unit_tests/test_RecHttp.cc)
44-
target_link_libraries(test_records PRIVATE records Catch2::Catch2 ts::tscore libswoc::libswoc)
43+
add_executable(test_records unit_tests/unit_test_main.cc unit_tests/test_RecHttp.cc unit_tests/test_RecRegister.cc)
44+
target_link_libraries(test_records PRIVATE records Catch2::Catch2 ts::tscore libswoc::libswoc ts::inkevent)
4545
add_catch2_test(NAME test_records COMMAND test_records)
4646
endif()
4747

‎src/records/P_RecCore.cc‎

Lines changed: 105 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -36,90 +36,158 @@
3636

3737
#include <fstream>
3838
#include <iterator>
39+
#include <type_traits>
3940

4041
//-------------------------------------------------------------------------
4142
// RecRegisterStatXXX
4243
//-------------------------------------------------------------------------
43-
#define REC_REGISTER_STAT_XXX(A, B) \
44-
ink_assert((rec_type == RECT_NODE) || (rec_type == RECT_PROCESS) || (rec_type == RECT_LOCAL) || (rec_type == RECT_PLUGIN)); \
45-
RecRecord *r; \
46-
RecData my_data_default; \
47-
my_data_default.A = data_default; \
48-
if ((r = RecRegisterStat(rec_type, name, B, my_data_default, persist_type)) != nullptr) { \
49-
if (i_am_the_record_owner(r->rec_type)) { \
50-
r->sync_required = r->sync_required | REC_PEER_SYNC_REQUIRED; \
51-
} \
52-
return REC_ERR_OKAY; \
53-
} else { \
54-
return REC_ERR_FAIL; \
44+
namespace
45+
{
46+
47+
template <typename> struct always_false : std::false_type {
48+
};
49+
50+
// Tag types for template dispatch - distinguishes RecInt from RecCounter
51+
// even though they're both int64_t type aliases.
52+
// Needed to make sure the data_type is set correctly for the given type.
53+
// without this, the data_type would be set to RECD_INT for both RecInt and RecCounter.
54+
namespace rec_detail
55+
{
56+
struct IntTag {
57+
using type = RecInt;
58+
static constexpr RecDataT data_type = RECD_INT;
59+
};
60+
struct CounterTag {
61+
using type = RecCounter;
62+
static constexpr RecDataT data_type = RECD_COUNTER;
63+
};
64+
struct FloatTag {
65+
using type = RecFloat;
66+
static constexpr RecDataT data_type = RECD_FLOAT;
67+
};
68+
struct StringTag {
69+
using type = RecString;
70+
static constexpr RecDataT data_type = RECD_STRING;
71+
};
72+
} // namespace rec_detail
73+
74+
template <typename Tag>
75+
RecErrT
76+
RecRegisterStatImpl(RecT rec_type, const char *name, typename Tag::type data_default, RecPersistT persist_type)
77+
{
78+
ink_assert((rec_type == RECT_NODE) || (rec_type == RECT_PROCESS) || (rec_type == RECT_LOCAL) || (rec_type == RECT_PLUGIN));
79+
80+
RecData my_data_default;
81+
82+
if constexpr (std::is_same_v<Tag, rec_detail::IntTag>) {
83+
my_data_default.rec_int = data_default;
84+
} else if constexpr (std::is_same_v<Tag, rec_detail::CounterTag>) {
85+
my_data_default.rec_counter = data_default;
86+
} else if constexpr (std::is_same_v<Tag, rec_detail::FloatTag>) {
87+
my_data_default.rec_float = data_default;
88+
} else {
89+
static_assert(always_false<Tag>::value, "Unsupported tag for RecRegisterStat");
90+
}
91+
92+
if (RecRecord *r = RecRegisterStat(rec_type, name, Tag::data_type, my_data_default, persist_type); r != nullptr) {
93+
if (i_am_the_record_owner(r->rec_type)) {
94+
r->sync_required = r->sync_required | REC_PEER_SYNC_REQUIRED;
95+
}
96+
return REC_ERR_OKAY;
5597
}
5698

99+
return REC_ERR_FAIL;
100+
}
101+
} // namespace
102+
57103
RecErrT
58104
_RecRegisterStatInt(RecT rec_type, const char *name, RecInt data_default, RecPersistT persist_type)
59105
{
60-
REC_REGISTER_STAT_XXX(rec_int, RECD_INT);
106+
return RecRegisterStatImpl<rec_detail::IntTag>(rec_type, name, data_default, persist_type);
61107
}
62108

63109
RecErrT
64110
_RecRegisterStatFloat(RecT rec_type, const char *name, RecFloat data_default, RecPersistT persist_type)
65111
{
66-
REC_REGISTER_STAT_XXX(rec_float, RECD_FLOAT);
112+
return RecRegisterStatImpl<rec_detail::FloatTag>(rec_type, name, data_default, persist_type);
67113
}
68114

69115
RecErrT
70116
_RecRegisterStatCounter(RecT rec_type, const char *name, RecCounter data_default, RecPersistT persist_type)
71117
{
72-
REC_REGISTER_STAT_XXX(rec_counter, RECD_COUNTER);
118+
return RecRegisterStatImpl<rec_detail::CounterTag>(rec_type, name, data_default, persist_type);
73119
}
74120

75121
//-------------------------------------------------------------------------
76122
// RecRegisterConfigXXX
77123
//-------------------------------------------------------------------------
78-
#define REC_REGISTER_CONFIG_XXX(A, B) \
79-
RecRecord *r; \
80-
RecData my_data_default; \
81-
my_data_default.A = data_default; \
82-
if ((r = RecRegisterConfig(rec_type, name, B, my_data_default, update_type, check_type, check_regex, source, access_type)) != \
83-
nullptr) { \
84-
if (i_am_the_record_owner(r->rec_type)) { \
85-
r->sync_required = r->sync_required | REC_PEER_SYNC_REQUIRED; \
86-
} \
87-
return REC_ERR_OKAY; \
88-
} else { \
89-
return REC_ERR_FAIL; \
124+
namespace
125+
{
126+
127+
template <typename Tag>
128+
RecErrT
129+
RecRegisterConfigImpl(RecT rec_type, const char *name, typename Tag::type data_default, RecUpdateT update_type,
130+
RecCheckT check_type, const char *check_regex, RecSourceT source, RecAccessT access_type)
131+
{
132+
ink_assert((rec_type == RECT_CONFIG) || (rec_type == RECT_LOCAL));
133+
134+
RecData my_data_default;
135+
136+
if constexpr (std::is_same_v<Tag, rec_detail::IntTag>) {
137+
my_data_default.rec_int = data_default;
138+
} else if constexpr (std::is_same_v<Tag, rec_detail::CounterTag>) {
139+
my_data_default.rec_counter = data_default;
140+
} else if constexpr (std::is_same_v<Tag, rec_detail::FloatTag>) {
141+
my_data_default.rec_float = data_default;
142+
} else if constexpr (std::is_same_v<Tag, rec_detail::StringTag>) {
143+
my_data_default.rec_string = data_default;
144+
} else {
145+
static_assert(always_false<Tag>::value, "Unsupported tag for RecRegisterConfig");
90146
}
91147

148+
if (RecRecord *r = RecRegisterConfig(rec_type, name, Tag::data_type, my_data_default, update_type, check_type, check_regex,
149+
source, access_type);
150+
r != nullptr) {
151+
if (i_am_the_record_owner(r->rec_type)) {
152+
r->sync_required = r->sync_required | REC_PEER_SYNC_REQUIRED;
153+
}
154+
return REC_ERR_OKAY;
155+
}
156+
157+
return REC_ERR_FAIL;
158+
}
159+
} // namespace
160+
92161
RecErrT
93162
RecRegisterConfigInt(RecT rec_type, const char *name, RecInt data_default, RecUpdateT update_type, RecCheckT check_type,
94163
const char *check_regex, RecSourceT source, RecAccessT access_type)
95164
{
96-
ink_assert((rec_type == RECT_CONFIG) || (rec_type == RECT_LOCAL));
97-
REC_REGISTER_CONFIG_XXX(rec_int, RECD_INT);
165+
return RecRegisterConfigImpl<rec_detail::IntTag>(rec_type, name, data_default, update_type, check_type, check_regex, source,
166+
access_type);
98167
}
99168

100169
RecErrT
101170
RecRegisterConfigFloat(RecT rec_type, const char *name, RecFloat data_default, RecUpdateT update_type, RecCheckT check_type,
102171
const char *check_regex, RecSourceT source, RecAccessT access_type)
103172
{
104-
ink_assert((rec_type == RECT_CONFIG) || (rec_type == RECT_LOCAL));
105-
REC_REGISTER_CONFIG_XXX(rec_float, RECD_FLOAT);
173+
return RecRegisterConfigImpl<rec_detail::FloatTag>(rec_type, name, data_default, update_type, check_type, check_regex, source,
174+
access_type);
106175
}
107176

108177
RecErrT
109-
RecRegisterConfigString(RecT rec_type, const char *name, const char *data_default_tmp, RecUpdateT update_type, RecCheckT check_type,
178+
RecRegisterConfigString(RecT rec_type, const char *name, const char *data_default, RecUpdateT update_type, RecCheckT check_type,
110179
const char *check_regex, RecSourceT source, RecAccessT access_type)
111180
{
112-
RecString data_default = const_cast<RecString>(data_default_tmp);
113-
ink_assert((rec_type == RECT_CONFIG) || (rec_type == RECT_LOCAL));
114-
REC_REGISTER_CONFIG_XXX(rec_string, RECD_STRING);
181+
return RecRegisterConfigImpl<rec_detail::StringTag>(rec_type, name, const_cast<RecString>(data_default), update_type, check_type,
182+
check_regex, source, access_type);
115183
}
116184

117185
RecErrT
118186
RecRegisterConfigCounter(RecT rec_type, const char *name, RecCounter data_default, RecUpdateT update_type, RecCheckT check_type,
119187
const char *check_regex, RecSourceT source, RecAccessT access_type)
120188
{
121-
ink_assert((rec_type == RECT_CONFIG) || (rec_type == RECT_LOCAL));
122-
REC_REGISTER_CONFIG_XXX(rec_counter, RECD_COUNTER);
189+
return RecRegisterConfigImpl<rec_detail::CounterTag>(rec_type, name, data_default, update_type, check_type, check_regex, source,
190+
access_type);
123191
}
124192

125193
//-------------------------------------------------------------------------

‎src/records/test_RecordsConfig.cc‎

Lines changed: 0 additions & 53 deletions
This file was deleted.

‎src/records/unit_tests/test_RecRegister.cc‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/** @file
2+
3+
Catch-based tests for RecRegister.cc
4+
5+
@section license License
6+
7+
Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements.
8+
See the NOTICE file distributed with this work for additional information regarding copyright
9+
ownership. The ASF licenses this file to you under the Apache License, Version 2.0 (the
10+
"License"); you may not use this file except in compliance with the License. You may obtain a
11+
copy of the License at
12+
13+
http://www.apache.org/licenses/LICENSE-2.0
14+
15+
Unless required by applicable law or agreed to in writing, software distributed under the License
16+
is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
17+
or implied. See the License for the specific language governing permissions and limitations under
18+
the License.
19+
*/
20+
#include <catch2/catch_test_macros.hpp>
21+
#include "records/RecCore.h"
22+
#include "iocore/eventsystem/EventSystem.h"
23+
#include "iocore/eventsystem/RecProcess.h"
24+
#include "tscore/Layout.h"
25+
#include "test_Diags.h"
26+
27+
TEST_CASE("RecRegisterConfig - Type Dispatch", "[librecords][RecConfig]")
28+
{
29+
SECTION("RecRegisterConfigInt")
30+
{
31+
RecErrT err = RecRegisterConfigInt(RECT_CONFIG, "proxy.test.int_value", 42, RECU_DYNAMIC, RECC_NULL, nullptr, REC_SOURCE_NULL);
32+
REQUIRE(err == REC_ERR_OKAY);
33+
34+
RecInt value = RecGetRecordInt("proxy.test.int_value").value_or(0);
35+
REQUIRE(value == 42);
36+
}
37+
38+
SECTION("RecRegisterConfigFloat")
39+
{
40+
RecErrT err =
41+
RecRegisterConfigFloat(RECT_CONFIG, "proxy.test.float_value", 3.14f, RECU_DYNAMIC, RECC_NULL, nullptr, REC_SOURCE_NULL);
42+
REQUIRE(err == REC_ERR_OKAY);
43+
44+
RecFloat value = RecGetRecordFloat("proxy.test.float_value").value_or(0.0f);
45+
REQUIRE(value == 3.14f);
46+
}
47+
48+
SECTION("RecRegisterConfigString")
49+
{
50+
RecErrT err =
51+
RecRegisterConfigString(RECT_CONFIG, "proxy.test.string_value", "hello", RECU_DYNAMIC, RECC_NULL, nullptr, REC_SOURCE_NULL);
52+
REQUIRE(err == REC_ERR_OKAY);
53+
54+
char value_buf[6];
55+
std::string_view value = RecGetRecordString("proxy.test.string_value", value_buf, sizeof(value_buf)).value_or("");
56+
REQUIRE(value.empty() == false);
57+
REQUIRE(value == "hello");
58+
}
59+
}
60+
61+
TEST_CASE("RecRegisterStat - Type Dispatch", "[librecords][RecStat]")
62+
{
63+
SECTION("RecRegisterStatInt")
64+
{
65+
RecErrT err = RecRegisterStatInt(RECT_NODE, "proxy.node.test.int", 99, RECP_NON_PERSISTENT);
66+
REQUIRE(err == REC_ERR_OKAY);
67+
68+
RecInt value = RecGetRecordInt("proxy.node.test.int").value_or(0);
69+
REQUIRE(value == 99);
70+
}
71+
72+
SECTION("RecRegisterStatFloat")
73+
{
74+
RecErrT err = RecRegisterStatFloat(RECT_NODE, "proxy.node.test.float", 2.71f, RECP_NON_PERSISTENT);
75+
REQUIRE(err == REC_ERR_OKAY);
76+
77+
RecFloat value = RecGetRecordFloat("proxy.node.test.float").value_or(0.0f);
78+
REQUIRE(value == 2.71f);
79+
}
80+
81+
SECTION("RecRegisterStatCounter")
82+
{
83+
RecErrT err = RecRegisterStatCounter(RECT_NODE, "proxy.node.test.counter", 500, RECP_NON_PERSISTENT);
84+
REQUIRE(err == REC_ERR_OKAY);
85+
86+
RecCounter value = RecGetRecordCounter("proxy.node.test.counter").value_or(0);
87+
REQUIRE(value == 500);
88+
}
89+
}

‎src/records/unit_tests/unit_test_main.cc‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
#include <vector>
2222
#include <string>
2323
#include "tscore/ink_resolver.h"
24+
#include "iocore/eventsystem/EventSystem.h"
25+
#include "iocore/eventsystem/RecProcess.h"
26+
#include "tscore/Layout.h"
2427
#include "test_Diags.h"
2528

2629
#include <catch2/catch_test_macros.hpp>
@@ -32,8 +35,9 @@ int
3235
main(int argc, char *argv[])
3336
{
3437
// Set the global diags variable
38+
Layout::create(); // RecProcess will fail if Layout is not created.
3539
DiagsPtr::set(new CatchDiags);
36-
40+
RecProcessInit();
3741
// Global data initialization needed for the unit tests.
3842
ts_session_protocol_well_known_name_indices_init();
3943
// Cheat for ts_host_res_global_init as there's no records.config to check for non-default.

0 commit comments

Comments
 (0)