Skip to content

Commit 3b65270

Browse files
Remove Registration of Factory Configuration object even with valid config (CppMicroServices#1006) (CppMicroServices#1178)
Signed Off By: tcormack@mathworks.com Co-authored-by: tcormackMW <113473781+tcormackMW@users.noreply.github.com>
1 parent 73bba2f commit 3b65270

11 files changed

Lines changed: 132 additions & 16 deletions

File tree

compendium/DeclarativeServices/src/manager/ConfigurationManager.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,11 @@ namespace cppmicroservices
154154
{
155155
bool allConfigsAvailable = configProperties.size() >= metadata->configurationPids.size();
156156

157-
if ((metadata->configurationPolicy != CONFIG_POLICY_REQUIRE) || (allConfigsAvailable))
157+
// If all configs are available or we do not require configs AND this component is not a factory component.
158+
// No factory component should ever have satisfied configurations even if an erroneous pid is added which
159+
// exactly matches the factory PID
160+
if (((metadata->configurationPolicy != CONFIG_POLICY_REQUIRE) || (allConfigsAvailable))
161+
&& metadata->factoryComponentID.empty())
158162
{
159163
return true;
160164
}

compendium/DeclarativeServices/test/gtest/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ set(_test_bundles
213213
TestBundleDSCA24
214214
TestBundleDSCA26
215215
TestBundleDSCA27
216+
TestBundleDSCA28
216217
TestBundleDSFAC1
217218
EnglishDictionary
218219
)

compendium/DeclarativeServices/test/gtest/ImportTestBundles.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA21)
4545
CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA24)
4646
CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA26)
4747
CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA27)
48+
CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSCA28)
4849
CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSDependent)
4950
CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSDependentNoInject)
5051
CPPMICROSERVICES_INITIALIZE_STATIC_BUNDLE(TestBundleDSDependentOptional)

compendium/DeclarativeServices/test/gtest/TestFixture.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ namespace test
117117
test::InstallLib(context, "TestBundleDSCA24");
118118
test::InstallLib(context, "TestBundleDSCA26");
119119
test::InstallLib(context, "TestBundleDSCA27");
120+
test::InstallLib(context, "TestBundleDSCA28");
120121
#endif
121122

122123
#ifndef US_BUILD_SHARED_LIBS

compendium/DeclarativeServices/test/gtest/TestUpdateConfiguration.cpp

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ namespace test
5555
// Install and start the bundle which has the service
5656
auto testBundle = ::test::InstallAndStartBundle(ctx, "TestBundleDSCA02");
5757
ASSERT_TRUE(testBundle);
58-
const std::string componentName { "sample::ServiceComponentCA02" };
58+
std::string const componentName { "sample::ServiceComponentCA02" };
5959

6060
// Confirm configuration object is available and service is resolved.
6161
scr::dto::ComponentDescriptionDTO compDescDTO;
@@ -124,7 +124,7 @@ namespace test
124124
{
125125
cppmicroservices::BundleContext ctx = framework.GetBundleContext();
126126

127-
const std::string svcComponentName { "sample::ServiceComponentCA02" };
127+
std::string const svcComponentName { "sample::ServiceComponentCA02" };
128128

129129
// Get a service reference to ConfigAdmin to create the configuration object.
130130
auto configAdminService = GetInstance<cppmicroservices::service::cm::ConfigurationAdmin>();
@@ -173,7 +173,7 @@ namespace test
173173
*/
174174
TEST_F(tServiceComponent, testConfigObjectInManifestResolvesService)
175175
{
176-
const std::string configObject = "mw.dependency";
176+
std::string const configObject = "mw.dependency";
177177

178178
// Install and start the bundle containing the configuration object and the
179179
// service which is dependent on the configuration object.
@@ -236,7 +236,7 @@ namespace test
236236

237237
// Confirm that the properties match the properties provided in the
238238
// manifest.json file.
239-
const std::string bar { "bar" };
239+
std::string const bar { "bar" };
240240
auto serviceProps = service->GetProperties();
241241
auto foo = serviceProps.find("foo");
242242
ASSERT_TRUE(foo != serviceProps.end()) << "foo not found in constructed instance";
@@ -283,7 +283,8 @@ namespace test
283283
auto fut1 = configuration1->Update(props1);
284284
fut1.wait();
285285
auto service0 = GetInstance<test::CAInterface>();
286-
ASSERT_TRUE(service0) << "Factory instance should be created when a '~' configuration is added";
286+
ASSERT_TRUE(service0)
287+
<< "Factory instance should be created when a '~' configuration is added";
287288
ASSERT_TRUE(service0->GetProperties().find("uniqueKey")->second == 6);
288289

289290
testBundle.Stop();
@@ -307,7 +308,7 @@ namespace test
307308
// starting the bundle which defines the service.
308309
auto configuration = configAdminService->GetConfiguration(configObjectName);
309310
cppmicroservices::AnyMap props(cppmicroservices::AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS);
310-
const std::string fooProp { "notbar" };
311+
std::string const fooProp { "notbar" };
311312
props["foo"] = fooProp;
312313
configuration->UpdateIfDifferent(props);
313314

@@ -383,7 +384,7 @@ namespace test
383384

384385
// Update property
385386
cppmicroservices::AnyMap props(cppmicroservices::AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS);
386-
const std::string instanceId { "instance1" };
387+
std::string const instanceId { "instance1" };
387388
props["uniqueProp"] = instanceId;
388389
auto fut = configObject->Update(props);
389390
fut.get();
@@ -437,7 +438,7 @@ namespace test
437438
EXPECT_EQ(compConfigs.at(0).state, scr::dto::ComponentState::ACTIVE) << "Component state should be ACTIVE";
438439

439440
// Update property
440-
const std::string instanceId { "instance1" };
441+
std::string const instanceId { "instance1" };
441442
props["uniqueProp"] = instanceId;
442443
fut = configObject->Update(props);
443444
fut.get();
@@ -484,9 +485,9 @@ namespace test
484485
// Verify that all changes to the configuration objects on which the component is dependent
485486
// result in a deactivation and reactivation of the component
486487
std::shared_ptr<cppmicroservices::service::cm::Configuration> configuration;
487-
const std::string uniqueProp[3] = { "uniqueProp1", "uniqueProp2", "uniqueProp3" };
488+
std::string const uniqueProp[3] = { "uniqueProp1", "uniqueProp2", "uniqueProp3" };
488489
cppmicroservices::AnyMap props(cppmicroservices::AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS);
489-
const std::string instance[3] = { "instance1", "instance2", "instance3" };
490+
std::string const instance[3] = { "instance1", "instance2", "instance3" };
490491

491492
int i = 0;
492493
while (i < 3)
@@ -551,7 +552,7 @@ namespace test
551552
auto configInstance = configuration->GetPid();
552553

553554
cppmicroservices::AnyMap props(cppmicroservices::AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS);
554-
const std::string instanceId { "instance1" };
555+
std::string const instanceId { "instance1" };
555556
props["uniqueProp"] = instanceId;
556557
auto fut = configuration->Update(props);
557558
fut.get();
@@ -573,7 +574,7 @@ namespace test
573574
EXPECT_EQ(uniqueProp->second, instanceId);
574575

575576
// Update property
576-
const std::string instanceId2 { "instance2" };
577+
std::string const instanceId2 { "instance2" };
577578
props["uniqueProp"] = instanceId2;
578579
fut = configuration->Update(props);
579580
fut.get();
@@ -650,7 +651,7 @@ namespace test
650651
auto configInstance = configuration->GetPid();
651652

652653
cppmicroservices::AnyMap props(cppmicroservices::AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS);
653-
const std::string newProp { "newProperty" };
654+
std::string const newProp { "newProperty" };
654655
props["uniqueProp"] = newProp;
655656
auto fut = configuration->Update(props);
656657
fut.get();
@@ -704,7 +705,7 @@ namespace test
704705
auto configInstance = configuration->GetPid();
705706

706707
cppmicroservices::AnyMap props(cppmicroservices::AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS);
707-
const std::string initialProp { "initialProp" };
708+
std::string const initialProp { "initialProp" };
708709
props["uniqueProp"] = initialProp;
709710
auto fut = configuration->Update(props);
710711
fut.get();
@@ -746,7 +747,7 @@ namespace test
746747
// this is a delayed component(immediate = false). The update call
747748
// should result in the component begin deactivated. Verify that the
748749
// state is SATISFIED.
749-
const std::string updatedProp { "updatedProp" };
750+
std::string const updatedProp { "updatedProp" };
750751
props["uniqueProp"] = updatedProp;
751752
fut = configuration->Update(props);
752753
fut.get();

compendium/test_bundles/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ add_subdirectory(TestBundleDSCA21)
6363
add_subdirectory(TestBundleDSCA24)
6464
add_subdirectory(TestBundleDSCA26)
6565
add_subdirectory(TestBundleDSCA27)
66+
add_subdirectory(TestBundleDSCA28)
6667
add_subdirectory(TestBundleDSDependent)
6768
add_subdirectory(TestBundleDSDependentNoInject)
6869
add_subdirectory(TestBundleDSDependentOptional)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
usFunctionCreateDSTestBundle(TestBundleDSCA28)
2+
3+
usFunctionCreateTestBundleWithResources(TestBundleDSCA28
4+
SOURCES src/ServiceImpl.cpp ${_glue_file}
5+
RESOURCES manifest.json
6+
BUNDLE_SYMBOLIC_NAME TestBundleDSCA28
7+
OTHER_LIBRARIES usTestInterfaces usServiceComponent)
8+
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
{
2+
"bundle.symbolic_name": "TestBundleDSCA28",
3+
"bundle.activator": false,
4+
"scr": {
5+
"version": 1,
6+
"components": [
7+
{
8+
"configuration-pid": [
9+
"my.dependency.pid"
10+
],
11+
"factory": "myFactoryComponent",
12+
"factory-properties": {
13+
"prop3": 3
14+
},
15+
"service": {
16+
"scope": "singleton",
17+
"interfaces": [
18+
"test::CAInterface"
19+
]
20+
},
21+
"inject-references": false,
22+
"enabled": true,
23+
"immediate": false,
24+
"implementation-class": "sample::ServiceComponentCA28",
25+
"configuration-policy": "optional",
26+
"properties": {
27+
"service.ranking": -10,
28+
"prop1": 1
29+
}
30+
}
31+
]
32+
},
33+
"cm": {
34+
"version": 1,
35+
"configurations": [
36+
{
37+
"pid": "my.dependency.pid",
38+
"properties": {
39+
"prop2": "2"
40+
}
41+
}
42+
]
43+
}
44+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#ifndef SERVICECOMPONENTS_HPP
2+
#define SERVICECOMPONENTS_HPP
3+
4+
#include "ServiceImpl.hpp"
5+
6+
#endif
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#include "ServiceImpl.hpp"
2+
#include <iostream>
3+
4+
namespace sample
5+
{
6+
7+
void
8+
ServiceComponentCA28::Modified(std::shared_ptr<ComponentContext> const& /*context*/,
9+
std::shared_ptr<cppmicroservices::AnyMap> const& configuration)
10+
{
11+
std::lock_guard<std::mutex> lock(propertiesLock);
12+
properties = configuration;
13+
}
14+
15+
cppmicroservices::AnyMap
16+
ServiceComponentCA28::GetProperties()
17+
{
18+
std::lock_guard<std::mutex> lock(propertiesLock);
19+
return *properties;
20+
}
21+
22+
} // namespace sample

0 commit comments

Comments
 (0)