Create CrashReportDatabase interface, a test, and a Mac implementation.

client/crash_report_database.h

+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_CLIENT_CRASH_REPORT_DATABASE_H_
+#define CRASHPAD_CLIENT_CRASH_REPORT_DATABASE_H_
+
+#include <time.h>
+
+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/files/file_path.h"
+#include "base/memory/scoped_ptr.h"
+#include "util/file/file_io.h"
+#include "util/misc/uuid.h"
+
+namespace crashpad {
+
+//! \brief An interface for managing a collection of crash report files and
+//!     metadata associated with the crash reports.
+//!
+//! All Report objects that are returned by this class are logically const.
+//! They are snapshots of the database at the time the query was run, and the
+//! data returned is liable to change after the query is executed.
+class CrashReportDatabase {
+ public:
+  //! \brief A crash report record.
+  //!
+  //! This represents the metadata for a crash report, as well as the location
+  //! of the report itself. A CrashReportDatabase maintains at least this
+  //! information.
+  struct Report {
+    Report();
+
+    //! A unique identifier by which this report will always be known to the
+    //! database.
+    UUID uuid;
+
+    //! The current location of the crash report on the client’s filesystem.
+    //! The location of a crash report may change over time, so the UUID should
+    //! be used as the canonical identifier.
+    base::FilePath file_path;
+
+    //! An identifier issued to this crash report by a collection server.
+    std::string id;
+
+    //! The time at which the report was generated.
+    time_t creation_time;
+
+    //! Whether this crash report was successfully uploaded to a collection
+    //! server.
+    bool uploaded;
+
+    //! The last timestamp at which an attempt was made to submit this crash
+    //! report to a collection server. If this is zero, then the report has
+    //! never been uploaded. If #uploaded is true, then this timestamp is the
+    //! time at which the report was uploaded, and no other attempts to upload
+    //! this report will be made.
+    time_t last_upload_attempt_time;
+
+    //! The number of times an attempt was made to submit this report to
+    //! a collection server. If this is more than zero, then
+    //! #last_upload_attempt_time will be set to the timestamp of the most
+    //! recent attempt.
+    int upload_attempts;
+  };
+
+  //! \brief The result code for operations performed on a database.
+  enum OperationStatus {
+    //! \brief No error occurred.
+    kNoError = 0,
+
+    //! \brief The report that was requested could not be located.
+    kReportNotFound,
+
+    //! \brief An error occured while performing a file operation on a crash
+    //!     report.
+    //!
+    //! A database is responsible for managing both the metadata about a report
+    //! and the actual crash report itself. This error is returned when an
+    //! error occurred when managing the report file. Additional information
+    //! will be logged.
+    kFileSystemError,
+
+    //! \brief An error occured while recording metadata for a crash report.
+    //!
+    //! A database is responsible for managing both the metadata about a report
+    //! and the actual crash report itself. This error is returned when an
+    //! error occurred when managing the metadata about a crash report.
+    //! Additional information will be logged.
+    kDatabaseError,
+
+    //! \brief The operation could not be completed because a concurrent

[Mark Mentovai, 2015/01/21 18:01:35]

A concurrent what?

[Robert Sesek, 2015/01/26 20:16:14]

Done.

+    //!     affecting the report is occurring.
+    kBusyError,
+  };
+
+  virtual ~CrashReportDatabase() {}
+
+  //! \brief Initializes a database of crash reports.
+  //!
+  //! \param[in] path A path to a writable directory, where the database can
+  //!     be created or opened.
+  //!
+  //! \return A database object on success, `nullptr` on failure with an error
+  //!     logged.
+  static scoped_ptr<CrashReportDatabase> Initialize(const base::FilePath& path);
+
+  //! \brief Creates a record of a new crash report.
+  //!
+  //! Callers can then write the crash report using the file handle provided.
+  //! The caller does not own this handle, and it must be explicitly closed with
+  //! FinishedWritingCrashReport().
+  //!
+  //! \param[out] handle A file handle to which the crash report data should be
+  //!     written. Only valid if this returns #kNoError. The caller should not

[Mark Mentovai, 2015/01/21 18:01:35]

should→must.

[Robert Sesek, 2015/01/26 20:16:14]

Done.

+  //!     close this handle.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus PrepareNewCrashReport(FileHandle* handle) = 0;
+
+  //! \brief Informs the database that a crash report has been written.
+  //!
+  //! After calling this method, the database is permitted to move and rename
+  //! the file at Record::file_path.

[Mark Mentovai, 2015/01/21 18:01:35]

I don’t know Record::, only Report::. Lines 177 and 197 too.

[Robert Sesek, 2015/01/26 20:16:14]

Done.

+  //!
+  //! \param[in] handle A handle obtained with PrepareNewCrashReport(). The
+  //!     handle will be invalidated as part of this call.
+  //! \param[out] uuid The UUID of this crash report.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus FinishedWritingCrashReport(FileHandle handle,
+                                                     UUID* uuid) = 0;
+
+  //! \brief Returns the crash report record for the unique identifier.
+  //!
+  //! \param[in] uuid The crash report record unique identifier.
+  //! \param[out] report A crash report record. Only valid if this returns
+  //!     #kNoError.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus LookUpCrashReport(const UUID& uuid,
+                                            Report* report) = 0;
+
+  //! \brief Returns a list of crash report records that have not been uploaded.
+  //!
+  //! \param[out] reports A list of crash report record objects. This must be
+  //!     empty on entry. Only valid if this returns #kNoError.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus GetNotUploadedReports(

[Mark Mentovai, 2015/01/21 18:01:35]

The only real major problem I see with this now is that there’s really only a
distinction between “not uploaded” and “uploaded.” In practice, I think that
these terms will be confusing:

We shouldn’t upload reports for with uploads turned off. If they subsequently
turn uploads on, we shouldn’t upload reports that we collected while uploads
were turned off. The reports should graduate to a “done” state somewhat rapidly.
You’ve got a comment in the .cc file indicating that this is how it works, but I
don’t think we should keep everything confused by perpetuating the
“uploaded” naming.

When uploads are on, we shouldn’t keep attempting to upload a report
indefinitely. After a certain number of attempts, we should give up. These, too,
should graduate to a “done” state.

We’ve got three possible top-level states:

 - report is being written
 - report has been written
 - report is “done”

In the done state, a report can have a server ID attribute if it was uploaded
successfully, some error attribute if it wasn’t ever able to be uploaded after a
number of attempts, or no attribute at all (or a different attribute) if it was
set aside without an upload attempt due to user preference.

These three states map pretty well to the directories you’ve already got, which
should maybe be named “tmp”, (“pending” or “written”), and “done”. More
importantly, the naming needs to bubble through to the method names like these.
GetNotUploadedReports(), this method, is almost OK, except it makes it sound
like the report is definitely going to be uploaded, which may not be the case if
the user preference is to not do so. GetReportForUploading() makes it even
worse. And if a report did graduate from pending to done without upload,
GetUploadedReports() doesn’t really do what it says.

Tying it all up: I expect the client of your class will be responsible for
“graduating” things between the pending and done states. Your class doesn’t need
to be involved in the decision-making process of “are uploads desired?” or
“should I give up trying to upload this report?” The client of your class will
be in charge of these decisions. All that CrashReportDatabase needs to know is
that the client might want to move something from “pending” to “done” without a
successful upload and crash ID. This has implications for [the reanmed]
RecordUploadAttempt().

[Robert Sesek, 2015/01/26 20:16:14]

Done. I've added a new SkipReportUpload() method and added some commentary about
report lifecycle (new -> pending -> completed).

+      std::vector<const Report>* reports) = 0;
+
+  //! \brief Returns a list of crash report records that have been uploaded.
+  //!
+  //! \param[out] reports A list of crash report record objects. This must be
+  //!     empty on entry. Only valid if this returns #kNoError.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus GetUploadedReports(
+      std::vector<const Report>* reports) = 0;
+
+  //! \brief Obtains a report object for uploading to a collection server.
+  //!
+  //! The file at Record::file_path should be uploaded by the caller, and then
+  //! the returned Record object should be disposed of via a call to

[Mark Mentovai, 2015/01/21 18:01:35]

Strengthen this to “must.”

[Robert Sesek, 2015/01/26 20:16:14]

Done.

+  //! RecordUploadAttempt().
+  //!
+  //! A subsequent call to this method with the same \a uuid is illegal if

[Mark Mentovai, 2015/01/21 18:01:35]

And since it’s a “must”, the “if x() has not been called first” can become
“until x() has been called”.

[Robert Sesek, 2015/01/26 20:16:14]

Done.

+  //! RecordUploadAttempt() has not been called first.
+  //!
+  //! \param[in] uuid The unique identifier for the crash report record.
+  //! \param[out] report A crash report record for the report to be uploaded.
+  //!     The caller does not own this object. Only valid if this returns
+  //!     #kNoError.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus GetReportForUploading(const UUID& uuid,
+                                                const Report** report) = 0;
+
+  //! \brief Adjusts a crash report record’s metadata to account for an upload
+  //!     attempt.
+  //!
+  //! After calling this method, the database is permitted to move and rename
+  //! the file at Record::file_path.
+  //!
+  //! \param[in] report The report object obtained from
+  //!     GetReportForUploading(). This object is invalidated after this call.
+  //! \param[in] successful Whether the upload attempt was successful.
+  //! \param[in] id The identifier assigned to this crash report by the
+  //!     collection server. Must be empty if \a successful is `false`; may be
+  //!     empty if it is `true`.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus RecordUploadAttempt(const Report* report,
+                                              bool successful,
+                                              const std::string& id) = 0;
+
+ protected:
+  CrashReportDatabase() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CrashReportDatabase);
+};
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_CLIENT_CRASH_REPORT_DATABASE_H_