Merge branch 'bugfix/nvs_lookup_failure' into 'master'

nvs: fix Page::findItem and Storage::findItem regression

When read caching was added, Page::findItem started modifying itemIndex reference argument even if item wasn't found.
Incidentally, Storage::findItem reused itemIndex when starting search at next page.
So,
- if the first page had a cached index (findItem was called for that page), and it pointed to a non-zero index,
- first page has a few empty items at the end (but is marked full),
- next search looked up the item on the second page,
- index of the item on the second page was less than the cached index on the first page,

then the search would fail because cached starting index was reused.

This change fixes both sides of the problem:
- Page::findItem shouldn't modify itemIndex argument if item is not found
- Storage::findItem should not reuse itemIndex between pages

Two tests have been added.

ref. TW8406

See merge request !171

components/nvs_flash/src/nvs_page.cpp

@@ -656,19 +656,20 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si
     if (mState == PageState::CORRUPT || mState == PageState::INVALID || mState == PageState::UNINITIALIZED) {
         return ESP_ERR_NVS_NOT_FOUND;
     }
-
-    if (itemIndex >= ENTRY_COUNT) {
+    
+    size_t findBeginIndex = itemIndex;
+    if (findBeginIndex >= ENTRY_COUNT) {
         return ESP_ERR_NVS_NOT_FOUND;
     }
 
     CachedFindInfo findInfo(nsIndex, datatype, key);
     if (mFindInfo == findInfo) {
-        itemIndex = mFindInfo.itemIndex();
+        findBeginIndex = mFindInfo.itemIndex();
     }
 
     size_t start = mFirstUsedEntry;
-    if (itemIndex > mFirstUsedEntry && itemIndex < ENTRY_COUNT) {
-        start = itemIndex;
+    if (findBeginIndex > mFirstUsedEntry && findBeginIndex < ENTRY_COUNT) {
+        start = findBeginIndex;
     }
 
     size_t end = mNextFreeEntry;

components/nvs_flash/src/nvs_storage.cpp

@@ -71,8 +71,8 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount)
 
 esp_err_t Storage::findItem(uint8_t nsIndex, ItemType datatype, const char* key, Page* &page, Item& item)
 {
-    size_t itemIndex = 0;
     for (auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) {
+        size_t itemIndex = 0;
         auto err = it->findItem(nsIndex, datatype, key, itemIndex, item);
         if (err == ESP_OK) {
             page = it;

components/nvs_flash/test/test_nvs.cpp

@@ -300,6 +300,27 @@ TEST_CASE("storage doesn't add duplicates within multiple pages", "[nvs]")
     CHECK(page.findItem(1, itemTypeOf<int>(), "bar") == ESP_OK);
 }
 
+TEST_CASE("storage can find items on second page if first is not fully written and has cached search data", "[nvs]")
+{
+    SpiFlashEmulator emu(3);
+    Storage storage;
+    CHECK(storage.init(0, 3) == ESP_OK);
+    int bar = 0;
+    uint8_t bigdata[100 * 32] = {0};
+    // write one big chunk of data
+    ESP_ERROR_CHECK(storage.writeItem(0, ItemType::BLOB, "first", bigdata, sizeof(bigdata)));
+    
+    // write second one; it will not fit into the first page
+    ESP_ERROR_CHECK(storage.writeItem(0, ItemType::BLOB, "second", bigdata, sizeof(bigdata)));
+    
+    size_t size;
+    ESP_ERROR_CHECK(storage.getItemDataSize(0, ItemType::BLOB, "first", size));
+    CHECK(size == sizeof(bigdata));
+    ESP_ERROR_CHECK(storage.getItemDataSize(0, ItemType::BLOB, "second", size));
+    CHECK(size == sizeof(bigdata));
+}
+
+
 TEST_CASE("can write and read variable length data lots of times", "[nvs]")
 {
     SpiFlashEmulator emu(8);
@@ -1055,6 +1076,39 @@ TEST_CASE("crc error in variable length item is handled", "[nvs]")
 }
 
 
+TEST_CASE("read/write failure (TW8406)", "[nvs]")
+{
+    SpiFlashEmulator emu(3);
+    nvs_flash_init_custom(0, 3);
+    for (int attempts = 0; attempts < 3; ++attempts) {
+        int i = 0;
+        nvs_handle light_handle = 0;
+        char key[15] = {0};
+        char data[76] = {12, 13, 14, 15, 16};
+        uint8_t number = 20;
+        size_t data_len = sizeof(data);
+        
+        ESP_ERROR_CHECK(nvs_open("LIGHT", NVS_READWRITE, &light_handle));
+        ESP_ERROR_CHECK(nvs_set_u8(light_handle, "RecordNum", number));
+        for (i = 0; i < number; ++i) {
+            sprintf(key, "light%d", i);
+            ESP_ERROR_CHECK(nvs_set_blob(light_handle, key, data, sizeof(data)));
+        }
+        nvs_commit(light_handle);
+        
+        uint8_t get_number = 0;
+        ESP_ERROR_CHECK(nvs_get_u8(light_handle, "RecordNum", &get_number));
+        REQUIRE(number == get_number);
+        for (i = 0; i < number; ++i) {
+            char data[76] = {0};
+            sprintf(key, "light%d", i);
+            ESP_ERROR_CHECK(nvs_get_blob(light_handle, key, data, &data_len));
+        }
+        nvs_close(light_handle);
+    }
+}
+
+
 TEST_CASE("dump all performance data", "[nvs]")
 {
     std::cout << "====================" << std::endl << "Dumping benchmarks" << std::endl;