Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in FindResources() for data-only bundles #667

Merged
merged 3 commits into from
May 11, 2022

Conversation

achristoforides
Copy link
Member

When a bundle is installed with the optional manifest argument, the bundle isn't fopen'd automatically. In the case of data-only bundles, it is never fopen'd and the BundleResourceContainer is never initialized.

A call to FindResources() on a data-only bundle who was installed via InstallBundles() with the optional manifest will result in 0 resources always being returned.

Signed-off-by: The MathWorks, Inc. alchrist@mathworks.com

Signed-off-by: The MathWorks, Inc. <alchrist@mathworks.com>
@codecov-commenter
Copy link

codecov-commenter commented May 11, 2022

Codecov Report

Merging #667 (844f29a) into development (47a3f92) will increase coverage by 0.01%.
The diff coverage is 83.33%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #667      +/-   ##
===============================================
+ Coverage        90.49%   90.50%   +0.01%     
===============================================
  Files              249      248       -1     
  Lines            13483    13477       -6     
===============================================
- Hits             12201    12198       -3     
+ Misses            1282     1279       -3     
Impacted Files Coverage Δ
framework/src/bundle/BundleResourceContainer.h 100.00% <ø> (ø)
framework/src/bundle/BundleResourceContainer.cpp 95.34% <83.33%> (+1.16%) ⬆️
util/include/cppmicroservices/util/BundleObjFile.h 0.00% <0.00%> (-100.00%) ⬇️
...nfigurationAdmin/src/ConfigurationAdminPrivate.hpp 80.00% <0.00%> (-11.67%) ⬇️
framework/include/cppmicroservices/BundleContext.h 94.91% <0.00%> (-1.81%) ⬇️
.../cppmicroservices/detail/ServiceTrackerPrivate.tpp 81.69% <0.00%> (-1.17%) ⬇️
...rativeServices/src/metadata/MetadataParserImpl.cpp 90.96% <0.00%> (-0.65%) ⬇️
.../ConfigurationAdmin/src/ConfigurationAdminImpl.cpp 92.93% <0.00%> (-0.04%) ⬇️
...ervices/src/manager/ComponentConfigurationImpl.cpp 97.33% <0.00%> (-0.02%) ⬇️
framework/src/service/ServiceListenerEntry.h 100.00% <0.00%> (ø)
... and 9 more

Signed-off-by: The MathWorks, Inc. <alchrist@mathworks.com>
@@ -265,11 +260,17 @@ bool BundleResourceContainer::Matches(const std::string& name,
return true;
}

void BundleResourceContainer::OpenContainer()
void BundleResourceContainer::OpenContainer() const
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this no longer only opens the container. I suggest a method name change to describe the intent. suggestion: OpenAndInitializeContainer

@achristoforides achristoforides merged commit d11c608 into development May 11, 2022
@achristoforides achristoforides deleted the fix-brc-not-open-when-using-cache branch May 11, 2022 21:54
insi-eb pushed a commit to insi-eb/CppMicroServices-cpp14 that referenced this pull request Oct 5, 2022
* Fixed bug in FindResources() for data-only bundles

Signed-off-by: The MathWorks, Inc. <alchrist@mathworks.com>

* Marked bundleManifest as UNUSED in non-threaded env

Signed-off-by: The MathWorks, Inc. <alchrist@mathworks.com>

* Made changes requested by reviewers
jeffdiclemente pushed a commit that referenced this pull request Oct 5, 2022
* Fixed bug in FindResources() for data-only bundles

Signed-off-by: The MathWorks, Inc. <alchrist@mathworks.com>

* Marked bundleManifest as UNUSED in non-threaded env

Signed-off-by: The MathWorks, Inc. <alchrist@mathworks.com>

* Made changes requested by reviewers

Co-authored-by: Alexander Christoforides <38366659+achristoforides@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants