commit a2d8239473ea9c441e157481c83195c5dcffe5bc
author Patrick Venture <venture@google.com> Mon Jun 03 10:55:17 2019 -0700
committer Patrick Venture <venture@google.com> Mon Jun 03 11:04:31 2019 -0700
tree 44c174e5c7cff5684ef8b3d9c0a68041ef01b666
parent ab1e96206b6adb29431d13a91628dd1fc2e057e9

bmc: firmware updateCompleted: stat(session)

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I0ae2eac8aaf736be3481c678e5c274d03f705e43

firmware_handler.cpp

diff --git a/firmware_handler.cpp b/firmware_handler.cpp
index 13e5d72..9cb78fb 100644
--- a/firmware_handler.cpp
+++ b/firmware_handler.cpp
@@ -175,14 +175,55 @@
     return true;
 }
 
+VerifyCheckResponses FirmwareBlobHandler::getVerifyStatus()
+{
+    VerifyCheckResponses value = VerifyCheckResponses::other;
+
+    switch (state)
+    {
+        case UpdateState::verificationPending:
+            value = VerifyCheckResponses::other;
+            break;
+        case UpdateState::verificationStarted:
+            value = verification->status();
+            lastVerificationResponse = value;
+            break;
+        case UpdateState::verificationCompleted:
+            value = lastVerificationResponse;
+            break;
+        default:
+            break;
+    }
+
+    return value;
+}
+
+UpdateStatus FirmwareBlobHandler::getUpdateStatus()
+{
+    UpdateStatus value = UpdateStatus::unknown;
+
+    switch (state)
+    {
+        case UpdateState::updatePending:
+            value = UpdateStatus::unknown;
+            break;
+        case UpdateState::updateStarted:
+            value = update->status();
+            lastUpdateStatus = value;
+            break;
+        case UpdateState::updateCompleted:
+            value = lastUpdateStatus;
+            break;
+        default:
+            break;
+    }
+
+    return value;
+}
+
 /*
  * Return stat information on an open session.  It therefore must be an active
  * handle to either the active image or active hash.
- *
- * The stat() and sessionstat() commands will return the same information in
- * many cases, therefore the logic will be combined.
- *
- * TODO: combine the logic for stat and sessionstat().
  */
 bool FirmwareBlobHandler::stat(uint16_t session, struct blobs::BlobMeta* meta)
 {
@@ -202,23 +243,7 @@
 
     if (item->second->activePath == verifyBlobId)
     {
-        VerifyCheckResponses value = VerifyCheckResponses::other;
-
-        switch (state)
-        {
-            case UpdateState::verificationPending:
-                value = VerifyCheckResponses::other;
-                break;
-            case UpdateState::verificationStarted:
-                value = verification->status();
-                lastVerificationResponse = value;
-                break;
-            case UpdateState::verificationCompleted:
-                value = lastVerificationResponse;
-                break;
-            default:
-                break;
-        }
+        VerifyCheckResponses value = getVerifyStatus();
 
         meta->metadata.push_back(static_cast<std::uint8_t>(value));
 
@@ -243,19 +268,7 @@
     }
     else if (item->second->activePath == updateBlobId)
     {
-        UpdateStatus value = UpdateStatus::unknown;
-
-        switch (state)
-        {
-            case UpdateState::updatePending:
-                value = UpdateStatus::unknown;
-                break;
-            case UpdateState::updateStarted:
-                value = update->status();
-                break;
-            default:
-                break;
-        }
+        UpdateStatus value = getUpdateStatus();
 
         meta->metadata.push_back(static_cast<std::uint8_t>(value));
 

firmware_handler.hpp

diff --git a/firmware_handler.hpp b/firmware_handler.hpp
index 67fb82b..e08cb66 100644
--- a/firmware_handler.hpp
+++ b/firmware_handler.hpp
@@ -195,6 +195,9 @@
                       blobIDs.end());
     }
 
+    VerifyCheckResponses getVerifyStatus();
+    UpdateStatus getUpdateStatus();
+
     /** List of handlers by type. */
     std::vector<HandlerPack> handlers;
 
@@ -233,6 +236,8 @@
     bool fileOpen = false;
 
     VerifyCheckResponses lastVerificationResponse = VerifyCheckResponses::other;
+
+    UpdateStatus lastUpdateStatus = UpdateStatus::unknown;
 };
 
 } // namespace ipmi_flash

test/firmware_state_updatecompleted_unittest.cpp

diff --git a/test/firmware_state_updatecompleted_unittest.cpp b/test/firmware_state_updatecompleted_unittest.cpp
index 37f228c..95e0a32 100644
--- a/test/firmware_state_updatecompleted_unittest.cpp
+++ b/test/firmware_state_updatecompleted_unittest.cpp
@@ -56,12 +56,48 @@
 }
 
 /*
+ * stat(session)
+ */
+TEST_F(FirmwareHandlerUpdateCompletedTest,
+       CallingStatSessionAfterCompletedSuccessReturnsStateWithoutRechecking)
+{
+    getToUpdateCompleted(UpdateStatus::success);
+    EXPECT_CALL(*updateMockPtr, status()).Times(0);
+
+    blobs::BlobMeta meta, expectedMeta = {};
+    expectedMeta.size = 0;
+    expectedMeta.blobState = flags | blobs::StateFlags::committed;
+    expectedMeta.metadata.push_back(
+        static_cast<std::uint8_t>(UpdateStatus::success));
+
+    EXPECT_TRUE(handler->stat(session, &meta));
+    EXPECT_EQ(expectedMeta, meta);
+    expectedState(FirmwareBlobHandler::UpdateState::updateCompleted);
+}
+
+TEST_F(FirmwareHandlerUpdateCompletedTest,
+       CallingStatSessionAfterCompletedFailureReturnsStateWithoutRechecking)
+{
+    getToUpdateCompleted(UpdateStatus::failed);
+    EXPECT_CALL(*updateMockPtr, status()).Times(0);
+
+    blobs::BlobMeta meta, expectedMeta = {};
+    expectedMeta.size = 0;
+    expectedMeta.blobState = flags | blobs::StateFlags::commit_error;
+    expectedMeta.metadata.push_back(
+        static_cast<std::uint8_t>(UpdateStatus::failed));
+
+    EXPECT_TRUE(handler->stat(session, &meta));
+    EXPECT_EQ(expectedMeta, meta);
+    expectedState(FirmwareBlobHandler::UpdateState::updateCompleted);
+}
+
+/*
  * There are the following calls (parameters may vary):
  * canHandleBlob(blob)
  * getBlobIds
  * deleteBlob(blob)
  * stat(blob)
- * stat(session)
  * close(session)
  * writemeta(session)
  * write(session)