Change ActiveAuthorizations to GetAuthorizations and add GetPurchasesByAuthorizationID

Also clean up some clang-tidy warnings.

Author: adam-p

datastore.cpp

@@ -84,14 +84,14 @@ Error Datastore::FileLoad() {
         // File probably doesn't exist. Check that we can write here.
         return WrapError(FileStore(), "f.fail and FileStore failed");
     } else if (!f.good()) {
-        return MakeCriticalError(utils::Stringer("not f.good; errno=", errno).c_str());
+        return MakeCriticalError(utils::Stringer("not f.good; errno=", errno));
     }
 
     try {
         f >> json_;
     }
     catch (json::exception& e) {
-        return MakeCriticalError(utils::Stringer("json load failed: ", e.what(), "; id:", e.id).c_str());
+        return MakeCriticalError(utils::Stringer("json load failed: ", e.what(), "; id:", e.id));
     }
 
     return nullerr;
@@ -107,14 +107,14 @@ Error Datastore::FileStore() {
     ofstream f;
     f.open(file_path_, ios::trunc | ios::binary);
     if (!f.is_open()) {
-        return MakeCriticalError(utils::Stringer("not f.is_open; errno=", errno).c_str());
+        return MakeCriticalError(utils::Stringer("not f.is_open; errno=", errno));
     }
 
     try {
         f << json_;
     }
     catch (json::exception& e) {
-        return MakeCriticalError(utils::Stringer("json dump failed: ", e.what(), "; id:", e.id).c_str());
+        return MakeCriticalError(utils::Stringer("json dump failed: ", e.what(), "; id:", e.id));
     }
 
     return nullerr;

datastore_test.cpp

@@ -31,7 +31,7 @@ using json = nlohmann::json;
 class TestDatastore : public ::testing::Test, public TempDir
 {
   public:
-    TestDatastore() {}
+    TestDatastore() = default;
 };
 
 TEST_F(TestDatastore, InitSimple)

datetime.cpp

@@ -34,7 +34,7 @@ using namespace std;
 namespace psicash {
 namespace datetime {
 
-const TimePoint kTimePointZero = TimePoint();
+static const TimePoint kTimePointZero = TimePoint();
 
 static constexpr const char* ISO8601_FORMAT_STRING = "%FT%TZ";
 
@@ -83,11 +83,11 @@ DateTime::DateTime(const TimePoint& src)
 }
 
 DateTime DateTime::Zero() {
-    return kTimePointZero;
+    return DateTime(kTimePointZero);
 }
 
 DateTime DateTime::Now() {
-    return NormalizeTimePoint(Clock::now());
+    return DateTime(NormalizeTimePoint(Clock::now()));
 }
 
 bool DateTime::IsZero() const {
@@ -129,11 +129,11 @@ Duration DateTime::Diff(const DateTime& other) const {
 }
 
 DateTime DateTime::Add(const Duration& d) const {
-    return NormalizeTimePoint(time_point_ + d);
+    return DateTime(NormalizeTimePoint(time_point_ + d));
 }
 
 DateTime DateTime::Sub(const Duration& d) const {
-    return NormalizeTimePoint(time_point_ - d);
+    return DateTime(NormalizeTimePoint(time_point_ - d));
 }
 
 int64_t DateTime::MillisSinceEpoch() const {
@@ -164,7 +164,7 @@ int64_t DurationToInt64(const Duration& d) {
     return d.count();
 }
 
-Duration DurationFromInt64(const int64_t d) {
+Duration DurationFromInt64(int64_t d) {
     return Duration(d);
 }
 

datetime.hpp

@@ -35,7 +35,7 @@ class DateTime {
     // By default, initializes to the "zero" value.
     DateTime();
     DateTime(const DateTime& src);
-    DateTime(const TimePoint& src);
+    explicit DateTime(const TimePoint& src);
     DateTime& operator=(const DateTime&) = default;
 
     static DateTime Zero();
@@ -74,7 +74,7 @@ class DateTime {
 
 // These are intended to help de/serialization of duration values.
 int64_t DurationToInt64(const Duration& d);
-Duration DurationFromInt64(const int64_t d);
+Duration DurationFromInt64(int64_t d);
 
 } // namespace datetime
 } // namespace psicash

error.cpp

@@ -30,10 +30,6 @@ Error::Error()
         : is_error_(false), critical_(false) {
 }
 
-Error::Error(const Error& src)
-        : is_error_(src.is_error_), critical_(src.critical_), stack_(src.stack_) {
-}
-
 Error::Error(bool critical, const std::string& message, const std::string& filename,
              const std::string& function, int line)
         : is_error_(true), critical_(critical) {

error.hpp

@@ -40,7 +40,7 @@ namespace error {
 class Error {
 public:
     Error();
-    Error(const Error& src);
+    Error(const Error& src) = default;
     Error(bool critical, const std::string& message,
           const std::string& filename, const std::string& function, int line);
     Error& operator=(const Error&) = default;
@@ -90,8 +90,8 @@ const Error nullerr;
 // Should be prefixed with namespaces: psicash::error::
 #define MakeNoncriticalError(message)   Error(false, (message), __FILE__, __PRETTY_FUNCTION__, __LINE__)
 #define MakeCriticalError(message)      Error(true, (message), __FILE__, __PRETTY_FUNCTION__, __LINE__)
-#define WrapError(err, message)         (err.Wrap((message), __FILE__, __PRETTY_FUNCTION__, __LINE__))
-#define PassError(err)                  (err.Wrap(__FILE__, __PRETTY_FUNCTION__, __LINE__))
+#define WrapError(err, message)         ((err).Wrap((message), __FILE__, __PRETTY_FUNCTION__, __LINE__))
+#define PassError(err)                  ((err).Wrap(__FILE__, __PRETTY_FUNCTION__, __LINE__))
 
 
 /*

psicash.cpp

@@ -22,6 +22,7 @@
 #include <sstream>
 #include <chrono>
 #include <thread>
+#include <algorithm>
 #include "psicash.hpp"
 #include "userdata.hpp"
 #include "datetime.hpp"
@@ -177,16 +178,29 @@ Purchases PsiCash::ActivePurchases() const {
     return res;
 }
 
-Authorizations PsiCash::ActiveAuthorizations() const {
+Authorizations PsiCash::GetAuthorizations(bool activeOnly/*=false*/) const {
     Authorizations res;
     for (const auto& p : user_data_->GetPurchases()) {
-        if (!IsExpired(p) && p.authorization) {
+        if (p.authorization && (!activeOnly || !IsExpired(p))) {
             res.push_back(*p.authorization);
         }
     }
     return res;
 }
 
+Purchases PsiCash::GetPurchasesByAuthorizationID(std::vector<std::string> authorization_ids) const {
+    auto purchases = user_data_->GetPurchases();
+
+    auto new_end = std::remove_if(purchases.begin(), purchases.end(), [&authorization_ids](const Purchase& p){
+        return !p.authorization
+            || std::find(authorization_ids.begin(), authorization_ids.end(), p.authorization->id) == authorization_ids.end();
+    });
+
+    purchases.erase(new_end, purchases.end());
+
+    return purchases;
+}
+
 optional<Purchase> PsiCash::NextExpiringPurchase() const {
     optional<Purchase> next;
     for (const auto& p : user_data_->GetPurchases()) {

psicash.hpp

@@ -203,8 +203,12 @@ class PsiCash {
     /// Returns the set of active purchases that are not expired, if any.
     Purchases ActivePurchases() const;
 
-    /// Returns the set of active (non-expired) purchase authorizations, if any.
-    Authorizations ActiveAuthorizations() const;
+    /// Returns all purchase authorizations. If activeOnly is true, only authorizations
+    /// for non-expired purchases will be returned.
+    Authorizations GetAuthorizations(bool activeOnly=false) const;
+
+    /// Returns all purchases that match the given set of Authorization IDs.
+    Purchases GetPurchasesByAuthorizationID(std::vector<std::string> authorization_ids) const;
 
     /// Get the next expiring purchase (with local_time_expiry populated).
     /// The returned optional will false if there is no outstanding expiring purchase (or

psicash_test.cpp

@@ -388,7 +388,7 @@ TEST_F(TestPsiCash, DecodeAuthorization) {
     ASSERT_TRUE(auth_res_fail.error().Critical());
 }
 
-TEST_F(TestPsiCash, ActiveAuthorizations) {
+TEST_F(TestPsiCash, GetAuthorizations) {
     PsiCashTester pc;
     auto err = pc.Init(user_agent_, GetTempDir().c_str(), nullptr, true);
     ASSERT_FALSE(err);
@@ -399,7 +399,10 @@ TEST_F(TestPsiCash, ActiveAuthorizations) {
     purchases = pc.ActivePurchases();
     ASSERT_EQ(purchases.size(), 0);
 
-    auto v = pc.ActiveAuthorizations();
+    auto v = pc.GetAuthorizations(false);
+    ASSERT_EQ(v.size(), 0);
+
+    v = pc.GetAuthorizations(true);
     ASSERT_EQ(v.size(), 0);
 
     auto before_now = datetime::DateTime::Now().Sub(datetime::Duration(54321));
@@ -421,9 +424,51 @@ TEST_F(TestPsiCash, ActiveAuthorizations) {
     ASSERT_FALSE(err);
     ASSERT_EQ(pc.GetPurchases().size(), purchases.size());
 
-    v = pc.ActiveAuthorizations();
+    v = pc.GetAuthorizations(false);
+    ASSERT_EQ(v.size(), 2);
+    ASSERT_THAT(v, AnyOf(Contains(*auth_res1), Contains(*auth_res2)));
+
+    v = pc.GetAuthorizations(true);
     ASSERT_EQ(v.size(), 1);
-    ASSERT_EQ(v[0].encoded, encoded2);
+    ASSERT_THAT(v, Contains(*auth_res2));
+}
+
+TEST_F(TestPsiCash, GetPurchasesByAuthorizationID) {
+    PsiCashTester pc;
+    auto err = pc.Init(user_agent_, GetTempDir().c_str(), nullptr, true);
+    ASSERT_FALSE(err);
+
+    auto purchases = pc.GetPurchases();
+    ASSERT_EQ(purchases.size(), 0);
+
+    // Empty set of auth IDs
+    purchases = pc.GetPurchasesByAuthorizationID({});
+    ASSERT_EQ(purchases.size(), 0);
+
+    auto before_now = datetime::DateTime::Now().Sub(datetime::Duration(54321));
+    auto after_now = datetime::DateTime::Now().Add(datetime::Duration(54321));
+
+    const auto encoded1 = "eyJBdXRob3JpemF0aW9uIjp7IklEIjoiMFYzRXhUdmlBdFNxTGZOd2FpQXlHNHpaRUJJOGpIYnp5bFdNeU5FZ1JEZz0iLCJBY2Nlc3NUeXBlIjoic3BlZWQtYm9vc3QtdGVzdCIsIkV4cGlyZXMiOiIyMDE5LTAxLTE0VDE3OjIyOjIzLjE2ODc2NDEyOVoifSwiU2lnbmluZ0tleUlEIjoiUUNZTzV2clIvZGhjRDZ6M2FMQlVNeWRuZlJyZFNRL1RWYW1IUFhYeTd0TT0iLCJTaWduYXR1cmUiOiJQL2NrenloVUJoSk5RQ24zMnluM1VTdGpLencxU04xNW9MclVhTU9XaW9scXBOTTBzNVFSNURHVEVDT1FzQk13ODdQdTc1TGE1OGtJTHRIcW1BVzhDQT09In0=";
+    const auto encoded2 = "eyJBdXRob3JpemF0aW9uIjp7IklEIjoibFRSWnBXK1d3TFJqYkpzOGxBUFVaQS8zWnhmcGdwNDFQY0dkdlI5a0RVST0iLCJBY2Nlc3NUeXBlIjoic3BlZWQtYm9vc3QtdGVzdCIsIkV4cGlyZXMiOiIyMDE5LTAxLTE0VDIxOjQ2OjMwLjcxNzI2NTkyNFoifSwiU2lnbmluZ0tleUlEIjoiUUNZTzV2clIvZGhjRDZ6M2FMQlVNeWRuZlJyZFNRL1RWYW1IUFhYeTd0TT0iLCJTaWduYXR1cmUiOiJtV1Z5Tm9ZU0pFRDNXU3I3bG1OeEtReEZza1M5ZWlXWG1lcDVvVWZBSHkwVmYrSjZaQW9WajZrN3ZVTDNrakIreHZQSTZyaVhQc3FzWENRNkx0eFdBQT09In0=";
+
+    auto auth_res1 = psicash::DecodeAuthorization(encoded1);
+    auto auth_res2 = psicash::DecodeAuthorization(encoded2);
+    ASSERT_TRUE(auth_res1);
+    ASSERT_TRUE(auth_res2);
+
+    purchases = {{"future_no_auth", "tc1", "d1", after_now, nonstd::nullopt, nonstd::nullopt},
+                 {"past_auth", "tc2", "d2", before_now, nonstd::nullopt, *auth_res1},
+                 {"future_auth", "tc3", "d3", after_now, nonstd::nullopt, *auth_res2}};
+
+    err = pc.user_data().SetPurchases(purchases);
+    ASSERT_FALSE(err);
+    ASSERT_EQ(pc.GetPurchases().size(), purchases.size());
+
+    // One that matches and some that don't
+    vector<string> authIDs = {"badid", auth_res2->id, "anotherbadid"};
+    purchases = pc.GetPurchasesByAuthorizationID(authIDs);
+    ASSERT_THAT(purchases, SizeIs(1));
+    ASSERT_EQ(purchases[0].authorization->id, auth_res2->id);
 }
 
 TEST_F(TestPsiCash, NextExpiringPurchase) {