fix: object_store: stop throwing errors when checking generation file

[amananas]

A new routine is added to File class, to check whether a file exists or not. It will allow file existence check, without trying to open the file and thus throw an error when the use case is valid.

This new routine is added to Generation wasUpdated routine: if the generation file does not exist, the routine will exit without error, instead of trying to open the file and report an error.

Fixes #753

Summary by CodeRabbit

• New Features

  • Added a file-existence check so callers can determine if a file exists without opening it.

• Bug Fixes

  • Update detection now treats missing files as needing updates, improving correctness of change detection.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Added a public static File::exists(path) that checks file existence using platform-specific calls (non-Windows: stat; Windows: _access). Generation::wasUpdated() now short-circuits by returning true when the generation file is missing, avoiding attempts to open it.

Changes

Cohort / File(s)	Summary
File existence API src/lib/object_store/File.h, src/lib/object_store/File.cpp	Added static bool File::exists(const std::string &path). Non-Windows implementation uses stat(path.c_str(), &buf) == 0; Windows implementation uses _access(path.c_str(), 0) == 0. Method is public and documented.
Generation update check src/lib/object_store/Generation.cpp	wasUpdated() now calls File::exists() and returns true immediately if the generation file is missing, preventing further open/read attempts and associated syslog noise.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Areas to review:
  • Correctness of platform-specific existence checks (stat vs _access) and include headers.
  • Ensure wasUpdated() early-return semantics (returning true when file is absent) match intended behavior for callers.
  • Any unit tests or call sites that assume the previous behavior when the file was missing.

Poem

🐇 I sniff the path, then give a nod,
No banging where no file has trod.
Generation sleeps, logs breathe free,
I hop away, content as can be. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately describes the main change: adding file existence checking to prevent errors when the generation file is missing.
Linked Issues check	✅ Passed	The code changes directly address the linked issue #753 by adding a File::exists() method and using it in Generation::wasUpdated() to avoid throwing errors for missing generation files.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing spurious error logs from missing generation files; no unrelated modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dda968a and 8f49d40.

📒 Files selected for processing (3)

• src/lib/object_store/File.cpp (1 hunks)
• src/lib/object_store/File.h (1 hunks)
• src/lib/object_store/Generation.cpp (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/object_store/File.cpp

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: bukka
Repo: softhsm/SoftHSMv2 PR: 815
File: src/lib/SoftHSM.cpp:6543-6546
Timestamp: 2025-09-11T16:54:00.370Z
Learning: SoftHSM PR #815 (pkcs11.h 3.1.0 update, CK_RSA_AES_KEY_WRAP_PARAMS fix): Maintainer bukka prefers to keep scope minimal (header sync and param rename only); broader OAEP parameter enforcement or backend plumbing is out of scope for this PR and should be handled in a follow-up issue.

🧬 Code graph analysis (2)

src/lib/object_store/File.h (2)

src/lib/object_store/File.cpp (2)

• exists (149-157)
• exists (149-149)

src/lib/object_store/test/FileTests.cpp (2)

• exists (355-377)
• exists (355-355)

src/lib/object_store/Generation.cpp (2)

src/lib/object_store/File.cpp (2)

• exists (149-157)
• exists (149-149)

src/lib/object_store/test/FileTests.cpp (2)

• exists (355-377)
• exists (355-355)

🔇 Additional comments (2)

src/lib/object_store/File.h (1)

50-53: LGTM! Well-documented existence check.

The static method declaration is appropriate for checking file existence without instantiation. The documentation clearly explains the use case of avoiding spurious errors when the file may legitimately be absent.

src/lib/object_store/Generation.cpp (1)

91-96: Effective fix for spurious error logs.

The early existence check correctly prevents the File constructor from attempting to open a missing generation file, which was causing the spurious error logs described in issue #753. The semantic behavior is preserved—returning true when the file doesn't exist matches the original logic at lines 106 and 132 where !genFile.isValid() also returns true.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/lib/object_store/File.h (1)

50-54: Consider passing path by const reference for efficiency.

The path parameter is passed by value, creating an unnecessary string copy.

Apply this diff to pass by const reference:

-	static bool exists(std::string path);
+	static bool exists(const std::string& path);

src/lib/object_store/File.cpp (1)

149-149: Parameter should be passed by const reference.

As noted in the File.h review, the path parameter creates an unnecessary copy. Apply the same change here to match the header declaration once updated.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 70e0d4c and dda968a.

📒 Files selected for processing (3)

• src/lib/object_store/File.cpp (1 hunks)
• src/lib/object_store/File.h (1 hunks)
• src/lib/object_store/Generation.cpp (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: bukka
Repo: softhsm/SoftHSMv2 PR: 815
File: src/lib/SoftHSM.cpp:6543-6546
Timestamp: 2025-09-11T16:54:00.370Z
Learning: SoftHSM PR #815 (pkcs11.h 3.1.0 update, CK_RSA_AES_KEY_WRAP_PARAMS fix): Maintainer bukka prefers to keep scope minimal (header sync and param rename only); broader OAEP parameter enforcement or backend plumbing is out of scope for this PR and should be handled in a follow-up issue.

🧬 Code graph analysis (3)

src/lib/object_store/File.cpp (1)

src/lib/object_store/test/FileTests.cpp (2)

• exists (355-377)
• exists (355-355)

src/lib/object_store/File.h (2)

src/lib/object_store/File.cpp (2)

• exists (149-165)
• exists (149-149)

src/lib/object_store/test/FileTests.cpp (2)

• exists (355-377)
• exists (355-355)

src/lib/object_store/Generation.cpp (2)

src/lib/object_store/File.cpp (2)

• exists (149-165)
• exists (149-149)

src/lib/object_store/test/FileTests.cpp (2)

• exists (355-377)
• exists (355-355)

🔇 Additional comments (1)

src/lib/object_store/Generation.cpp (1)

91-96: LGTM - Effectively prevents spurious error logs.

The early existence check successfully avoids attempting to open non-existent generation files, directly addressing issue #753. Returning true when the file doesn't exist is consistent with the existing behavior at lines 104-107 and 130-133, where invalid files also return true.