Skip to content

Commit dbaa100

Browse files
authored
DPL: protect against multiple additions of 0xccdb queries (#12020)
1 parent 6010fca commit dbaa100

File tree

5 files changed

+53
-4
lines changed

5 files changed

+53
-4
lines changed

Detectors/Raw/TFReaderDD/src/TFReaderSpec.cxx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,9 @@ o2f::DataProcessorSpec o2::rawdd::getTFReaderSpec(o2::rawdd::TFReaderInp& rinp)
465465
}
466466
}
467467
}
468-
spec.outputs.emplace_back(o2f::OutputSpec{{"stfDist"}, o2h::gDataOriginFLP, o2h::gDataDescriptionDISTSTF, 0});
468+
o2f::DataSpecUtils::updateOutputList(spec.outputs, o2f::OutputSpec{{"stfDist"}, o2h::gDataOriginFLP, o2h::gDataDescriptionDISTSTF, 0});
469469
if (!rinp.sup0xccdb) {
470-
spec.outputs.emplace_back(o2f::OutputSpec{{"stfDistCCDB"}, o2h::gDataOriginFLP, o2h::gDataDescriptionDISTSTF, 0xccdb});
470+
o2f::DataSpecUtils::updateOutputList(spec.outputs, o2f::OutputSpec{{"stfDistCCDB"}, o2h::gDataOriginFLP, o2h::gDataDescriptionDISTSTF, 0xccdb});
471471
}
472472
if (!rinp.metricChannel.empty()) {
473473
spec.options.emplace_back(o2f::ConfigParamSpec{"channel-config", o2f::VariantType::String, rinp.metricChannel, {"Out-of-band channel config for TF throttling"}});

Framework/Core/include/Framework/DataSpecUtils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,9 @@ struct DataSpecUtils {
230230

231231
/// Updates list of InputSpecs by merging metadata
232232
static void updateInputList(std::vector<InputSpec>& list, InputSpec&& input);
233+
234+
/// Updates list of OutputSpecs by merging metadata (or adding output).
235+
static void updateOutputList(std::vector<OutputSpec>& list, OutputSpec&& input);
233236
};
234237

235238
} // namespace framework

Framework/Core/src/DataSpecUtils.cxx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,4 +795,20 @@ void DataSpecUtils::updateInputList(std::vector<InputSpec>& list, InputSpec&& in
795795
}
796796
}
797797

798+
void DataSpecUtils::updateOutputList(std::vector<OutputSpec>& list, OutputSpec&& spec)
799+
{
800+
auto locate = std::find(list.begin(), list.end(), spec);
801+
if (locate != list.end()) {
802+
// amend entry
803+
auto& entryMetadata = locate->metadata;
804+
entryMetadata.insert(entryMetadata.end(), spec.metadata.begin(), spec.metadata.end());
805+
std::sort(entryMetadata.begin(), entryMetadata.end(), [](ConfigParamSpec const& a, ConfigParamSpec const& b) { return a.name < b.name; });
806+
auto new_end = std::unique(entryMetadata.begin(), entryMetadata.end(), [](ConfigParamSpec const& a, ConfigParamSpec const& b) { return a.name == b.name; });
807+
entryMetadata.erase(new_end, entryMetadata.end());
808+
} else {
809+
// add entry
810+
list.emplace_back(std::move(spec));
811+
}
812+
}
813+
798814
} // namespace o2::framework

Framework/Core/src/WorkflowHelpers.cxx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ void WorkflowHelpers::injectServiceDevices(WorkflowSpec& workflow, ConfigContext
588588
}
589589
if (enumCandidate != -1) {
590590
auto& dp = workflow[enumCandidate];
591-
dp.outputs.push_back(OutputSpec{{"ccdb-diststf"}, dstf, Lifetime::Timeframe});
591+
DataSpecUtils::updateOutputList(dp.outputs, OutputSpec{{"ccdb-diststf"}, dstf, Lifetime::Timeframe});
592592
ccdbBackend.inputs.push_back(InputSpec{"tfn", dstf, Lifetime::Timeframe});
593593
} else if (timerCandidate != -1) {
594594
auto& dp = workflow[timerCandidate];
@@ -635,7 +635,7 @@ void WorkflowHelpers::injectServiceDevices(WorkflowSpec& workflow, ConfigContext
635635
}
636636
if (enumCandidate != -1) {
637637
auto& dp = workflow[enumCandidate];
638-
dp.outputs.push_back(OutputSpec{{"ccdb-diststf"}, dstf, Lifetime::Timeframe});
638+
DataSpecUtils::updateOutputList(dp.outputs, OutputSpec{{"ccdb-diststf"}, dstf, Lifetime::Timeframe});
639639
ccdbBackend.inputs.push_back(InputSpec{"tfn", dstf, Lifetime::Timeframe});
640640
} else if (timerCandidate != -1) {
641641
auto& dp = workflow[timerCandidate];

Framework/Core/test/unittest_DataSpecUtils.cxx

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,3 +460,33 @@ TEST_CASE("optionalConcreteDataMatcherFrom")
460460
REQUIRE(DataSpecUtils::asOptionalConcreteDataMatcher(ConcreteDataMatcher{"ITS", "RAWDATA", 0}) == ConcreteDataMatcher{"ITS", "RAWDATA", 0});
461461
REQUIRE(DataSpecUtils::asOptionalConcreteDataMatcher(ConcreteDataTypeMatcher{"ITS", "RAWDATA"}) == std::nullopt);
462462
}
463+
464+
// Testcase for DataSpecUtils::updateOutputList
465+
TEST_CASE("UpdateOutputList")
466+
{
467+
// Empty vector of output specs should simply add the new spec
468+
std::vector<OutputSpec> outputSpecs;
469+
DataSpecUtils::updateOutputList(outputSpecs, {"TST", "DATA1", 0});
470+
CHECK(outputSpecs.size() == 1);
471+
472+
// Adding the same spec again should not change the size
473+
DataSpecUtils::updateOutputList(outputSpecs, {"TST", "DATA1", 0});
474+
CHECK(outputSpecs.size() == 1);
475+
476+
// Adding a different spec should increase the size
477+
DataSpecUtils::updateOutputList(outputSpecs, {"TST", "DATA2", 0});
478+
CHECK(outputSpecs.size() == 2);
479+
480+
// Adding again the same spec again should not change the size
481+
DataSpecUtils::updateOutputList(outputSpecs, {"TST", "DATA1", 0});
482+
CHECK(outputSpecs.size() == 2);
483+
// Check with something real..
484+
ConcreteDataMatcher dstf{"FLP", "DISTSUBTIMEFRAME", 0xccdb};
485+
DataSpecUtils::updateOutputList(outputSpecs, OutputSpec{{"ccdb-diststf"}, dstf, Lifetime::Timeframe});
486+
CHECK(outputSpecs.size() == 3);
487+
DataSpecUtils::updateOutputList(outputSpecs, OutputSpec{{"ccdb-diststf"}, dstf, Lifetime::Timeframe});
488+
CHECK(outputSpecs.size() == 3);
489+
// Label does not matter
490+
DataSpecUtils::updateOutputList(outputSpecs, OutputSpec{{"ccdb2-diststf"}, dstf, Lifetime::Timeframe});
491+
CHECK(outputSpecs.size() == 3);
492+
}

0 commit comments

Comments
 (0)