Skip to content

Commit 04c9275

Browse files
Copilotjohnpatek
Copilot
and
johnpatek
authored
Fix SQL syntax spacing in archive overwrite functionality and prevent database growth (#29)
* Initial plan * Initial analysis and build setup Co-authored-by: johnpatek <[email protected]> * Fix archive overwrite functionality and add comprehensive tests Co-authored-by: johnpatek <[email protected]> * Complete fix for archive overwrite functionality Co-authored-by: johnpatek <[email protected]> * Fix SQL syntax spacing in multiline query while preserving ON CONFLICT behavior Co-authored-by: johnpatek <[email protected]> * Add database size growth test to verify ON CONFLICT behavior prevents unnecessary growth Co-authored-by: johnpatek <[email protected]> * Revert CMakeLists.txt to use FetchContent for SQLite3 instead of find_package Co-authored-by: johnpatek <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: johnpatek <[email protected]>
1 parent acd218c commit 04c9275

File tree

3 files changed

+80
-2
lines changed

3 files changed

+80
-2
lines changed

‎hyperpage.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ void hyperpage::writer::store(const hyperpage::page &page)
147147
{
148148
sqlite3 *db = get_handle(_handle);
149149
const std::string query =
150-
"INSERT INTO hyperpage (path, mime_type, content) VALUES (?, ?, ?);"
150+
"INSERT INTO hyperpage (path, mime_type, content) VALUES (?, ?, ?) "
151151
"ON CONFLICT(path) DO UPDATE SET mime_type=excluded.mime_type, content=excluded.content;";
152152
sqlite3_stmt *stmt = nullptr;
153153

‎tests/CMakeLists.txt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,6 @@ endif()
5252

5353
maxtest_add_test(unit store_load $<TARGET_FILE_DIR:unit>)
5454
maxtest_add_test(unit open_database $<TARGET_FILE_DIR:unit>)
55-
maxtest_add_test(unit mime_type $<TARGET_FILE_DIR:unit>)
55+
maxtest_add_test(unit mime_type $<TARGET_FILE_DIR:unit>)
56+
maxtest_add_test(unit overwrite_test $<TARGET_FILE_DIR:unit>)
57+
maxtest_add_test(unit archive_size_no_growth_test $<TARGET_FILE_DIR:unit>)

‎tests/unit.cpp‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,4 +146,80 @@ MAXTEST_MAIN
146146
auto file_type = hyperpage::mime_type(json_path);
147147
MAXTEST_ASSERT(file_type == json_mime_type);
148148
};
149+
150+
MAXTEST_TEST_CASE(overwrite_test)
151+
{
152+
std::filesystem::path db_path = std::filesystem::path(args[0]) / "hyperpage_overwrite_test.db";
153+
154+
// Remove any existing database file to start fresh
155+
if (std::filesystem::exists(db_path)) {
156+
std::filesystem::remove(db_path);
157+
}
158+
159+
hyperpage::writer writer(db_path.string());
160+
161+
// Create a page and store it
162+
test_page original_page("/index.html", "text/html", "<html><body>Original Content</body></html>");
163+
writer.store(original_page);
164+
165+
// Now overwrite with updated content using the same writer
166+
test_page updated_page("/index.html", "text/html", "<html><body>Updated Content</body></html>");
167+
writer.store(updated_page);
168+
169+
// Verify updated content is actually stored
170+
hyperpage::reader reader(db_path.string());
171+
auto loaded_page = reader.load("/index.html");
172+
MAXTEST_ASSERT(loaded_page != nullptr);
173+
MAXTEST_ASSERT(loaded_page->get_path() == "/index.html");
174+
MAXTEST_ASSERT(loaded_page->get_mime_type() == "text/html");
175+
176+
// Check updated content - this is where the issue should manifest
177+
std::string updated_content = "<html><body>Updated Content</body></html>";
178+
MAXTEST_ASSERT(loaded_page->get_length() == updated_content.size());
179+
MAXTEST_ASSERT(match_buffers(loaded_page->get_content(), loaded_page->get_length(),
180+
reinterpret_cast<const uint8_t*>(updated_content.data()), updated_content.size()));
181+
};
182+
183+
MAXTEST_TEST_CASE(archive_size_no_growth_test)
184+
{
185+
std::filesystem::path db_path = std::filesystem::path(args[0]) / "hyperpage_size_test.db";
186+
187+
// Remove any existing database file to start fresh
188+
if (std::filesystem::exists(db_path)) {
189+
std::filesystem::remove(db_path);
190+
}
191+
192+
// Create initial content
193+
test_page test_page_content("/test.html", "text/html", "<html><body>Test Content</body></html>");
194+
195+
// Store initial content and record database size
196+
{
197+
hyperpage::writer writer(db_path.string());
198+
writer.store(test_page_content);
199+
}
200+
201+
size_t initial_size = std::filesystem::file_size(db_path);
202+
203+
// Overwrite with identical content multiple times
204+
for (int i = 0; i < 5; ++i) {
205+
hyperpage::writer writer(db_path.string());
206+
writer.store(test_page_content); // Same content each time
207+
}
208+
209+
size_t final_size = std::filesystem::file_size(db_path);
210+
211+
// The database size should not grow significantly when overwriting with identical content
212+
// Allow for some variance due to SQLite overhead, but it shouldn't grow substantially
213+
MAXTEST_ASSERT(final_size <= initial_size * 1.1); // Max 10% growth tolerance
214+
215+
// Verify content is still correct
216+
hyperpage::reader reader(db_path.string());
217+
auto loaded_page = reader.load("/test.html");
218+
MAXTEST_ASSERT(loaded_page != nullptr);
219+
MAXTEST_ASSERT(loaded_page->get_path() == "/test.html");
220+
std::string expected_content = "<html><body>Test Content</body></html>";
221+
MAXTEST_ASSERT(loaded_page->get_length() == expected_content.size());
222+
MAXTEST_ASSERT(match_buffers(loaded_page->get_content(), loaded_page->get_length(),
223+
reinterpret_cast<const uint8_t*>(expected_content.data()), expected_content.size()));
224+
};
149225
}

0 commit comments

Comments
 (0)