Merge lp:~marcustomlinson/unity-api/lp-1552082 into lp:unity-api

Proposed by Marcus Tomlinson on 2016-03-14
Status: Merged
Approved by: Michi Henning on 2016-03-30
Approved revision: 240
Merged at revision: 222
Proposed branch: lp:~marcustomlinson/unity-api/lp-1552082
Merge into: lp:unity-api
Diff against target: 958 lines (+604/-65)
7 files modified
CMakeLists.txt (+1/-1)
debian/changelog (+6/-0)
include/unity/util/IniParser.h (+62/-18)
src/unity/util/IniParser.cpp (+247/-16)
test/data/sample.ini (+2/-0)
test/gtest/unity/util/IniParser/CMakeLists.txt (+2/-0)
test/gtest/unity/util/IniParser/IniParser_test.cpp (+284/-30)
To merge this branch: bzr merge lp:~marcustomlinson/unity-api/lp-1552082
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Approve on 2016-03-30
Michi Henning (community) Approve on 2016-03-30
Paweł Stołowski 2016-03-14 Needs Fixing on 2016-03-16
Review via email: mp+288895@code.launchpad.net

Commit message

Add set*() methods to IniParser

To post a comment you must log in.
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:219
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/34/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/922
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/938
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/938
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/936
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/936/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/936
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/936/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/936
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/936/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/936
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/936/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/936
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/936/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/936
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/936/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/34/rebuild

review: Approve (continuous-integration)
Paweł Stołowski (stolowski) wrote :

Looks good! Only problem is missing file locking, mention in the review of https://code.launchpad.net/~marcustomlinson/unity-scopes-shell/lp-1552082/+merge/288918

Also, can you mention in the headers docs that sync() may throw? Can you add a test case for exception on sync()?

review: Needs Fixing
Marcus Tomlinson (marcustomlinson) wrote :

> Looks good! Only problem is missing file locking, mention in the review of
> https://code.launchpad.net/~marcustomlinson/unity-scopes-
> shell/lp-1552082/+merge/288918
>
> Also, can you mention in the headers docs that sync() may throw? Can you add a
> test case for exception on sync()?

done.

Marcus Tomlinson (marcustomlinson) wrote :

@Pawel, Alright, this is ready for review again.

Paweł Stołowski (stolowski) wrote :

Thanks Marcus. These changes look good when considered in isolation, but I'm afaird they have big impact in SettingsDB implementation in the scope. By having file locking implemented on the IniParser level, we're now locking twice (SettingsDB locks the file too); moreover there is some logic around in SettingsDB to give defaults if locking fails etc. If we want to go that route, then I suspect SettingsDB will need changes... Should we consider manual locking (by the called) instead? Let's discuss that in the standup.

review: Needs Fixing
Paweł Stołowski (stolowski) wrote :

> Thanks Marcus. These changes look good when considered in isolation, but I'm
> afaird they have big impact in SettingsDB implementation in the scope. By
> having file locking implemented on the IniParser level, we're now locking
> twice (SettingsDB locks the file too); moreover there is some logic around in
> SettingsDB to give defaults if locking fails etc. If we want to go that route,
> then I suspect SettingsDB will need changes... Should we consider manual
> locking (by the called) instead? Let's discuss that in the standup.

Uh, sorry, didn't mean to set it to 'needs fixing', but 'needs discussion' instead ;) As I said, nothing wrong with these changes in general, we just need to consider the implications.

review: Needs Information
Paweł Stołowski (stolowski) wrote :

Oh, actually there is a problem.

+ fl.l_type = F_RDLCK;

We're using a read-lock for both reading and writing.

review: Needs Fixing
Michi Henning (michihenning) wrote :

The locking here is at the wrong level of abstraction, I believe. It is not necessary to lock at this level because there will be many cases where the writer and reader are the same single process.

The locking needs to be added at the level above, which knows whether a particular file might be accessed concurrently.

We already have locking in SettingsDB in scopes API. The locking needs to be moved in to the shell, to do the equivalent of what QSettings does.

review: Needs Fixing
Marcus Tomlinson (marcustomlinson) wrote :

Indeed, for us to maintain behavioural compatibility, the file locking should occur on in unity-scopes-shell, as it was before via QSettings.

Michi Henning (michihenning) wrote :

One option to deal with this *might* be to make the SettingsDB class visible as part of the public API. That would allow the shell to just use it. Alternatively, steal the code from SettingsDB.cpp and put that into the shell. (It's not a lot of code.)

Marcus Tomlinson (marcustomlinson) wrote :

> Thanks Marcus. These changes look good when considered in isolation, but I'm
> afaird they have big impact in SettingsDB implementation in the scope. By
> having file locking implemented on the IniParser level, we're now locking
> twice (SettingsDB locks the file too); moreover there is some logic around in
> SettingsDB to give defaults if locking fails etc. If we want to go that route,
> then I suspect SettingsDB will need changes... Should we consider manual
> locking (by the called) instead? Let's discuss that in the standup.

Fixed in https://code.launchpad.net/~marcustomlinson/unity-scopes-shell/lp-1552082/+merge/288918

Marcus Tomlinson (marcustomlinson) wrote :

Ok, Michi made a good point this morning that the write functionality added to this class is looking too much like an after thought. As this lib is meant to serve a very wide audience, we need to ensure that the API is complete such that it is useful for wide range of applications (not just the shell plugin).

From a quick look over the API, one example of missing functionality is some way to delete groups and keys.

Let's hold back on landing this MP until I've managed to wrap up the API a little tighter.

Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:230
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/50/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1139
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1122
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1122
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1120
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1120/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1120
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1120/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1120
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1120/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1120
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1120/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1120
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1120/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1120
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1120/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/50/rebuild

review: Approve (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:235
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/55/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1170
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1149
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1149
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1147
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1147/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1147
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1147/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1147
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1147/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1147
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1147/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1147
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1147/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1147
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1147/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/55/rebuild

review: Approve (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:238
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/59/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1177
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1154
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1154
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1152
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1152/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1152
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1152/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1152
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1152/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1152
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1152/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1152
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1152/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1152
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1152/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/59/rebuild

review: Approve (continuous-integration)
Michi Henning (michihenning) wrote :

Looks good! Nice coverage too! :-)

Some very minor code and doc quibbles below...

In the constructor and destructor, there is no need to lock the mutex.

"To extract / insert a value, simply specify the group and key
 names to the get* / set* methods of this class respectively."

The "/" needs to be close set. I'd also remove the "*", and a comma needs to be added:

"To extract/insert a value, simply specify the group and key
 names to the get/set methods of this class, respectively."

"The get methods indicate errors by throwing LogicExceptions."

I'd remove the plural:

"The get methods indicate errors by throwing LogicException."

"Examples why this might happen is because a value can't be
coerced into the given type (i.e trying to convert the value
"hello" into a boolean)."

"i.e" is missing a full stop here. But I'd be inclined to delete this sentence entirely because almost the same information is provided in the doc for the read methods below. For completeness, I'd reword that comment as:

    /** @name Read Methods
     * These member functions provide read access to configuration entries by group and key.<br>
     * Attempts to retrieve a value as the wrong type (such as retrieving a string value
     * as an integer) or to retrieve a value for a non-existent group or
     * key throw LogicException.
     **/

In the summary comment for the class, I'd add that all methods are thread-safe.

    /** @name Write Methods
     * These member functions provide write access to configuration entries by group and key.<br>
     * Attempts to remove groups or keys that do not exist, throw LogicException.
      **/

Remove the comma before "throw LogicException." I would also add:

       The set methods replace the value for a key if the key exists. Calling a set method for
       a non-existent group and/or key creates the group and/or key.

    /** @name Sync Method
     * This member function will write unsaved changes back to the configuration file.<br>
     * A failure to write to file will throw a FileException.
      **/

I would get rid of the future tense:

    /** @name Sync Method
     * This member function writes unsaved changes back to the configuration file.<br>
     * A failure to write to the file throws a FileException.
     **/

review: Needs Fixing
Marcus Tomlinson (marcustomlinson) wrote :

> Looks good! Nice coverage too! :-)
>
> Some very minor code and doc quibbles below...
>
> In the constructor and destructor, there is no need to lock the mutex.

Sorry, I just had to have a look ;P

52.2.27 jussi.p | IniParser::IniParser(const char* filename)
52.2.12 jussi.p | {
120.1.1 michi.h | lock_guard<std::mutex> lock(internal::parser_mutex);

...

52.2.17 jussi.p | IniParser::~IniParser() noexcept
52.2.12 jussi.p | {
120.1.1 michi.h | lock_guard<std::mutex> lock(internal::parser_mutex);

>
> "To extract / insert a value, simply specify the group and key
> names to the get* / set* methods of this class respectively."
>
> The "/" needs to be close set. I'd also remove the "*", and a comma needs to
> be added:
>
> "To extract/insert a value, simply specify the group and key
> names to the get/set methods of this class, respectively."
>
> "The get methods indicate errors by throwing LogicExceptions."
>
> I'd remove the plural:
>
> "The get methods indicate errors by throwing LogicException."
>
> "Examples why this might happen is because a value can't be
> coerced into the given type (i.e trying to convert the value
> "hello" into a boolean)."
>
> "i.e" is missing a full stop here. But I'd be inclined to delete this sentence
> entirely because almost the same information is provided in the doc for the
> read methods below. For completeness, I'd reword that comment as:
>
> /** @name Read Methods
> * These member functions provide read access to configuration entries by
> group and key.<br>
> * Attempts to retrieve a value as the wrong type (such as retrieving a
> string value
> * as an integer) or to retrieve a value for a non-existent group or
> * key throw LogicException.
> **/
>
> In the summary comment for the class, I'd add that all methods are thread-
> safe.
>
> /** @name Write Methods
> * These member functions provide write access to configuration entries by
> group and key.<br>
> * Attempts to remove groups or keys that do not exist, throw
> LogicException.
> **/
>
> Remove the comma before "throw LogicException." I would also add:
>
> The set methods replace the value for a key if the key exists. Calling
> a set method for
> a non-existent group and/or key creates the group and/or key.
>
>
> /** @name Sync Method
> * This member function will write unsaved changes back to the
> configuration file.<br>
> * A failure to write to file will throw a FileException.
> **/
>
> I would get rid of the future tense:
>
> /** @name Sync Method
> * This member function writes unsaved changes back to the configuration
> file.<br>
> * A failure to write to the file throws a FileException.
> **/

Thanks Michi, done.

Michi Henning (michihenning) wrote :

> Sorry, I just had to have a look ;P

Yes, me bad. That was caused by a false positive from helgrind, from memory. The lock makes the FP go away, because helgrind doesn't understand atomics. I should have gone back and removed this again.

Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:239
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/61/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1190
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1167
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1167
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1165
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1165/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1165
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1165/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1165
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1165/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1165
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1165/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1165
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1165/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1165
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1165/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/61/rebuild

review: Approve (continuous-integration)
Michi Henning (michihenning) wrote :

Looks good, thanks!

review: Approve
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:240
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/62/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1192
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1169
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1169
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1167
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1167/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1167
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1167/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1167
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1167/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1167
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1167/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1167
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1167/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1167
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1167/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/62/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2015-08-11 14:05:12 +0000
3+++ CMakeLists.txt 2016-03-30 08:50:57 +0000
4@@ -122,7 +122,7 @@
5 # API version
6 set(UNITY_API_MAJOR 0)
7 set(UNITY_API_MINOR 1)
8-set(UNITY_API_MICRO 5)
9+set(UNITY_API_MICRO 6)
10 set(UNITY_API_VERSION "${UNITY_API_MAJOR}.${UNITY_API_MINOR}.${UNITY_API_MICRO}")
11
12 # API library
13
14=== modified file 'debian/changelog'
15--- debian/changelog 2016-03-22 07:48:55 +0000
16+++ debian/changelog 2016-03-30 08:50:57 +0000
17@@ -1,3 +1,9 @@
18+unity-api (7.109-0ubuntu1) UNRELEASED; urgency=medium
19+
20+ * Add set*() methods to IniParser
21+
22+ -- Marcus Tomlinson <marcus.tomlinson@canonical.com> Tue, 29 Mar 2016 08:32:15 +0200
23+
24 unity-api (7.108+16.04.20160322-0ubuntu1) xenial; urgency=medium
25
26 [ Lukáš Tinkl, Michał Sawicz ]
27
28=== modified file 'include/unity/util/IniParser.h'
29--- include/unity/util/IniParser.h 2014-07-03 05:00:00 +0000
30+++ include/unity/util/IniParser.h 2016-03-30 08:50:57 +0000
31@@ -22,8 +22,8 @@
32 #include <unity/SymbolExport.h>
33 #include <unity/util/DefinesPtrs.h>
34
35-#include<string>
36-#include<vector>
37+#include <string>
38+#include <vector>
39
40 namespace unity
41 {
42@@ -37,11 +37,11 @@
43 }
44
45 /**
46-\brief Helper class to read configuration files.
47+\brief Helper class to read and write configuration files.
48
49 This class reads configuration files in the .ini format
50 and provides for a simple and type safe way of extracting
51-information. A typical ini file looks like this:
52+and inserting information. A typical ini file looks like this:
53
54 ~~~
55 [group1]
56@@ -55,14 +55,17 @@
57 key2 = othervalue2
58 ~~~
59
60-To obtain a value, simply specify the group and key names to
61-the get* functions of this class. The array functions use
62-a semicolon as a separator.
63-
64-The get functions indicate errors by throwing LogicExceptions.
65-Examples why this might happen is because a value can't be
66-coerced into the given type (i.e trying to convert the value
67-"hello" into a boolean).
68+To extract/insert a value, simply specify the group and key
69+names to the get/set methods of this class, respectively.
70+The array methods use a semicolon as a separator.
71+
72+To write unsaved changes back to the configuration file, call
73+sync(). The sync() method will throw a FileException if it
74+fails to write to file.
75+
76+The get methods indicate errors by throwing LogicException.
77+
78+All methods are thread-safe.
79 */
80
81 class UNITY_API IniParser final {
82@@ -78,31 +81,72 @@
83 IniParser() = delete;
84 /// @endcond
85
86- /** @name Accessors
87- * These member functions provide access to configuration entries by group and key.
88- * Attempts to retrieve a value as the wrong type, such as retrieving a string value
89- * "abc" as an integer, throw LogicException.
90- **/
91 //{@
92+
93+ /** @name Read Methods
94+ * These member functions provide read access to configuration entries by group and key.<br>
95+ * Attempts to retrieve a value as the wrong type (such as retrieving a string value as an
96+ * integer) or to retrieve a value for a non-existent group or key throw LogicException.
97+ **/
98+
99 bool has_group(const std::string& group) const noexcept;
100 bool has_key(const std::string& group, const std::string& key) const;
101+
102 std::string get_string(const std::string& group, const std::string& key) const;
103 std::string get_locale_string(const std::string& group,
104 const std::string& key,
105 const std::string& locale = std::string()) const;
106 bool get_boolean(const std::string& group, const std::string& key) const;
107 int get_int(const std::string& group, const std::string& key) const;
108+ double get_double(const std::string& group, const std::string& key) const;
109
110 std::vector<std::string> get_string_array(const std::string& group, const std::string& key) const;
111 std::vector<std::string> get_locale_string_array(const std::string& group,
112 const std::string& key,
113 const std::string& locale = std::string()) const;
114+ std::vector<bool> get_boolean_array(const std::string& group, const std::string& key) const;
115 std::vector<int> get_int_array(const std::string& group, const std::string& key) const;
116- std::vector<bool> get_boolean_array(const std::string& group, const std::string& key) const;
117+ std::vector<double> get_double_array(const std::string& group, const std::string& key) const;
118
119 std::string get_start_group() const;
120 std::vector<std::string> get_groups() const;
121 std::vector<std::string> get_keys(const std::string& group) const;
122+
123+ /** @name Write Methods
124+ * These member functions provide write access to configuration entries by group and key.<br>
125+ * Attempts to remove groups or keys that do not exist throw LogicException.<br>
126+ * The set methods replace the value for a key if the key exists. Calling a set method for a
127+ * non-existent group and/or key creates the group and/or key.
128+ **/
129+
130+ bool remove_group(const std::string& group);
131+ bool remove_key(const std::string& group, const std::string& key);
132+
133+ void set_string(const std::string& group, const std::string& key, const std::string& value);
134+ void set_locale_string(const std::string& group,
135+ const std::string& key,
136+ const std::string& value,
137+ const std::string& locale = std::string());
138+ void set_boolean(const std::string& group, const std::string& key, bool value);
139+ void set_int(const std::string& group, const std::string& key, int value);
140+ void set_double(const std::string& group, const std::string& key, double value);
141+
142+ void set_string_array(const std::string& group, const std::string& key, const std::vector<std::string>& value);
143+ void set_locale_string_array(const std::string& group,
144+ const std::string& key,
145+ const std::vector<std::string>& value,
146+ const std::string& locale = std::string());
147+ void set_boolean_array(const std::string& group, const std::string& key, const std::vector<bool>& value);
148+ void set_int_array(const std::string& group, const std::string& key, const std::vector<int>& value);
149+ void set_double_array(const std::string& group, const std::string& key, const std::vector<double>& value);
150+
151+ /** @name Sync Method
152+ * This member function writes unsaved changes back to the configuration file.<br>
153+ * A failure to write to the file throws a FileException.
154+ **/
155+
156+ void sync();
157+
158 //@}
159
160 private:
161
162=== modified file 'src/unity/util/IniParser.cpp'
163--- src/unity/util/IniParser.cpp 2014-07-03 05:00:00 +0000
164+++ src/unity/util/IniParser.cpp 2016-03-30 08:50:57 +0000
165@@ -16,10 +16,12 @@
166 * Authored by: Jussi Pakkanen <jussi.pakkanen@canonical.com>
167 */
168
169-#include<unity/UnityExceptions.h>
170-#include<unity/util/IniParser.h>
171-#include<glib.h>
172-#include<mutex>
173+#include <unity/UnityExceptions.h>
174+#include <unity/util/IniParser.h>
175+
176+#include <mutex>
177+
178+#include <glib.h>
179
180 using namespace std;
181
182@@ -29,7 +31,6 @@
183 namespace util
184 {
185
186-
187 namespace internal
188 {
189
190@@ -37,6 +38,7 @@
191 {
192 GKeyFile *k;
193 string filename;
194+ bool dirty = false;
195 };
196
197 static std::mutex parser_mutex;
198@@ -68,8 +70,6 @@
199
200 IniParser::IniParser(const char* filename)
201 {
202- lock_guard<std::mutex> lock(internal::parser_mutex);
203-
204 GKeyFile* kf = g_key_file_new();
205 GError* e = nullptr;
206 if (!kf)
207@@ -95,14 +95,14 @@
208
209 IniParser::~IniParser() noexcept
210 {
211- lock_guard<std::mutex> lock(internal::parser_mutex);
212-
213 g_key_file_free(p->k);
214 delete p;
215 }
216
217 bool IniParser::has_group(const std::string& group) const noexcept
218 {
219+ lock_guard<std::mutex> lock(internal::parser_mutex);
220+
221 gboolean rval;
222 rval = g_key_file_has_group(p->k, group.c_str());
223 return rval;
224@@ -110,6 +110,8 @@
225
226 bool IniParser::has_key(const std::string& group, const std::string& key) const
227 {
228+ lock_guard<std::mutex> lock(internal::parser_mutex);
229+
230 gboolean rval;
231 GError* e = nullptr;
232 rval = g_key_file_has_key(p->k, group.c_str(), key.c_str(), &e);
233@@ -119,6 +121,8 @@
234
235 std::string IniParser::get_string(const std::string& group, const std::string& key) const
236 {
237+ lock_guard<std::mutex> lock(internal::parser_mutex);
238+
239 gchar* value;
240 GError* e = nullptr;
241 string result;
242@@ -131,6 +135,8 @@
243
244 std::string IniParser::get_locale_string(const std::string& group, const std::string& key, const std::string& locale) const
245 {
246+ lock_guard<std::mutex> lock(internal::parser_mutex);
247+
248 gchar* value;
249 GError* e = nullptr;
250 string result;
251@@ -143,6 +149,8 @@
252
253 bool IniParser::get_boolean(const std::string& group, const std::string& key) const
254 {
255+ lock_guard<std::mutex> lock(internal::parser_mutex);
256+
257 bool rval;
258 GError* e = nullptr;
259 rval = g_key_file_get_boolean(p->k, group.c_str(), key.c_str(), &e);
260@@ -152,6 +160,8 @@
261
262 int IniParser::get_int(const std::string& group, const std::string& key) const
263 {
264+ lock_guard<std::mutex> lock(internal::parser_mutex);
265+
266 int rval;
267 GError* e = nullptr;
268 rval = g_key_file_get_integer(p->k, group.c_str(), key.c_str(), &e);
269@@ -159,8 +169,21 @@
270 return rval;
271 }
272
273+double IniParser::get_double(const std::string& group, const std::string& key) const
274+{
275+ lock_guard<std::mutex> lock(internal::parser_mutex);
276+
277+ double rval;
278+ GError* e = nullptr;
279+ rval = g_key_file_get_double(p->k, group.c_str(), key.c_str(), &e);
280+ inspect_error(e, "Could not get double value", p->filename, group);
281+ return rval;
282+}
283+
284 std::vector<std::string> IniParser::get_string_array(const std::string& group, const std::string& key) const
285 {
286+ lock_guard<std::mutex> lock(internal::parser_mutex);
287+
288 vector<string> result;
289 GError* e = nullptr;
290 gchar** strlist;
291@@ -179,6 +202,8 @@
292 const std::string& key,
293 const std::string& locale) const
294 {
295+ lock_guard<std::mutex> lock(internal::parser_mutex);
296+
297 vector<string> result;
298 GError* e = nullptr;
299 gchar** strlist;
300@@ -194,8 +219,28 @@
301 return result;
302 }
303
304+vector<bool> IniParser::get_boolean_array(const std::string& group, const std::string& key) const
305+{
306+ lock_guard<std::mutex> lock(internal::parser_mutex);
307+
308+ vector<bool> result;
309+ GError* e = nullptr;
310+ gboolean* bools;
311+ gsize count;
312+ bools = g_key_file_get_boolean_list(p->k, group.c_str(), key.c_str(), &count, &e);
313+ inspect_error(e, "Could not get boolean array", p->filename, group);
314+ for (gsize i = 0; i < count; i++)
315+ {
316+ result.push_back(bools[i]);
317+ }
318+ g_free(bools);
319+ return result;
320+}
321+
322 vector<int> IniParser::get_int_array(const std::string& group, const std::string& key) const
323 {
324+ lock_guard<std::mutex> lock(internal::parser_mutex);
325+
326 vector<int> result;
327 GError* e = nullptr;
328 gint* ints;
329@@ -210,24 +255,28 @@
330 return result;
331 }
332
333-vector<bool> IniParser::get_boolean_array(const std::string& group, const std::string& key) const
334+vector<double> IniParser::get_double_array(const std::string& group, const std::string& key) const
335 {
336- vector<bool> result;
337+ lock_guard<std::mutex> lock(internal::parser_mutex);
338+
339+ vector<double> result;
340 GError* e = nullptr;
341- gboolean* bools;
342+ gdouble* doubles;
343 gsize count;
344- bools = g_key_file_get_boolean_list(p->k, group.c_str(), key.c_str(), &count, &e);
345- inspect_error(e, "Could not get boolean array", p->filename, group);
346+ doubles = g_key_file_get_double_list(p->k, group.c_str(), key.c_str(), &count, &e);
347+ inspect_error(e, "Could not get double array", p->filename, group);
348 for (gsize i = 0; i < count; i++)
349 {
350- result.push_back(bools[i]);
351+ result.push_back(doubles[i]);
352 }
353- g_free(bools);
354+ g_free(doubles);
355 return result;
356 }
357
358 string IniParser::get_start_group() const
359 {
360+ lock_guard<std::mutex> lock(internal::parser_mutex);
361+
362 gchar* sg = g_key_file_get_start_group(p->k);
363 string result(sg);
364 g_free(sg);
365@@ -236,6 +285,8 @@
366
367 vector<string> IniParser::get_groups() const
368 {
369+ lock_guard<std::mutex> lock(internal::parser_mutex);
370+
371 vector<string> result;
372 gsize count;
373 gchar** groups = g_key_file_get_groups(p->k, &count);
374@@ -249,6 +300,8 @@
375
376 vector<string> IniParser::get_keys(const std::string& group) const
377 {
378+ lock_guard<std::mutex> lock(internal::parser_mutex);
379+
380 vector<string> result;
381 GError* e = nullptr;
382 gchar** strlist;
383@@ -264,6 +317,184 @@
384 return result;
385 }
386
387+bool IniParser::remove_group(const std::string& group)
388+{
389+ lock_guard<std::mutex> lock(internal::parser_mutex);
390+
391+ gboolean rval;
392+ GError* e = nullptr;
393+ rval = g_key_file_remove_group(p->k, group.c_str(), &e);
394+ inspect_error(e, "Error removing group", p->filename, group);
395+ return rval;
396+}
397+
398+bool IniParser::remove_key(const std::string& group, const std::string& key)
399+{
400+ lock_guard<std::mutex> lock(internal::parser_mutex);
401+
402+ gboolean rval;
403+ GError* e = nullptr;
404+ rval = g_key_file_remove_key(p->k, group.c_str(), key.c_str(), &e);
405+ inspect_error(e, "Error removing key", p->filename, group);
406+ return rval;
407+}
408+
409+void IniParser::set_string(const std::string& group, const std::string& key, const std::string& value)
410+{
411+ lock_guard<std::mutex> lock(internal::parser_mutex);
412+
413+ g_key_file_set_string(p->k, group.c_str(), key.c_str(), value.c_str());
414+ p->dirty = true;
415+}
416+
417+void IniParser::set_locale_string(const std::string& group, const std::string& key,
418+ const std::string& value, const std::string& locale)
419+{
420+ lock_guard<std::mutex> lock(internal::parser_mutex);
421+
422+ g_key_file_set_locale_string(p->k, group.c_str(), key.c_str(), locale.c_str(), value.c_str());
423+ p->dirty = true;
424+}
425+
426+void IniParser::set_boolean(const std::string& group, const std::string& key, bool value)
427+{
428+ lock_guard<std::mutex> lock(internal::parser_mutex);
429+
430+ g_key_file_set_boolean(p->k, group.c_str(), key.c_str(), value);
431+ p->dirty = true;
432+}
433+
434+void IniParser::set_int(const std::string& group, const std::string& key, int value)
435+{
436+ lock_guard<std::mutex> lock(internal::parser_mutex);
437+
438+ g_key_file_set_integer(p->k, group.c_str(), key.c_str(), value);
439+ p->dirty = true;
440+}
441+
442+void IniParser::set_double(const std::string& group, const std::string& key, double value)
443+{
444+ lock_guard<std::mutex> lock(internal::parser_mutex);
445+
446+ g_key_file_set_double(p->k, group.c_str(), key.c_str(), value);
447+ p->dirty = true;
448+}
449+
450+void IniParser::set_string_array(const std::string& group, const std::string& key,
451+ const std::vector<std::string>& value)
452+{
453+ lock_guard<std::mutex> lock(internal::parser_mutex);
454+
455+ int count = value.size();
456+ gchar** strlist = g_new(gchar*, count+1);
457+
458+ for (int i = 0; i < count; ++i)
459+ {
460+ strlist[i] = g_strdup(value[i].c_str());
461+ }
462+ strlist[count] = nullptr;
463+
464+ g_key_file_set_string_list(p->k, group.c_str(), key.c_str(), strlist, count);
465+ p->dirty = true;
466+
467+ g_strfreev(strlist);
468+}
469+
470+void IniParser::set_locale_string_array(const std::string& group, const std::string& key,
471+ const std::vector<std::string>& value, const std::string& locale)
472+{
473+ lock_guard<std::mutex> lock(internal::parser_mutex);
474+
475+ int count = value.size();
476+ gchar** strlist = g_new(gchar*, count+1);
477+
478+ for (int i = 0; i < count; ++i)
479+ {
480+ strlist[i] = g_strdup(value[i].c_str());
481+ }
482+ strlist[count] = nullptr;
483+
484+ g_key_file_set_locale_string_list(p->k, group.c_str(), key.c_str(), locale.c_str(), strlist, count);
485+ p->dirty = true;
486+
487+ g_strfreev(strlist);
488+}
489+
490+void IniParser::set_boolean_array(const std::string& group, const std::string& key, const std::vector<bool>& value)
491+{
492+ lock_guard<std::mutex> lock(internal::parser_mutex);
493+
494+ int count = value.size();
495+ gboolean* boollist = g_new(gboolean, count);
496+
497+ for (int i = 0; i < count; ++i)
498+ {
499+ boollist[i] = value[i];
500+ }
501+
502+ g_key_file_set_boolean_list(p->k, group.c_str(), key.c_str(), boollist, count);
503+ p->dirty = true;
504+
505+ g_free(boollist);
506+}
507+
508+void IniParser::set_int_array(const std::string& group, const std::string& key, const std::vector<int>& value)
509+{
510+ lock_guard<std::mutex> lock(internal::parser_mutex);
511+
512+ int count = value.size();
513+ gint* intlist = g_new(gint, count);
514+
515+ for (int i = 0; i < count; ++i)
516+ {
517+ intlist[i] = value[i];
518+ }
519+
520+ g_key_file_set_integer_list(p->k, group.c_str(), key.c_str(), intlist, count);
521+ p->dirty = true;
522+
523+ g_free(intlist);
524+}
525+
526+void IniParser::set_double_array(const std::string& group, const std::string& key, const std::vector<double>& value)
527+{
528+ lock_guard<std::mutex> lock(internal::parser_mutex);
529+
530+ int count = value.size();
531+ gdouble* doublelist = g_new(gdouble, count);
532+
533+ for (int i = 0; i < count; ++i)
534+ {
535+ doublelist[i] = value[i];
536+ }
537+
538+ g_key_file_set_double_list(p->k, group.c_str(), key.c_str(), doublelist, count);
539+ p->dirty = true;
540+
541+ g_free(doublelist);
542+}
543+
544+void IniParser::sync()
545+{
546+ lock_guard<std::mutex> lock(internal::parser_mutex);
547+
548+ if (p->dirty)
549+ {
550+ GError* e = nullptr;
551+ if (!g_key_file_save_to_file(p->k, p->filename.c_str(), &e))
552+ {
553+ string message = "Could not write ini file ";
554+ message += p->filename;
555+ message += ": ";
556+ message += e->message;
557+ int errnum = e->code;
558+ g_error_free(e);
559+ throw FileException(message, errnum);
560+ }
561+
562+ p->dirty = false;
563+ }
564+}
565
566 } // namespace util
567
568
569=== modified file 'test/data/sample.ini'
570--- test/data/sample.ini 2014-07-03 05:00:00 +0000
571+++ test/data/sample.ini 2016-03-30 08:50:57 +0000
572@@ -1,6 +1,7 @@
573 [first]
574
575 intvalue = 1
576+doublevalue = 2.345
577 boolvalue = true
578 stringvalue = hello
579 locstring = world
580@@ -17,3 +18,4 @@
581 boolvalue = false
582 stringvalue = there
583 intarray = 4;5;6;78;8;9;9;345;3
584+doublearray = 4.5;6.78;9;10.11
585
586=== modified file 'test/gtest/unity/util/IniParser/CMakeLists.txt'
587--- test/gtest/unity/util/IniParser/CMakeLists.txt 2013-06-06 12:02:05 +0000
588+++ test/gtest/unity/util/IniParser/CMakeLists.txt 2016-03-30 08:50:57 +0000
589@@ -1,4 +1,6 @@
590 add_executable(IniParser_test IniParser_test.cpp)
591 target_link_libraries(IniParser_test ${LIBS} ${TESTLIBS})
592
593+add_definitions(-DTEST_RUNTIME_PATH="${CMAKE_CURRENT_BINARY_DIR}")
594+
595 add_test(IniParser IniParser_test)
596
597=== modified file 'test/gtest/unity/util/IniParser/IniParser_test.cpp'
598--- test/gtest/unity/util/IniParser/IniParser_test.cpp 2014-07-03 05:00:00 +0000
599+++ test/gtest/unity/util/IniParser/IniParser_test.cpp 2016-03-30 08:50:57 +0000
600@@ -26,6 +26,7 @@
601 using namespace unity::util;
602
603 #define INI_FILE UNITY_API_TEST_DATADIR "/sample.ini"
604+#define INI_TEMP_FILE TEST_RUNTIME_PATH "/temp.ini"
605
606 TEST(IniParser, basic)
607 {
608@@ -45,11 +46,23 @@
609 {
610 IniParser conf(INI_FILE);
611
612- ASSERT_TRUE(conf.has_group("first"));
613+ ASSERT_FALSE(conf.has_key("first", "missingvalue"));
614 ASSERT_FALSE(conf.has_group("nonexisting"));
615
616 ASSERT_TRUE(conf.has_key("first", "stringvalue"));
617- ASSERT_FALSE(conf.has_key("first", "missingvalue"));
618+ ASSERT_TRUE(conf.has_group("first"));
619+
620+ ASSERT_TRUE(conf.remove_key("first", "stringvalue"));
621+ EXPECT_THROW(conf.remove_key("first", "missingvalue"), LogicException);
622+
623+ ASSERT_FALSE(conf.has_key("first", "stringvalue"));
624+ ASSERT_TRUE(conf.has_group("first"));
625+
626+ ASSERT_TRUE(conf.remove_group("first"));
627+ EXPECT_THROW(conf.remove_group("nonexisting"), LogicException);
628+
629+ EXPECT_THROW(conf.has_key("first", "stringvalue"), LogicException);
630+ ASSERT_FALSE(conf.has_group("first"));
631 }
632
633 TEST(IniParser, simpleQueries)
634@@ -64,11 +77,13 @@
635 ASSERT_EQ("second", groups[1]);
636
637 vector<string> firstKeys = conf.get_keys("first");
638- ASSERT_EQ(10, firstKeys.size());
639- ASSERT_EQ("boolvalue", firstKeys[1]);
640+ ASSERT_EQ(11, firstKeys.size());
641+ ASSERT_EQ("boolvalue", firstKeys[2]);
642
643 ASSERT_EQ(conf.get_string("first", "stringvalue"), "hello");
644 ASSERT_EQ(1, conf.get_int("first", "intvalue"));
645+ ASSERT_EQ(1, conf.get_double("first", "intvalue"));
646+ ASSERT_EQ(2.345, conf.get_double("first", "doublevalue"));
647 ASSERT_FALSE(conf.get_boolean("second", "boolvalue"));
648
649 ASSERT_EQ("world", conf.get_string("first", "locstring"));
650@@ -92,6 +107,18 @@
651 ASSERT_EQ(4, intArr[0]);
652 ASSERT_EQ(3, intArr[8]);
653
654+ vector<double> intToDblArr = conf.get_double_array("second", "intarray");
655+ ASSERT_EQ(9, intToDblArr.size());
656+ ASSERT_EQ(4, intToDblArr[0]);
657+ ASSERT_EQ(3, intToDblArr[8]);
658+
659+ vector<double> dblArr = conf.get_double_array("second", "doublearray");
660+ ASSERT_EQ(4, dblArr.size());
661+ ASSERT_EQ(4.5, dblArr[0]);
662+ ASSERT_EQ(6.78, dblArr[1]);
663+ ASSERT_EQ(9, dblArr[2]);
664+ ASSERT_EQ(10.11, dblArr[3]);
665+
666 vector<bool> boolArr = conf.get_boolean_array("first", "boolarray");
667 ASSERT_EQ(3, boolArr.size());
668 ASSERT_TRUE(boolArr[0]);
669@@ -115,36 +142,263 @@
670 {
671 IniParser conf(INI_FILE);
672
673- try {
674+ try
675+ {
676 conf.get_string("foo", "bar");
677- FAIL();
678- } catch(const LogicException &e) {
679- }
680- try {
681+ }
682+ catch (const LogicException& e)
683+ {
684+ EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
685+ }
686+ try
687+ {
688 conf.get_locale_string("foo", "bar");
689- FAIL();
690- } catch(const LogicException &e) {
691- }
692- try {
693+ }
694+ catch (const LogicException& e)
695+ {
696+ EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
697+ }
698+ try
699+ {
700 conf.get_int("foo", "bar");
701- FAIL();
702- } catch(const LogicException &e) {
703- }
704- try {
705+ }
706+ catch (const LogicException& e)
707+ {
708+ EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
709+ }
710+ try
711+ {
712+ conf.get_int("first", "doublevalue");
713+ }
714+ catch (const LogicException& e)
715+ {
716+ EXPECT_NE(string::npos, string(e.what()).find("Key file contains key 'doublevalue' in group 'first' which has a value that cannot be interpreted."));
717+ }
718+ try
719+ {
720 conf.get_boolean("foo", "bar");
721- FAIL();
722- } catch(const LogicException &e) {
723- }
724-
725- try {
726+ }
727+ catch (const LogicException& e)
728+ {
729+ EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
730+ }
731+ try
732+ {
733 conf.get_int_array("first", "array");
734- FAIL();
735- } catch(const LogicException &e) {
736- }
737-
738- try {
739+ }
740+ catch (const LogicException& e)
741+ {
742+ EXPECT_NE(string::npos, string(e.what()).find("Value 'foo' cannot be interpreted as a number."));
743+ }
744+ try
745+ {
746+ conf.get_int_array("second", "doublearray");
747+ }
748+ catch (const LogicException& e)
749+ {
750+ EXPECT_NE(string::npos, string(e.what()).find("Value '4.5' cannot be interpreted as a number."));
751+ }
752+ try
753+ {
754 conf.get_boolean_array("first", "array");
755- FAIL();
756- } catch(const LogicException &e) {
757- }
758+ }
759+ catch (const LogicException& e)
760+ {
761+ EXPECT_NE(string::npos, string(e.what()).find("Value 'foo' cannot be interpreted as a boolean."));
762+ }
763+}
764+
765+TEST(IniParser, writeValues)
766+{
767+ // Create an empty ini file for writing
768+ auto f = fopen(INI_TEMP_FILE, "w");
769+ fclose(f);
770+
771+ IniParser conf(INI_TEMP_FILE);
772+
773+ conf.set_string("g1", "k1", "s0;s1");
774+ conf.set_locale_string("g1", "k2", "s2", "en");
775+ conf.set_locale_string("g1", "k2", "s3", "pt_BR");
776+
777+ // Check temp file before sync()
778+ {
779+ IniParser conf2(INI_TEMP_FILE);
780+ EXPECT_EQ("s0;s1", conf.get_string("g1", "k1"));
781+ EXPECT_THROW(conf2.get_string("g1", "k1"), LogicException);
782+
783+ EXPECT_EQ("s2", conf.get_locale_string("g1", "k2", "en"));
784+ EXPECT_THROW(conf2.get_locale_string("g1", "k2", "en"), LogicException);
785+
786+ EXPECT_EQ("s3", conf.get_locale_string("g1", "k2", "pt_BR"));
787+ EXPECT_THROW(conf2.get_locale_string("g1", "k2", "pt_BR"), LogicException);
788+ }
789+
790+ // Sync
791+ conf.sync();
792+
793+ // Write some post-sync values
794+ conf.set_boolean("g2", "k1", false);
795+ conf.set_int("g2", "k2", 123);
796+ conf.set_double("g3", "k3", 4.56789);
797+
798+ // Check temp file after first sync()
799+ {
800+ IniParser conf2(INI_TEMP_FILE);
801+ EXPECT_EQ("s0;s1", conf.get_string("g1", "k1"));
802+ EXPECT_EQ("s0;s1", conf2.get_string("g1", "k1"));
803+
804+ EXPECT_EQ("s2", conf.get_locale_string("g1", "k2", "en"));
805+ EXPECT_EQ("s2", conf2.get_locale_string("g1", "k2", "en"));
806+
807+ EXPECT_EQ("s3", conf.get_locale_string("g1", "k2", "pt_BR"));
808+ EXPECT_EQ("s3", conf2.get_locale_string("g1", "k2", "pt_BR"));
809+
810+ EXPECT_EQ(false, conf.get_boolean("g2", "k1"));
811+ EXPECT_THROW(conf2.get_boolean("g2", "k1"), LogicException);
812+
813+ EXPECT_EQ(123, conf.get_int("g2", "k2"));
814+ EXPECT_THROW(conf2.get_int("g2", "k2"), LogicException);
815+
816+ EXPECT_EQ(4.56789, conf.get_double("g3", "k3"));
817+ EXPECT_THROW(conf2.get_double("g3", "k3"), LogicException);
818+ }
819+
820+ // Sync
821+ conf.sync();
822+
823+ // Check temp file after second sync()
824+ {
825+ IniParser conf2(INI_TEMP_FILE);
826+ EXPECT_EQ("s0;s1", conf.get_string("g1", "k1"));
827+ EXPECT_EQ("s0;s1", conf2.get_string("g1", "k1"));
828+
829+ EXPECT_EQ("s2", conf.get_locale_string("g1", "k2", "en"));
830+ EXPECT_EQ("s2", conf2.get_locale_string("g1", "k2", "en"));
831+
832+ EXPECT_EQ("s3", conf.get_locale_string("g1", "k2", "pt_BR"));
833+ EXPECT_EQ("s3", conf2.get_locale_string("g1", "k2", "pt_BR"));
834+
835+ EXPECT_EQ(false, conf.get_boolean("g2", "k1"));
836+ EXPECT_EQ(false, conf2.get_boolean("g2", "k1"));
837+
838+ EXPECT_EQ(123, conf.get_int("g2", "k2"));
839+ EXPECT_EQ(123, conf2.get_int("g2", "k2"));
840+
841+ EXPECT_EQ(4.56789, conf.get_double("g3", "k3"));
842+ EXPECT_EQ(4.56789, conf2.get_double("g3", "k3"));
843+ }
844+}
845+
846+template<class T>
847+void EXPECT_ARRAY_EQ(const vector<T>& expected, const vector<T>& actual)
848+{
849+ EXPECT_TRUE(expected == actual);
850+}
851+
852+TEST(IniParser, writeArrays)
853+{
854+ // Create an empty ini file for writing
855+ auto f = fopen(INI_TEMP_FILE, "w");
856+ fclose(f);
857+
858+ IniParser conf(INI_TEMP_FILE);
859+
860+ conf.set_string_array("g1", "k1", {"s0;s1", "s2", "s3"});
861+ conf.set_locale_string_array("g1", "k2", {"s4", "s5", "s6"}, "en");
862+ conf.set_locale_string_array("g1", "k2", {"s7", "s8", "s9=s10"}, "pt_BR");
863+
864+ // Check temp file before sync()
865+ {
866+ IniParser conf2(INI_TEMP_FILE);
867+ EXPECT_ARRAY_EQ({"s0;s1", "s2", "s3"}, conf.get_string_array("g1", "k1"));
868+ EXPECT_THROW(conf2.get_string_array("g1", "k1"), LogicException);
869+
870+ EXPECT_ARRAY_EQ({"s4", "s5", "s6"}, conf.get_locale_string_array("g1", "k2", "en"));
871+ EXPECT_THROW(conf2.get_locale_string_array("g1", "k2", "en"), LogicException);
872+
873+ EXPECT_ARRAY_EQ({"s7", "s8", "s9=s10"}, conf.get_locale_string_array("g1", "k2", "pt_BR"));
874+ EXPECT_THROW(conf2.get_locale_string_array("g1", "k2", "pt_BR"), LogicException);
875+ }
876+
877+ // Sync
878+ conf.sync();
879+
880+ // Write some post-sync values
881+ conf.set_boolean_array("g2", "k1", {true, false, false});
882+ conf.set_int_array("g2", "k2", {123, 456789, 101112});
883+ conf.set_double_array("g2", "k3", {13.14, 15.161718, 19.2});
884+
885+ // Check temp file after first sync()
886+ {
887+ IniParser conf2(INI_TEMP_FILE);
888+ EXPECT_ARRAY_EQ({"s0;s1", "s2", "s3"}, conf.get_string_array("g1", "k1"));
889+ EXPECT_ARRAY_EQ({"s0;s1", "s2", "s3"}, conf2.get_string_array("g1", "k1"));
890+
891+ EXPECT_ARRAY_EQ({"s4", "s5", "s6"}, conf.get_locale_string_array("g1", "k2", "en"));
892+ EXPECT_ARRAY_EQ({"s4", "s5", "s6"}, conf2.get_locale_string_array("g1", "k2", "en"));
893+
894+ EXPECT_ARRAY_EQ({"s7", "s8", "s9=s10"}, conf.get_locale_string_array("g1", "k2", "pt_BR"));
895+ EXPECT_ARRAY_EQ({"s7", "s8", "s9=s10"}, conf2.get_locale_string_array("g1", "k2", "pt_BR"));
896+
897+ EXPECT_ARRAY_EQ({true, false, false}, conf.get_boolean_array("g2", "k1"));
898+ EXPECT_THROW(conf2.get_boolean_array("g2", "k1"), LogicException);
899+
900+ EXPECT_ARRAY_EQ({123, 456789, 101112}, conf.get_int_array("g2", "k2"));
901+ EXPECT_THROW(conf2.get_int_array("g2", "k2"), LogicException);
902+
903+ EXPECT_ARRAY_EQ({13.14, 15.161718, 19.2}, conf.get_double_array("g2", "k3"));
904+ EXPECT_THROW(conf2.get_double_array("g2", "k3"), LogicException);
905+ }
906+
907+ // Sync
908+ conf.sync();
909+
910+ // Check temp file after second sync()
911+ {
912+ IniParser conf2(INI_TEMP_FILE);
913+ EXPECT_ARRAY_EQ({"s0;s1", "s2", "s3"}, conf.get_string_array("g1", "k1"));
914+ EXPECT_ARRAY_EQ({"s0;s1", "s2", "s3"}, conf2.get_string_array("g1", "k1"));
915+
916+ EXPECT_ARRAY_EQ({"s4", "s5", "s6"}, conf.get_locale_string_array("g1", "k2", "en"));
917+ EXPECT_ARRAY_EQ({"s4", "s5", "s6"}, conf2.get_locale_string_array("g1", "k2", "en"));
918+
919+ EXPECT_ARRAY_EQ({"s7", "s8", "s9=s10"}, conf.get_locale_string_array("g1", "k2", "pt_BR"));
920+ EXPECT_ARRAY_EQ({"s7", "s8", "s9=s10"}, conf2.get_locale_string_array("g1", "k2", "pt_BR"));
921+
922+ EXPECT_ARRAY_EQ({true, false, false}, conf.get_boolean_array("g2", "k1"));
923+ EXPECT_ARRAY_EQ({true, false, false}, conf2.get_boolean_array("g2", "k1"));
924+
925+ EXPECT_ARRAY_EQ({123, 456789, 101112}, conf.get_int_array("g2", "k2"));
926+ EXPECT_ARRAY_EQ({123, 456789, 101112}, conf2.get_int_array("g2", "k2"));
927+
928+ EXPECT_ARRAY_EQ({13.14, 15.161718, 19.2}, conf.get_double_array("g2", "k3"));
929+ EXPECT_ARRAY_EQ({13.14, 15.161718, 19.2}, conf2.get_double_array("g2", "k3"));
930+ }
931+}
932+
933+TEST(IniParser, writeError)
934+{
935+ // Create an empty ini file for writing
936+ auto f = fopen(INI_TEMP_FILE, "w");
937+ fclose(f);
938+
939+ IniParser conf(INI_TEMP_FILE);
940+
941+ // Replace ini file with a directory
942+ ASSERT_EQ(0, remove(INI_TEMP_FILE));
943+ ASSERT_EQ(0, mkdir(INI_TEMP_FILE, 0700));
944+
945+ std::shared_ptr<void> rmdir_raii(nullptr, [](void*)
946+ {
947+ rmdir(INI_TEMP_FILE);
948+ });
949+
950+ // Sync (no exception as config is not dirty)
951+ EXPECT_NO_THROW(conf.sync());
952+
953+ // Dirty the config
954+ conf.set_boolean("g1", "k1", true);
955+
956+ // Sync (exception as target is a directory)
957+ EXPECT_THROW(conf.sync(), FileException);
958 }

Subscribers

People subscribed via source and target branches

to all changes: