GH-46217: [C++][Parquet] Update the timestamp of parquet::encryption::TwoLevelCacheWithExpiration correctly (#46283)

### Rationale for this change

Fix the bug mentioned in #46217 (comment)

### What changes are included in this PR?

* Fix the logic of timestamp modification.
* Extend the test in two_level_cache_with_expiration_test.cc (CleanupPeriodOk).
* Replace the usage of std::make_unique's constructor with std::make_unique().

### Are these changes tested?

Yes. The original test didn't expose the problem because CheckCacheForExpiredTokens was only called once.

### Are there any user-facing changes?

No.
* GitHub Issue: #46217

Lead-authored-by: Eddie Chang <kalcifer7319@gmail.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Antoine Pitrou <pitrou@free.fr>

Author: kapoisu

cpp/src/parquet/encryption/two_level_cache_with_expiration.h

@@ -103,32 +103,24 @@ class TwoLevelCacheWithExpiration {
     if (external_cache_entry == cache_.end() ||
         external_cache_entry->second.IsExpired()) {
       cache_.insert({access_token, internal::ExpiringCacheMapEntry<V>(
-                                       std::shared_ptr<ConcurrentMap<std::string, V>>(
-                                           new ConcurrentMap<std::string, V>()),
+                                       std::make_shared<ConcurrentMap<std::string, V>>(),
                                        cache_entry_lifetime_seconds)});
     }
 
     return cache_[access_token].cached_item();
   }
 
-  void CheckCacheForExpiredTokens(double cache_cleanup_period_seconds) {
+  void CheckCacheForExpiredTokens(double cache_cleanup_period_seconds = 0.0) {
     auto lock = mutex_.Lock();
 
     const auto now = internal::CurrentTimePoint();
     if (now > (last_cache_cleanup_timestamp_ +
                std::chrono::duration<double>(cache_cleanup_period_seconds))) {
       RemoveExpiredEntriesNoMutex();
-      last_cache_cleanup_timestamp_ =
-          now + std::chrono::duration<double>(cache_cleanup_period_seconds);
+      last_cache_cleanup_timestamp_ = now;
     }
   }
 
-  void RemoveExpiredEntriesFromCache() {
-    auto lock = mutex_.Lock();
-
-    RemoveExpiredEntriesNoMutex();
-  }
-
   void Remove(const std::string& access_token) {
     auto lock = mutex_.Lock();
     cache_.erase(access_token);

cpp/src/parquet/encryption/two_level_cache_with_expiration_test.cc

@@ -77,7 +77,7 @@ TEST_F(TwoLevelCacheWithExpirationTest, RemoveExpiration) {
   // lifetime2 will not be expired
   SleepFor(0.3);
   // now clear expired items from the cache
-  cache_.RemoveExpiredEntriesFromCache();
+  cache_.CheckCacheForExpiredTokens();
 
   // lifetime1 (with 2 items) is expired and has been removed from the cache.
   // Now the cache create a new object which has no item.
@@ -114,6 +114,14 @@ TEST_F(TwoLevelCacheWithExpirationTest, CleanupPeriodOk) {
   // lifetime2 is not expired and still contains 2 items.
   auto lifetime2 = cache_.GetOrCreateInternalCache("lifetime2", 3);
   ASSERT_EQ(lifetime2->size(), 2);
+
+  // The further process is added to test whether the timestamp is set correctly.
+  // CheckCacheForExpiredTokens() should be called at least twice to verify the
+  // correctness.
+  SleepFor(0.3);
+  cache_.CheckCacheForExpiredTokens(0.2);
+  lifetime2 = cache_.GetOrCreateInternalCache("lifetime2", 0.5);
+  ASSERT_EQ(lifetime2->size(), 0);
 }
 
 TEST_F(TwoLevelCacheWithExpirationTest, RemoveByToken) {