From 95c9b64c41b1a29299a91f1e9934ee630d34c85c Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Mon, 29 Jun 2020 10:09:51 -0400 Subject: [PATCH 01/22] add new install bundles api rfc --- text/0006-BundleContext-Install-API.md | 136 +++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 text/0006-BundleContext-Install-API.md diff --git a/text/0006-BundleContext-Install-API.md b/text/0006-BundleContext-Install-API.md new file mode 100644 index 0000000..dd7fd88 --- /dev/null +++ b/text/0006-BundleContext-Install-API.md @@ -0,0 +1,136 @@ +- Start Date: 2020-06-09 +- RFC PR: (in a subsequent commit to the PR, fill me with the PR's URL) +- CppMicroServices Issue: (fill me with the URL of the CppMicroServices issue that necessisated this RFC, otherwise leave blank) + +# Add BundleContext::InstallBundles API + +## Summary + +Provide a api to initialize the bundle registry with a bundle manifest + +## Motivation + +Allows users of the system to inject bundle manifests into the registry directly. We will be able to use this mechanism to speed up initial install of very large numbers of bundles. + +## Detailed design + +### BundleContext + +```c++ +/** Wrapper around BundleRegistry::Install +* +* @param location The absolute path to the location of the bundle whose manifest +* is to be installed +* @param manifest the manifest for the bundle. +*/ +vector BundleContext::InstallBundles(std::string location + , std::vector manifest); + + +``` + +* Modify old API to call new API with empty manifest map +* Wrapper around call to BundleRegistry::Install +* Refactor original API to call new API so we only have one implementation +* + +### BundleRegistry + +```c++ +/** Install the manifest for the bundle at location into the registry. +* +* The difference between this implementation and the existing implementation +* that takes only the location argument is that in the original the manifest +* is read from the bundle file rather than it being passed in as an argument. +* +* @param location The absolute path to the location of the bundle whose manifest is to +* be installed +* @manifest the manifest of the bundle at location +*/ +vector BundleRegistry::Install(std::string location + , std::vector manifest); +``` + +* Update Install to accept a manifest to use for the installation instead of reading from bundle at location +* If manifest is and empty map, then proceed with install action as before. This accomplishes a lazy loading of the manifest from the file. The manifest is only read from the file in the event that it is empty when asked for. + +### BundleManifest + +```c++ +/** Initialize manifest with content of AnyMap +* +* Note: No validation is done on manifestHeaders. It is assumed that by +* by this point, the headers are correct. +* +* @param manifestHeaders a map of the headers for a bundle manifest. +*/ +explicit BundleManifest::BundleManifest(const AnyMap& manifestHeaders); + +``` + +### BundlePrivate + +* Additional constructor that takes bundle manifest to be injected +* Modified to fetch bundle manifest from BundleArchive instead of parsing the resource +* Factor out manifest validation from constructor to allow for use from new constructor which takes a manifest to be injected as an argument + +### BundleStorage + +The **BundleStorage** class hierarchy is used for implementing different strategies for tracking bundle **BundleArchives**. There are two subclasses, but only one of them provides an actual implementation. They are **BundleStorageMemory**, and **BundleStorageFile**. It also tracks which bundles are to be auto started. + +```c++ + /** + * Insert bundles from a container into persistent storagedata. + * + * This is a simpler version of the InstallBundles api that only deals with one bundle at a time. + * + * @param resCont The container for the bundle data and resources. + * @param topLevelEntries The top level entries in the container to be inserted as bundle archives. + * @return A list of BundleArchive instances representing the installed bundles. + */ + using ManifestT = cppmicroservices::AnyMap; + virtual std::shared_ptr InsertArchive(const std::shared_ptr& resCont + , const std::string& topLevelEntry + , const ManifestT&) = 0; + +``` + +* Combined BundleStorage::InsertBundleLib and BundleStorage::InsertArchives + * InsertBundleLib was a wrapper around InsertArchives + * It created a BundleResourceContainer for the location and called InsertArchives + * Now the BundleResourceContainer is created if needed in BundleRegistry::GetAlreadyInstalldBundlesAtLocation and then passed in to BundleStorage::InsertArchive. + +### BundleArchive + +* Refactor and remove Data class simplifying design. +* Add BundleManifest and constructor argument + * Call BundleManifest::Parse from BundleArchive in the event that a manifest is not passed in at construction +* Add new **GetManifest()** method which does a lazy loadof the manifest from the archive if and only if we don't already have the manifest. + +### BundleResourceContainer + +* Add **manifest** argument to constructor + * If the **manifest** is not empty, use it to initialize the **m_SortedToplevelDirs**, and leave **m_IsContainerOpen** set to false + * if **manifest** is empty, no manifest injection is happening, so open up the archive and read the manifest from there, set set **m_IsContainerOpen** to true. + +### BundleStorage(Memory) + +* Refactor + * Remove **InsertArchives()** method and move loop to caller in **BundleRegistry::Install0** + +### Other Changes + +* Various code cleanups (getting rid of "using std", exraneous cout output, etc.) + +## Test Cases + +- Install bundle manifest at a location, and then retrieve bundle and check manifest +- Install bundle manifest for a test bundle that exists. Then get the service, call a method through the service interface and made sure that it was successfully started. +- Install bundle manifest for multiple bundles that exist on disk and then start each one and verify that they have been properly instantiated. +- Ensure Install bundle manifest for bundle already installed is ignored +- Test for malformed manifest data. + - I need to add manifest validation for required pieces of metadata for a bundle to work properly... we think that's only the bundle.symbolic_name. + +## Unresolved questions + +* \ No newline at end of file From 2faa5cbe47897ea840bf7f69ebcdf19f1f183b30 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Mon, 6 Jul 2020 08:52:22 -0400 Subject: [PATCH 02/22] Moved text to folder and added sequence diagram. --- .../0006-BundleContext-Install-API.md | 2 + .../InstallBundlesSequence.svg | 294 ++++++++++++++++++ 2 files changed, 296 insertions(+) rename text/{ => 0006-BundleContext-Install-API}/0006-BundleContext-Install-API.md (99%) create mode 100644 text/0006-BundleContext-Install-API/InstallBundlesSequence.svg diff --git a/text/0006-BundleContext-Install-API.md b/text/0006-BundleContext-Install-API/0006-BundleContext-Install-API.md similarity index 99% rename from text/0006-BundleContext-Install-API.md rename to text/0006-BundleContext-Install-API/0006-BundleContext-Install-API.md index dd7fd88..40ed53b 100644 --- a/text/0006-BundleContext-Install-API.md +++ b/text/0006-BundleContext-Install-API/0006-BundleContext-Install-API.md @@ -14,6 +14,8 @@ Allows users of the system to inject bundle manifests into the registry directly ## Detailed design +![InstallBundlesSequence](./InstallBundlesSequence.svg) + ### BundleContext ```c++ diff --git a/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg b/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg new file mode 100644 index 0000000..ead6590 --- /dev/null +++ b/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg @@ -0,0 +1,294 @@ + + + + + + + BundleContext::InstallBundles + + + + BundleContext + + + + BundleRegistry + + + + BundlePrivate + + + + BundleStorageMemory + + + + BundleArchive + + + + BundleResourceContainer + + + + BundleManifest + + + + Bundle + + + + BundleContext + + + + BundleRegistry + + + + BundlePrivate + + + + BundleStorageMemory + + + + BundleArchive + + + + BundleResourceContainer + + + + BundleManifest + + + + Bundle + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + loop + foreach symbolicName : dirnames + + + + + + loop + foreach ba : barchives + + + InstallBundles + + + Install + + + GetAlreadyInstalledBundlesAtLocation + + + Install0 + + + vector<installedBundles> + + + vector<installedBundles> + + + Install0 + + + GetTopLevelDirs + + + vector<dirnames> + + + CreateAndInsertArchive + + + + + + new BundleArchive + + + new BundleArchive + + + create BundlePrivate object + + + new BundlePrivate + + + create Bundle(BundlePrivate) + + + new Bundle + + + vector<installedBundles> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Store new BundleArchive in barchives for later processing + + + + Store created bundle in results vector to be returned + + + + BundleContext::InstallBundles + + + + + + + + BundleRegistry::Install0 + + + + + + + + + From 722cb5c618608263be3200d57bed04087f74c4bb Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Mon, 6 Jul 2020 12:47:24 -0400 Subject: [PATCH 03/22] moved 0006 text back to text root and updated image link. --- .../0006-BundleContext-Install-API.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename text/{0006-BundleContext-Install-API => }/0006-BundleContext-Install-API.md (98%) diff --git a/text/0006-BundleContext-Install-API/0006-BundleContext-Install-API.md b/text/0006-BundleContext-Install-API.md similarity index 98% rename from text/0006-BundleContext-Install-API/0006-BundleContext-Install-API.md rename to text/0006-BundleContext-Install-API.md index 40ed53b..93487f6 100644 --- a/text/0006-BundleContext-Install-API/0006-BundleContext-Install-API.md +++ b/text/0006-BundleContext-Install-API.md @@ -14,7 +14,7 @@ Allows users of the system to inject bundle manifests into the registry directly ## Detailed design -![InstallBundlesSequence](./InstallBundlesSequence.svg) +![InstallBundlesSequence](0006-BundleContext-Install-API/InstallBundlesSequence.svg) ### BundleContext From 4e2544666f09226844e9fab8a1370f654a8216e9 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Mon, 6 Jul 2020 12:57:41 -0400 Subject: [PATCH 04/22] added some grouping in the sequence diagram --- .../InstallBundlesSequence.svg | 270 +++++++++--------- 1 file changed, 133 insertions(+), 137 deletions(-) diff --git a/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg b/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg index ead6590..bba3cfc 100644 --- a/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg +++ b/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg @@ -1,292 +1,288 @@ - + - + - BundleContext::InstallBundles + BundleContext::InstallBundles - - BundleContext + + BundleContext - - BundleRegistry + + BundleRegistry - - BundlePrivate + + BundlePrivate - - BundleStorageMemory + + BundleStorageMemory - - BundleArchive + + BundleArchive - - BundleResourceContainer + + BundleResourceContainer - - BundleManifest + + BundleManifest - - Bundle + + Bundle - - BundleContext + + BundleContext - - BundleRegistry + + BundleRegistry - - BundlePrivate + + BundlePrivate - - BundleStorageMemory + + BundleStorageMemory - - BundleArchive + + BundleArchive - - BundleResourceContainer + + BundleResourceContainer - - BundleManifest + + BundleManifest - - Bundle + + Bundle - + - + - + - + - + - + - + - + - + - + + + + + + + BundleContext::InstallBundles + + + + + + BundleRegistry:Install0 - - - - loop - foreach symbolicName : dirnames + + + + loop + foreach symbolicName : dirnames - - - - loop - foreach ba : barchives + + + + loop + foreach ba : barchives - InstallBundles + InstallBundles - Install + Install - GetAlreadyInstalledBundlesAtLocation + GetAlreadyInstalledBundlesAtLocation - Install0 + Install0 - vector<installedBundles> + vector<installedBundles> - vector<installedBundles> + vector<installedBundles> - Install0 + Install0 - GetTopLevelDirs + GetTopLevelDirs - vector<dirnames> + vector<dirnames> - CreateAndInsertArchive + CreateAndInsertArchive - + - new BundleArchive + new BundleArchive - new BundleArchive + new BundleArchive - create BundlePrivate object + create BundlePrivate object - new BundlePrivate + new BundlePrivate - create Bundle(BundlePrivate) + create Bundle(BundlePrivate) - new Bundle + new Bundle - vector<installedBundles> + vector<installedBundles> - + - - + + - + - + - - - - + + + + - - - - + + + + - + - + - + - - + + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - - Store new BundleArchive in barchives for later processing + + Store new BundleArchive in barchives for later processing - - Store created bundle in results vector to be returned - - - - BundleContext::InstallBundles - - - - - - - - BundleRegistry::Install0 - - - - + + Store created bundle in results vector to be returned From db83bd30dd4f2357b5e551451633d575bdd7913f Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Mon, 6 Jul 2020 13:05:22 -0400 Subject: [PATCH 05/22] updated. --- .../InstallBundlesSequence.svg | 259 +++++++++--------- 1 file changed, 124 insertions(+), 135 deletions(-) diff --git a/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg b/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg index bba3cfc..60b8ee2 100644 --- a/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg +++ b/text/0006-BundleContext-Install-API/InstallBundlesSequence.svg @@ -1,288 +1,277 @@ - + - + - BundleContext::InstallBundles + BundleContext::InstallBundles - - BundleContext + + BundleContext - - BundleRegistry + + BundleRegistry - - BundlePrivate + + Bundle - - BundleStorageMemory + + BundlePrivate - - BundleArchive + + BundleStorageMemory - - BundleResourceContainer + + BundleArchive - - BundleManifest + + BundleResourceContainer - - Bundle + + BundleContext - - BundleContext + + BundleRegistry - - BundleRegistry + + Bundle - - BundlePrivate + + BundlePrivate - - BundleStorageMemory + + BundleStorageMemory - - BundleArchive + + BundleArchive - - BundleResourceContainer - - - - BundleManifest - - - - Bundle - - - + + BundleResourceContainer - + - + - + - + - + - + - + - + - + - - - - BundleContext::InstallBundles + + + + BundleContext::InstallBundles - - - - BundleRegistry:Install0 + + + + BundleRegistry:Install0 - - - - loop - foreach symbolicName : dirnames + + + + loop + foreach symbolicName : dirnames - - - - loop - foreach ba : barchives + + + + loop + foreach ba : barchives - InstallBundles + InstallBundles - Install + Install - GetAlreadyInstalledBundlesAtLocation + GetAlreadyInstalledBundlesAtLocation - Install0 + Install0 - vector<installedBundles> + vector<installedBundles> - vector<installedBundles> + vector<installedBundles> - Install0 + Install0 - GetTopLevelDirs + GetTopLevelDirs - vector<dirnames> + vector<dirnames> - CreateAndInsertArchive + CreateAndInsertArchive - + - new BundleArchive + new BundleArchive - new BundleArchive + new BundleArchive - create BundlePrivate object + create BundlePrivate object - new BundlePrivate + new BundlePrivate - create Bundle(BundlePrivate) + create Bundle(BundlePrivate) - new Bundle + new Bundle - vector<installedBundles> + vector<installedBundles> - + - - + + - + - + - - - - + + + + - - - - + + + + - + - + - + - - + + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - - Store new BundleArchive in barchives for later processing + + Store new BundleArchive in barchives for later processing - - Store created bundle in results vector to be returned + + Store created bundle in results vector to be returned From 86ce7076557bb8dcc3ce8de6750876917196a9c9 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Tue, 7 Jul 2020 09:28:48 -0400 Subject: [PATCH 06/22] add class diagram. review feedback. --- text/0006-BundleContext-Install-API.md | 61 +++- .../ClassDiagram.svg | 342 ++++++++++++++++++ 2 files changed, 392 insertions(+), 11 deletions(-) create mode 100644 text/0006-BundleContext-Install-API/ClassDiagram.svg diff --git a/text/0006-BundleContext-Install-API.md b/text/0006-BundleContext-Install-API.md index 93487f6..d532299 100644 --- a/text/0006-BundleContext-Install-API.md +++ b/text/0006-BundleContext-Install-API.md @@ -14,9 +14,19 @@ Allows users of the system to inject bundle manifests into the registry directly ## Detailed design +### Class Diagram + +This is a partial class diagram showing the relationship between the pieces of the system used during the BundleContext::InstallBundles operation. Only those classes and fields that participate in that operation are shown. + +![ClassDiagram](0006-BundleContext-Install-API/ClassDiagram.svg) + +### Sequence + ![InstallBundlesSequence](0006-BundleContext-Install-API/InstallBundlesSequence.svg) -### BundleContext +### Classes + +#### BundleContext ```c++ /** Wrapper around BundleRegistry::Install @@ -34,9 +44,8 @@ vector BundleContext::InstallBundles(std::string location * Modify old API to call new API with empty manifest map * Wrapper around call to BundleRegistry::Install * Refactor original API to call new API so we only have one implementation -* -### BundleRegistry +#### BundleRegistry ```c++ /** Install the manifest for the bundle at location into the registry. @@ -56,7 +65,7 @@ vector BundleRegistry::Install(std::string location * Update Install to accept a manifest to use for the installation instead of reading from bundle at location * If manifest is and empty map, then proceed with install action as before. This accomplishes a lazy loading of the manifest from the file. The manifest is only read from the file in the event that it is empty when asked for. -### BundleManifest +#### BundleManifest ```c++ /** Initialize manifest with content of AnyMap @@ -70,13 +79,13 @@ explicit BundleManifest::BundleManifest(const AnyMap& manifestHeaders); ``` -### BundlePrivate +#### BundlePrivate * Additional constructor that takes bundle manifest to be injected * Modified to fetch bundle manifest from BundleArchive instead of parsing the resource * Factor out manifest validation from constructor to allow for use from new constructor which takes a manifest to be injected as an argument -### BundleStorage +#### BundleStorage The **BundleStorage** class hierarchy is used for implementing different strategies for tracking bundle **BundleArchives**. There are two subclasses, but only one of them provides an actual implementation. They are **BundleStorageMemory**, and **BundleStorageFile**. It also tracks which bundles are to be auto started. @@ -102,25 +111,27 @@ The **BundleStorage** class hierarchy is used for implementing different strateg * It created a BundleResourceContainer for the location and called InsertArchives * Now the BundleResourceContainer is created if needed in BundleRegistry::GetAlreadyInstalldBundlesAtLocation and then passed in to BundleStorage::InsertArchive. -### BundleArchive +#### BundleArchive * Refactor and remove Data class simplifying design. * Add BundleManifest and constructor argument * Call BundleManifest::Parse from BundleArchive in the event that a manifest is not passed in at construction * Add new **GetManifest()** method which does a lazy loadof the manifest from the archive if and only if we don't already have the manifest. -### BundleResourceContainer +#### BundleResourceContainer * Add **manifest** argument to constructor * If the **manifest** is not empty, use it to initialize the **m_SortedToplevelDirs**, and leave **m_IsContainerOpen** set to false * if **manifest** is empty, no manifest injection is happening, so open up the archive and read the manifest from there, set set **m_IsContainerOpen** to true. -### BundleStorage(Memory) +#### BundleStorage(Memory) + +**BundleStorage** is a base class from which 2 different implementations have been derived: BundleStorageMemory, and BundleStorageFile. Only BundleStorageMemory, has an actual implementation. * Refactor * Remove **InsertArchives()** method and move loop to caller in **BundleRegistry::Install0** -### Other Changes +#### Other Changes * Various code cleanups (getting rid of "using std", exraneous cout output, etc.) @@ -133,6 +144,34 @@ The **BundleStorage** class hierarchy is used for implementing different strateg - Test for malformed manifest data. - I need to add manifest validation for required pieces of metadata for a bundle to work properly... we think that's only the bundle.symbolic_name. +## How we teach this + +> What names and terminology work best for these concepts and why? How is this +> idea best presented? As a continuation of existing CppMicroServices patterns, or as a +> wholly new one? + +> Would the acceptance of this proposal mean the CppMicroServices guides must be +> re-organized or altered? Does it change how CppMicroServices is taught to new users +> at any level? + +> How should this feature be introduced and taught to existing CppMicroServices +> users? + +## Drawbacks + +> Why should we *not* do this? Please consider the impact on teaching CppMicroServices, +> on the integration of this feature with other existing and planned features, +> on the impact of the API churn on existing apps, etc. + +> There are tradeoffs to choosing any path, please attempt to identify them here. + +## Alternatives + +> What other designs have been considered? What is the impact of not doing this? + +> This section could also include prior art, that is, how other frameworks in the same domain have solved this problem. + ## Unresolved questions -* \ No newline at end of file +> Optional, but suggested for first drafts. What parts of the design are still +> TBD? \ No newline at end of file diff --git a/text/0006-BundleContext-Install-API/ClassDiagram.svg b/text/0006-BundleContext-Install-API/ClassDiagram.svg new file mode 100644 index 0000000..70cd038 --- /dev/null +++ b/text/0006-BundleContext-Install-API/ClassDiagram.svg @@ -0,0 +1,342 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Produced by OmniGraffle 7.16 + 2020-07-07 13:26:09 +0000 + + + Canvas 1 + + + Layer 1 + + Class with Attributes + + Attributes + + + map<long, shared_ptr<BundleArchive>> archives + + + + Attributes + + + long nextFreeId + + + + Class Name + + + BundleStorageMemory + + + + + Class with Attributes + + Attributes + + + + Class Name + + + BundleStorage + + + + + Class with Attributes + + Attributes + + + shared_ptr<BundleContextPrivate> d + + + + Class Name + + + BundleContext + + + + + Class with Attributes + + Attributes + + + BundlePrivate* bundle + + + + Class Name + + + BundleContextPrivate + + + + + Association + + + + Class with Attributes + + Attributes + + + BundleManifest bundleManifest + + + + Attributes + + + shared_ptr<BundleContextPrivate> bundleContext + + + + Attributes + + + String location + + + + Attributes + + + long id + + + + Attributes + + + string bundleDir + + + + Attributes + + + shared_ptr<BundleArchive> archive + + + + Attributes + + + CoreBundleContext* coreCtx + + + + Attributes + + + TimeStamp timeStamp + + + + Attributes + + + String symbolicName + + + + Class Name + + + BundlePrivate + + + + + + + + + + + Class with Attributes + + Attributes + + + BundleManifest manifest + + + + Attributes + + + long bundleId + + + + Attributes + + + string location + + + + Attributes + + + shared_ptr<BundleResourceContainer> resourceContainer + + + + Class Name + + + BundleArchive + + + + Attributes + + + string resourcePrefix + + + + Attributes + + + BundleStorage* storage + + + + + + + + + + + Class with Attributes + + Attributes + + + set<string> m_SortedToplevelDirs + + + + Class Name + + + BundleResourceContainer + + + + + + + + + + 1..1 + + + + Class with Attributes + + Attributes + + + AnyMap m_Headers + + + + Class Name + + + BundleManifest + + + + + + + + + + + Class with Attributes + + Attributes + + + unique_ptr<BundleStorage> storage + + + + Class Name + + + CoreBundleContext + + + + + + + + + + + + + 1..1 + + + + + From 1d9353cdae77f1afbaab180ead487cb399b89860 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Tue, 7 Jul 2020 09:39:17 -0400 Subject: [PATCH 07/22] updated class diagram --- .../ClassDiagram.svg | 131 +++++++++++------- 1 file changed, 84 insertions(+), 47 deletions(-) diff --git a/text/0006-BundleContext-Install-API/ClassDiagram.svg b/text/0006-BundleContext-Install-API/ClassDiagram.svg index 70cd038..81966ce 100644 --- a/text/0006-BundleContext-Install-API/ClassDiagram.svg +++ b/text/0006-BundleContext-Install-API/ClassDiagram.svg @@ -1,6 +1,6 @@ - + @@ -39,33 +39,33 @@ Produced by OmniGraffle 7.16 - 2020-07-07 13:26:09 +0000 + 2020-07-07 13:37:59 +0000 Canvas 1 - + Layer 1 Class with Attributes Attributes - - + + map<long, shared_ptr<BundleArchive>> archives Attributes - - + + long nextFreeId Class Name - - + + BundleStorageMemory @@ -74,12 +74,12 @@ Class with Attributes Attributes - + Class Name - - + + BundleStorage @@ -205,79 +205,79 @@ Class with Attributes Attributes - - + + BundleManifest manifest Attributes - - + + long bundleId Attributes - - + + string location Attributes - - + + shared_ptr<BundleResourceContainer> resourceContainer Class Name - - + + BundleArchive Attributes - - + + string resourcePrefix Attributes - - + + BundleStorage* storage - + - + Class with Attributes Attributes - - + + set<string> m_SortedToplevelDirs Class Name - - + + BundleResourceContainer - + @@ -289,54 +289,91 @@ Class with Attributes Attributes - - + + AnyMap m_Headers Class Name - - + + BundleManifest - + - + Class with Attributes + + Attributes + + + BundleRegistry bundleRegistry + + Attributes - - + + unique_ptr<BundleStorage> storage Class Name - - + + CoreBundleContext - + - + - - + + 1..1 + + Class with Attributes + + Attributes + + + multimap<string, shared_ptr<BundlePrivate>> bundles + + + + Attributes + + + CoreBundleContext* coreCtx + + + + Class Name + + + BundleRegistry + + + + + + + + + From 0a7c22595c25a6c3fd77053768f1fa4ba0a2addf Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Thu, 9 Jul 2020 10:46:28 -0400 Subject: [PATCH 08/22] updated diagram with more information. --- .../ClassDiagram.svg | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/text/0006-BundleContext-Install-API/ClassDiagram.svg b/text/0006-BundleContext-Install-API/ClassDiagram.svg index 81966ce..62784ad 100644 --- a/text/0006-BundleContext-Install-API/ClassDiagram.svg +++ b/text/0006-BundleContext-Install-API/ClassDiagram.svg @@ -1,6 +1,6 @@ - + @@ -39,11 +39,11 @@ Produced by OmniGraffle 7.16 - 2020-07-07 13:37:59 +0000 + 2020-07-07 20:21:11 +0000 Canvas 1 - + Layer 1 @@ -289,21 +289,21 @@ Class with Attributes Attributes - - + + AnyMap m_Headers Class Name - - + + BundleManifest - + @@ -374,6 +374,9 @@ + + + From e5b88867dbee610dba739141138d79821f048a45 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Thu, 9 Jul 2020 11:06:36 -0400 Subject: [PATCH 09/22] Found another connection. --- .../ClassDiagram.svg | 63 ++++++++++--------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/text/0006-BundleContext-Install-API/ClassDiagram.svg b/text/0006-BundleContext-Install-API/ClassDiagram.svg index 62784ad..fa3ef17 100644 --- a/text/0006-BundleContext-Install-API/ClassDiagram.svg +++ b/text/0006-BundleContext-Install-API/ClassDiagram.svg @@ -39,7 +39,7 @@ Produced by OmniGraffle 7.16 - 2020-07-07 20:21:11 +0000 + 2020-07-09 15:05:56 +0000 Canvas 1 @@ -205,56 +205,56 @@ Class with Attributes Attributes - - + + BundleManifest manifest Attributes - - + + long bundleId Attributes - - + + string location Attributes - - + + shared_ptr<BundleResourceContainer> resourceContainer Class Name - - + + BundleArchive Attributes - - + + string resourcePrefix Attributes - - + + BundleStorage* storage - + @@ -277,7 +277,7 @@ - + @@ -303,10 +303,10 @@ - + - + Class with Attributes @@ -339,8 +339,8 @@ - - + + 1..1 @@ -348,34 +348,37 @@ Class with Attributes Attributes - - + + multimap<string, shared_ptr<BundlePrivate>> bundles Attributes - - + + CoreBundleContext* coreCtx Class Name - - + + BundleRegistry - + - + - + + + + From 1225670c99d9114fe05ca15e9cf4a1be3af7a626 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Mon, 13 Jul 2020 16:20:30 -0400 Subject: [PATCH 10/22] some review feedback. --- text/0006-BundleContext-Install-API.md | 47 +++++++++++++++++++++----- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/text/0006-BundleContext-Install-API.md b/text/0006-BundleContext-Install-API.md index d532299..2903878 100644 --- a/text/0006-BundleContext-Install-API.md +++ b/text/0006-BundleContext-Install-API.md @@ -2,16 +2,20 @@ - RFC PR: (in a subsequent commit to the PR, fill me with the PR's URL) - CppMicroServices Issue: (fill me with the URL of the CppMicroServices issue that necessisated this RFC, otherwise leave blank) -# Add BundleContext::InstallBundles API +# Add BundleManifest Injection Capabilities ## Summary -Provide a api to initialize the bundle registry with a bundle manifest +Provide a api to initialize the bundle registry with a bundle manifest during the BundleContext::InstallBundles operation by providing an optional argument containing a manifest. ## Motivation Allows users of the system to inject bundle manifests into the registry directly. We will be able to use this mechanism to speed up initial install of very large numbers of bundles. +For example, imagine an application architected for extesibility using plugins, and imagine that the basic operations of the application are all implemented using that architecture. When the application starts, the list of plugins along with a description needs to be made available to the user (as a requirement of the application, not CppMicroServices). In order to get the names and descriptions of the plugins, each bundle needs to be installed so that information from the manifest can be used. As the number of plugins grows, startup of the program takes longer and longer. After doing some analysis, the developer discovers that much of the application startup time is spent opening and reading the bundles for their manifests. + +This proposal discusses an approach that could solve this problem with an API for injecting manifests into the BundleRegistry directly, without opening and reading the manifests from the bundles. + ## Detailed design ### Class Diagram @@ -29,12 +33,37 @@ This is a partial class diagram showing the relationship between the pieces of t #### BundleContext ```c++ -/** Wrapper around BundleRegistry::Install -* -* @param location The absolute path to the location of the bundle whose manifest -* is to be installed -* @param manifest the manifest for the bundle. -*/ +/** + * Installs all bundles from the bundle library at the specified location. + * + * The following steps are required to install a bundle: + * -# If a bundle containing the same install location is already installed, the Bundle object for that + * bundle is returned. + * -# The bundle's associated resources are allocated. The associated resources minimally consist of a + * unique identifier and a persistent storage area if the platform has file system support. If this step + * fails, a std::runtime_error is thrown. + * -# A bundle event of type BundleEvent::BUNDLE_INSTALLED is fired. + * -# The Bundle object for the newly or previously installed bundle is returned. + * + * @remarks An install location is an absolute path to a shared library or executable file + * which may contain several bundles, i. e. acts as a bundle library. + * + * @remarks If the bundleManifest is passed in, it is installed. When the bundle is Started, the + * BundleManifest is NOT read from the file (as it has already been installed). In the event that + * the injected bundle manifest does NOT match the manifest in the bundle's file, the behavior of + * the system is undefined. That is, the content of the injected manifest and the manifest on disk + * are expected to be the same and are not compared. + * + * @param location The location of the bundle library to install. + * @param bundleManifest the manifest of the bundle at "location". If non-empty + * this will be used without opening the bundle at "location". Otherwise, the bundle will + * be opened and the manifest read from there. + * @return The Bundle objects of the installed bundle library. + * @throws std::runtime_error If the BundleContext is no longer valid, or if the installation failed. + * @throws std::logic_error If the framework instance is no longer active + * @throws std::invalid_argument If the location is not a valid UTF8 string + */ + vector BundleContext::InstallBundles(std::string location , std::vector manifest); @@ -140,7 +169,7 @@ The **BundleStorage** class hierarchy is used for implementing different strateg - Install bundle manifest at a location, and then retrieve bundle and check manifest - Install bundle manifest for a test bundle that exists. Then get the service, call a method through the service interface and made sure that it was successfully started. - Install bundle manifest for multiple bundles that exist on disk and then start each one and verify that they have been properly instantiated. -- Ensure Install bundle manifest for bundle already installed is ignored +- a bundle installed more than once always returns the originally installed bundle to the caller and does not create multiple entries in the bundle registry for the same bundle. - Test for malformed manifest data. - I need to add manifest validation for required pieces of metadata for a bundle to work properly... we think that's only the bundle.symbolic_name. From 75bf1a5244a5956bf56a2a501c79da3c9b56956e Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Tue, 14 Jul 2020 14:22:37 -0400 Subject: [PATCH 11/22] added some info to the "How do we teach this" section. --- text/0006-BundleContext-Install-API.md | 59 ++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/text/0006-BundleContext-Install-API.md b/text/0006-BundleContext-Install-API.md index 2903878..63c966c 100644 --- a/text/0006-BundleContext-Install-API.md +++ b/text/0006-BundleContext-Install-API.md @@ -55,7 +55,7 @@ This is a partial class diagram showing the relationship between the pieces of t * are expected to be the same and are not compared. * * @param location The location of the bundle library to install. - * @param bundleManifest the manifest of the bundle at "location". If non-empty + * @param bundleManifest the OPTIONAL manifest of the bundle at "location". If non-empty * this will be used without opening the bundle at "location". Otherwise, the bundle will * be opened and the manifest read from there. * @return The Bundle objects of the installed bundle library. @@ -175,16 +175,57 @@ The **BundleStorage** class hierarchy is used for implementing different strateg ## How we teach this -> What names and terminology work best for these concepts and why? How is this -> idea best presented? As a continuation of existing CppMicroServices patterns, or as a -> wholly new one? +| Term | Definition | +| ---- | ---------- | +| | | +| | | +| | | + +The optional bundleManifest argument to BundleContext::InstallBundles(location, bundleManifest) is a map that is formatted as follows: + +- **Key**: Symbolic Name of the bundle. +- **Value**: The manifest for the bundled named in the **Key**. + - The value should be an exact representation of the entire manifest for the named bundle and should follow all the same rules + - bundle.symbolic_name must be defined (and also must match the **Key** from above) + - If bundle.version is specified, it must be in the correct format: "major.minor.patch" + +The following JSON snippet represents an example of the content of the AnyMap passed in the optional second parameter to BundleContext::InstallBundles(location, bundleManifest) + +```json +{ + "SymbolicNameBundle1" : { + "bundle.activator" : false, + "bundle.version" : "1.0.0", + "bundle.symbolic_name" : "SymbolicNameBundle1", + "bundle.vendor" : "cppms co.", + ... any other data in the manifest + } +} + +``` + +There may be more than one bundle manifest in the case of a statically linked bundle. In this case, there needs to be an entry for each bundle present: + +```json +{ + "SymbolicNameBundle1" : { + "bundle.activator" : false, + "bundle.version" : "1.0.0", + "bundle.symbolic_name" : "SymbolicNameBundle1", + "bundle.vendor" : "cppms co.", + ... any other data in the manifest + }, + "SymbolicNameBundle2" : { + "bundle.activator" : false, + "bundle.version" : "1.0.0", + "bundle.symbolic_name" : "SymbolicNameBundle2", + "bundle.vendor" : "cppms co.", + ... any other data in the manifest + } +} +``` -> Would the acceptance of this proposal mean the CppMicroServices guides must be -> re-organized or altered? Does it change how CppMicroServices is taught to new users -> at any level? -> How should this feature be introduced and taught to existing CppMicroServices -> users? ## Drawbacks From 2fe0179a265b144394f0e43f93a144304b91c863 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Tue, 14 Jul 2020 14:51:33 -0400 Subject: [PATCH 12/22] removed unneeded table. --- text/0006-BundleContext-Install-API.md | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/text/0006-BundleContext-Install-API.md b/text/0006-BundleContext-Install-API.md index 63c966c..bf43450 100644 --- a/text/0006-BundleContext-Install-API.md +++ b/text/0006-BundleContext-Install-API.md @@ -175,12 +175,6 @@ The **BundleStorage** class hierarchy is used for implementing different strateg ## How we teach this -| Term | Definition | -| ---- | ---------- | -| | | -| | | -| | | - The optional bundleManifest argument to BundleContext::InstallBundles(location, bundleManifest) is a map that is formatted as follows: - **Key**: Symbolic Name of the bundle. @@ -229,11 +223,9 @@ There may be more than one bundle manifest in the case of a statically linked bu ## Drawbacks -> Why should we *not* do this? Please consider the impact on teaching CppMicroServices, -> on the integration of this feature with other existing and planned features, -> on the impact of the API churn on existing apps, etc. +There are some things to be aware of: -> There are tradeoffs to choosing any path, please attempt to identify them here. +* If a **bundleManifest** is passed in as the second argument to **BundleContext::InstallBundles(location, bundleManifest)** which does not match the bundleManifest actually contained in the bundle, the behavior is undefined. There is no error checking to make sure the manifests match. Doing so would be very time consuming and the whole point of this change is to provide a mechanism to reduce the overhead of installing, and eventually starting a bundle. ## Alternatives From 363aff54f7443044d16528f5323666d4f9d4ac48 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Thu, 16 Jul 2020 15:44:35 -0400 Subject: [PATCH 13/22] added link to OSGi spec for equivalent BundleContext::InstallBundles --- text/0006-BundleContext-Install-API.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0006-BundleContext-Install-API.md b/text/0006-BundleContext-Install-API.md index bf43450..50056ea 100644 --- a/text/0006-BundleContext-Install-API.md +++ b/text/0006-BundleContext-Install-API.md @@ -16,6 +16,8 @@ For example, imagine an application architected for extesibility using plugins, This proposal discusses an approach that could solve this problem with an API for injecting manifests into the BundleRegistry directly, without opening and reading the manifests from the bundles. +The inspiration for this API comes from the second [installBundles API](https://docs.osgi.org/specification/osgi.core/7.0.0/framework.api.html#org.osgi.framework.BundleContext.installBundle-String-InputStream-) in the OSGi spec which provides for a stream from which to read the bundle. + ## Detailed design ### Class Diagram From 844841ae31bd3ea693fff9ef88c316fc50e9cdae Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Mon, 20 Jul 2020 11:52:18 -0400 Subject: [PATCH 14/22] feedback. --- text/0006-BundleContext-Install-API.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0006-BundleContext-Install-API.md b/text/0006-BundleContext-Install-API.md index 50056ea..fc38df2 100644 --- a/text/0006-BundleContext-Install-API.md +++ b/text/0006-BundleContext-Install-API.md @@ -10,7 +10,7 @@ Provide a api to initialize the bundle registry with a bundle manifest during th ## Motivation -Allows users of the system to inject bundle manifests into the registry directly. We will be able to use this mechanism to speed up initial install of very large numbers of bundles. +Allows users of the system to install bundle manifests into the registry directly. We will be able to use this mechanism to speed up initial install of very large numbers of bundles. For example, imagine an application architected for extesibility using plugins, and imagine that the basic operations of the application are all implemented using that architecture. When the application starts, the list of plugins along with a description needs to be made available to the user (as a requirement of the application, not CppMicroServices). In order to get the names and descriptions of the plugins, each bundle needs to be installed so that information from the manifest can be used. As the number of plugins grows, startup of the program takes longer and longer. After doing some analysis, the developer discovers that much of the application startup time is spent opening and reading the bundles for their manifests. From 504b7d4e761df1c6429d10edbedb621da701ff6b Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Mon, 27 Jul 2020 08:49:15 -0400 Subject: [PATCH 15/22] modified sentence to better describe the intent of the API. --- text/0006-BundleContext-Install-API.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0006-BundleContext-Install-API.md b/text/0006-BundleContext-Install-API.md index fc38df2..38d5f4f 100644 --- a/text/0006-BundleContext-Install-API.md +++ b/text/0006-BundleContext-Install-API.md @@ -10,7 +10,7 @@ Provide a api to initialize the bundle registry with a bundle manifest during th ## Motivation -Allows users of the system to install bundle manifests into the registry directly. We will be able to use this mechanism to speed up initial install of very large numbers of bundles. +Allows users of the system to install bundle without opening the archive containing the bundle. We will be able to use this mechanism to speed up initial install of very large numbers of bundles. For example, imagine an application architected for extesibility using plugins, and imagine that the basic operations of the application are all implemented using that architecture. When the application starts, the list of plugins along with a description needs to be made available to the user (as a requirement of the application, not CppMicroServices). In order to get the names and descriptions of the plugins, each bundle needs to be installed so that information from the manifest can be used. As the number of plugins grows, startup of the program takes longer and longer. After doing some analysis, the developer discovers that much of the application startup time is spent opening and reading the bundles for their manifests. From 1bb34d419c2ab12c1e49d373838dbdb2a5c8bc63 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Thu, 3 Dec 2020 13:24:17 -0500 Subject: [PATCH 16/22] add ManifestCacheService rfc --- text/0009-ManifestCacheService.md | 115 ++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 text/0009-ManifestCacheService.md diff --git a/text/0009-ManifestCacheService.md b/text/0009-ManifestCacheService.md new file mode 100644 index 0000000..89fe728 --- /dev/null +++ b/text/0009-ManifestCacheService.md @@ -0,0 +1,115 @@ +- Start Date: 2020-12-03 +- RFC PR: (in a subsequent commit to the PR, fill me with the PR's URL) +- CppMicroServices Issue: (fill me with the URL of the CppMicroServices issue that necessisated this RFC, otherwise leave blank) + +# Bundle Manifest Cache Service Interface + +## Summary + +A service interface for retrieving bundle manifests + +## Motivation + +In applications with services numbering in the hundreds or thousands, the act of reading manifests from bundles can add significant overhead. By providing an implementation for this interface, an application may adopt a strategy for more quickly retrieving these manifests during the install operation. + +## Detailed design + +### Interface + +```c++ +/** + * Service interface for allowing users of cppmicroservices to inject manifests for bundles + * instead of having them read from the bundle files directly. + * + * If an implementation is not provided, or if the manifest returned is empty, the manifest + * is read from the bundle. + */ +class ManifestCacheService +{ + public: + + /** + * @param bundleLocation the location of the bundle for which to retrieve the manifest. + * the value of location is the same as that passed in to + * BundleContext::InstallBundles(). + * @return an r-value reference to an AnyMap suitable for moving into the + * BundleRegistry, either populated, or if not present in the cache, + * empty. + */ + AnyMap&& manifestForBundle(std::string const& bundleLocation) noexcept = 0; +}; +``` + +### Integration + +This interface provdes a mechanism for users of CppMicroServices to manage bundle manifests outside of the bundles themselves. + +* Remove manifest argument from the call stack: + + * Remove optional manifest argument from BundleContext::InstallBundles + * BundleRegistry::Install + * BundlRegistry::Install0 + +* Update BundleRegistry::Install0 + + * Find implementation of ManifestCacheService + + * If one exists, retrieve manifest to use + * otherwise use empty manifest + + * Pass manifest to CreateAndInsertArchive() as before. + + + +```c++ +AnyMap&& BundleRegistry::FetchBundleManifest(std::string const& location) +{ + auto cacheService = /* ... fetch from coreCtx */; + if (cacheService) { + return cacheService.ManifestForBundle(location); + } + return AnyMap { any_map::UNORDERED_MAP_CASEINSENSITIVE_KEYS }; +} + +std::vector BundleRegistry::Install0( + const std::string& location, + const std::shared_ptr& resCont, + const std::vector& alreadyInstalled) +{ +... + auto manifest = FetchBundleManifest(location); + + // Now, create a BundleArchive with the given manifest at 'entry' in the + // BundleResourceContainer, and remember the created BundleArchive here for later + // processing, including purging any items created if an exception is thrown. + barchives.push_back(coreCtx->storage->CreateAndInsertArchive(resCont, + symbolicName, + manifest)); +... + return installedBundles; +} + +``` + + + +A manifest from the cache is used only if a non-empty manifest is returned. Otherwise, the manifest is read from the bundle. + +## How we teach this + +Add documentation and an example implementation. + +## Drawbacks + +It is up to the implementor to manage the consistency of manifests available in the service with those in the bundles themselves. The behavior is undefined should a manifest provided for a bundle not meet the expectations of the bundle implementation. + +## Alternatives + +> What other designs have been considered? What is the impact of not doing this? + +> This section could also include prior art, that is, how other frameworks in the same domain have solved this problem. + +## Unresolved questions + +> Optional, but suggested for first drafts. What parts of the design are still +> TBD? \ No newline at end of file From 9f943f81043da707c90656eceafd307cde861af9 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Thu, 3 Dec 2020 13:34:12 -0500 Subject: [PATCH 17/22] Ready to push. --- text/0009-ManifestCacheService.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/text/0009-ManifestCacheService.md b/text/0009-ManifestCacheService.md index 89fe728..ec32f9c 100644 --- a/text/0009-ManifestCacheService.md +++ b/text/0009-ManifestCacheService.md @@ -6,8 +6,6 @@ ## Summary -A service interface for retrieving bundle manifests - ## Motivation In applications with services numbering in the hundreds or thousands, the act of reading manifests from bundles can add significant overhead. By providing an implementation for this interface, an application may adopt a strategy for more quickly retrieving these manifests during the install operation. From d6d5721b5b34647e6916829a6cb2bb206671084b Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Tue, 15 Dec 2020 15:41:46 -0500 Subject: [PATCH 18/22] fixed some typos. --- text/0009-ManifestCacheService.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text/0009-ManifestCacheService.md b/text/0009-ManifestCacheService.md index ec32f9c..8cec3c8 100644 --- a/text/0009-ManifestCacheService.md +++ b/text/0009-ManifestCacheService.md @@ -62,9 +62,9 @@ This interface provdes a mechanism for users of CppMicroServices to manage bundl ```c++ AnyMap&& BundleRegistry::FetchBundleManifest(std::string const& location) { - auto cacheService = /* ... fetch from coreCtx */; - if (cacheService) { - return cacheService.ManifestForBundle(location); + auto manifestCacheService = /* ... fetch from coreCtx */; + if (manifestCacheService) { + return manifestCacheService->ManifestForBundle(location); } return AnyMap { any_map::UNORDERED_MAP_CASEINSENSITIVE_KEYS }; } From 8bf7b852000d3e8db2b3c0dd7c93cde4340abb8d Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Tue, 15 Dec 2020 15:44:33 -0500 Subject: [PATCH 19/22] Added some explanatory notes --- text/0009-ManifestCacheService.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/text/0009-ManifestCacheService.md b/text/0009-ManifestCacheService.md index 8cec3c8..364446d 100644 --- a/text/0009-ManifestCacheService.md +++ b/text/0009-ManifestCacheService.md @@ -40,13 +40,7 @@ class ManifestCacheService ### Integration -This interface provdes a mechanism for users of CppMicroServices to manage bundle manifests outside of the bundles themselves. - -* Remove manifest argument from the call stack: - - * Remove optional manifest argument from BundleContext::InstallBundles - * BundleRegistry::Install - * BundlRegistry::Install0 +This interface provdes a mechanism for users of CppMicroServices to manage bundle manifests outside of the bundles themselves. Within the BundleRegistry code, if a ManifestCacheService is found, query the service for a manifest to use for the location being installed and use it if found. * Update BundleRegistry::Install0 From fa29ffa51d1d575762115896c487f7beff0262f9 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Tue, 15 Dec 2020 16:15:17 -0500 Subject: [PATCH 20/22] changed return typ to optional reference. --- text/0009-ManifestCacheService.md | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/text/0009-ManifestCacheService.md b/text/0009-ManifestCacheService.md index 364446d..2d3ab0d 100644 --- a/text/0009-ManifestCacheService.md +++ b/text/0009-ManifestCacheService.md @@ -30,11 +30,10 @@ class ManifestCacheService * @param bundleLocation the location of the bundle for which to retrieve the manifest. * the value of location is the same as that passed in to * BundleContext::InstallBundles(). - * @return an r-value reference to an AnyMap suitable for moving into the - * BundleRegistry, either populated, or if not present in the cache, - * empty. + * @return an optional reference to an AnyMap containing the manifest for the bundleLocation, */ - AnyMap&& manifestForBundle(std::string const& bundleLocation) noexcept = 0; + std::optional> + manifestForBundle(std::string const& bundleLocation) noexcept = 0; }; ``` @@ -54,13 +53,13 @@ This interface provdes a mechanism for users of CppMicroServices to manage bundl ```c++ -AnyMap&& BundleRegistry::FetchBundleManifest(std::string const& location) +std::optional> BundleRegistry::FetchBundleManifest(std::string const& location) { auto manifestCacheService = /* ... fetch from coreCtx */; if (manifestCacheService) { return manifestCacheService->ManifestForBundle(location); } - return AnyMap { any_map::UNORDERED_MAP_CASEINSENSITIVE_KEYS }; + return {}; } std::vector BundleRegistry::Install0( @@ -70,13 +69,19 @@ std::vector BundleRegistry::Install0( { ... auto manifest = FetchBundleManifest(location); - - // Now, create a BundleArchive with the given manifest at 'entry' in the - // BundleResourceContainer, and remember the created BundleArchive here for later - // processing, including purging any items created if an exception is thrown. - barchives.push_back(coreCtx->storage->CreateAndInsertArchive(resCont, - symbolicName, - manifest)); + if (manifest) { + // Now, create a BundleArchive with the given manifest at 'entry' in the + // BundleResourceContainer, and remember the created BundleArchive here for later + // processing, including purging any items created if an exception is thrown. + barchives.push_back(coreCtx->storage->CreateAndInsertArchive(resCont, + symbolicName, + manifest)); + } + else { + barchives.push_back(coreCtx->storage->CreateAndInsertArchive(resCont, + symbolicName)); + + } ... return installedBundles; } From 97a8aaa2e42a51c85d141496ef8283049956a785 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Wed, 16 Dec 2020 11:24:16 -0500 Subject: [PATCH 21/22] add some information about manifest precedence --- text/0009-ManifestCacheService.md | 85 +++++++++++++++++-------------- 1 file changed, 48 insertions(+), 37 deletions(-) diff --git a/text/0009-ManifestCacheService.md b/text/0009-ManifestCacheService.md index 2d3ab0d..5c4a27f 100644 --- a/text/0009-ManifestCacheService.md +++ b/text/0009-ManifestCacheService.md @@ -39,59 +39,70 @@ class ManifestCacheService ### Integration -This interface provdes a mechanism for users of CppMicroServices to manage bundle manifests outside of the bundles themselves. Within the BundleRegistry code, if a ManifestCacheService is found, query the service for a manifest to use for the location being installed and use it if found. +This interface provdes a mechanism for users of CppMicroServices to manage bundle manifests outside of the bundles themselves. Within the BundleRegistry code, if a ManifestCacheService is found, query the service for a manifest to use for the location being installed and use it if found. This means that there are several ways for a manifest to be specified for installation in the BundleRegistry. These different ways imply a precedence hierarchy for specifying the manifest: -* Update BundleRegistry::Install0 +* Highest precedence: BundleContext::InstallBundles() - a manifest provided as an argument here is always used. +* ManifestCacheService::manifestForBundle - if no manifest is provided as an argument, the service is consulted and if a manifest is returned, it is used +* Lowest precedence: manifest embedded within bundle - if a manifest is not provided as an argument, nor returned from the cache service, the manifest is read out of the bundle - * Find implementation of ManifestCacheService - * If one exists, retrieve manifest to use - * otherwise use empty manifest - * Pass manifest to CreateAndInsertArchive() as before. +```c++ +/* From BundleContext */ +std::vector BundleContext::InstallBundles( + const std::string& location, + const cppmicroservices::AnyMap& bundleManifest) +{ +... + return b->coreCtx->bundleRegistry.Install(location, b, bundleManifest); +} - +/* From BundleRegistry */ -```c++ -std::optional> BundleRegistry::FetchBundleManifest(std::string const& location) +AnyMap Bundleregistry::GetOverrideManifest(std::string const& location, + AnyMap const& bundleManifest) { - auto manifestCacheService = /* ... fetch from coreCtx */; - if (manifestCacheService) { - return manifestCacheService->ManifestForBundle(location); - } - return {}; + if (!bundleManifest.empty()) + return bundleManifest; // this will make a copy which can be moved into the registry + + auto emptyManifest = AnyMap { any_map::UNORDERED_MAP_CASEINSENSITIVE_KEYS) }; + auto manifestCacheService = /* ... fetch from coreCtx */; + if (manifestCacheService) { + auto manifest = manifestCacheService->manifestForBundle(lcoation); + + /* an empty map forces the manifest to be read from the bundle */ + return manifest.value_or(emptyManifest); + } + + /* an empty map forces the manifest to be read from the bundle */ + return emptyManifest; } -std::vector BundleRegistry::Install0( - const std::string& location, - const std::shared_ptr& resCont, - const std::vector& alreadyInstalled) +std::vector BundleRegistry::Install(std::string const& location, + BundlePrivate*, + cppmicroservices::AnyMap const& bundleManifest) { ... - auto manifest = FetchBundleManifest(location); - if (manifest) { - // Now, create a BundleArchive with the given manifest at 'entry' in the - // BundleResourceContainer, and remember the created BundleArchive here for later - // processing, including purging any items created if an exception is thrown. - barchives.push_back(coreCtx->storage->CreateAndInsertArchive(resCont, - symbolicName, - manifest)); - } - else { - barchives.push_back(coreCtx->storage->CreateAndInsertArchive(resCont, - symbolicName)); - - } + auto overrideManifest = GetOverrideManifest(location, bundleManifest); +... + + // Populate the resultingBundles and alreadyInstalled vectors with the appropriate data + // based on what bundles are already installed + auto resCont = GetAlreadyInstalledBundlesAtLocation(bundlesAtLocationRange, + location, + overrideManifest, + resultingBundles, + alreadyInstalled); + + // Perform the install, which will move the overrideManifest into the registry if it's not empty, + // or read the manifest out of the bundle otherwise. + auto newBundles = Install0(location, resCont, alreadyInstalled, overrideManifest); ... - return installedBundles; + } ``` - - -A manifest from the cache is used only if a non-empty manifest is returned. Otherwise, the manifest is read from the bundle. - ## How we teach this Add documentation and an example implementation. From e19005f9d19e6c427a04f266da0eab34fcb245d4 Mon Sep 17 00:00:00 2001 From: Michael Carney Date: Wed, 16 Dec 2020 11:26:12 -0500 Subject: [PATCH 22/22] fixed a type. --- text/0009-ManifestCacheService.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0009-ManifestCacheService.md b/text/0009-ManifestCacheService.md index 5c4a27f..493b457 100644 --- a/text/0009-ManifestCacheService.md +++ b/text/0009-ManifestCacheService.md @@ -68,7 +68,7 @@ AnyMap Bundleregistry::GetOverrideManifest(std::string const& location, auto emptyManifest = AnyMap { any_map::UNORDERED_MAP_CASEINSENSITIVE_KEYS) }; auto manifestCacheService = /* ... fetch from coreCtx */; if (manifestCacheService) { - auto manifest = manifestCacheService->manifestForBundle(lcoation); + auto manifest = manifestCacheService->manifestForBundle(location); /* an empty map forces the manifest to be read from the bundle */ return manifest.value_or(emptyManifest);