handle save overwriting file being edited (#241)

[node_publish]

CMakeLists.txt

@@ -13,7 +13,7 @@ cmake_minimum_required(VERSION 3.13)
 
 # Project information
 project(omega_edit
-        VERSION 0.8.1
+        VERSION 0.8.3
         DESCRIPTION "Apache open source library for building editors"
         HOMEPAGE_URL "https://github.com/ctc-oss/omega-edit"
         LANGUAGES C CXX)

src/include/omega_edit/edit.h

@@ -32,13 +32,11 @@ extern "C" {
 /** On session change callback.  This under-defined function will be called when an associated session changes. */
 // TODO: Session change events can now be session creation, checkpoints, session clearing, transformations, session
 //  saving, in addition to changes, so we might want to consider adding a session change event type to the callback
-typedef void (*omega_session_event_cbk_t)(const omega_session_t *, omega_session_event_t session_event,
-                                          const omega_change_t *);
+typedef void (*omega_session_event_cbk_t)(const omega_session_t *, omega_session_event_t, const omega_change_t *);
 
 /** On viewport change callback.  This under-defined function will be called when an associated viewport changes. */
 // TODO: Like session changes, there are events other than standard changes that could change the state of a viewport
-typedef void (*omega_viewport_event_cbk_t)(const omega_viewport_t *, omega_viewport_event_t viewport_event,
-                                           const omega_change_t *);
+typedef void (*omega_viewport_event_cbk_t)(const omega_viewport_t *, omega_viewport_event_t, const omega_change_t *);
 
 /**
  * Create a file editing session from a file path
@@ -111,7 +109,7 @@ OMEGA_EDIT_EXPORT int64_t omega_edit_redo_last_undo(omega_session_t *session_ptr
  * this parameter is non-null, the saved file path will be copied here (must be able to accommodate FILENAME_MAX bytes)
  * @return 0 on success, non-zero otherwise
  */
-OMEGA_EDIT_EXPORT int omega_edit_save(const omega_session_t *session_ptr, const char *file_path, int overwrite,
+OMEGA_EDIT_EXPORT int omega_edit_save(omega_session_t *session_ptr, const char *file_path, int overwrite,
                                       char *saved_file_path);
 
 /**

src/include/omega_edit/filesystem.h

@@ -81,6 +81,14 @@ OMEGA_EDIT_EXPORT int omega_util_remove_directory(char const *path);
  */
 OMEGA_EDIT_EXPORT int64_t omega_util_file_size(char const *path);
 
+/**
+ * Given two file paths, determine if they are equivalent
+ * @param path1 first path
+ * @param path2 second path
+ * @return non-zero if the paths are equivalent and zero otherwise
+ */
+OMEGA_EDIT_EXPORT int omega_util_paths_equivalent(char const *path1, char const *path2);
+
 /**
  * Given a file name, return the associated basename (filename without the directory) and if a matching suffix is given, the returned basename will have the suffix removed
  * @param path file path

src/lib/edit.cpp

@@ -435,7 +435,7 @@ int omega_edit_apply_transform(omega_session_t *session_ptr, omega_util_byte_tra
     return -1;
 }
 
-int omega_edit_save(const omega_session_t *session_ptr, const char *file_path, int overwrite, char *saved_file_path) {
+int omega_edit_save(omega_session_t *session_ptr, const char *file_path, int overwrite, char *saved_file_path) {
     char temp_filename[FILENAME_MAX];
     omega_util_dirname(file_path, temp_filename);
     if (!temp_filename[0]) { omega_util_get_current_dir(temp_filename); }
@@ -501,8 +501,18 @@ int omega_edit_save(const omega_session_t *session_ptr, const char *file_path, i
         write_offset += segment->computed_length;
     }
     fclose(temp_fptr);
+    bool reset_session = false;
     if (omega_util_file_exists(file_path)) {
         if (overwrite) {
+            // reset the session if we're overwriting the file being edited
+            reset_session =
+                    (omega_session_get_file_path(session_ptr) &&
+                     omega_util_paths_equivalent(file_path, omega_session_get_file_path(session_ptr)));
+            if (reset_session) {
+                assert(session_ptr->models_.front()->file_ptr);
+                fclose(session_ptr->models_.front()->file_ptr);
+                session_ptr->models_.front()->file_ptr = nullptr;
+            }
             if (0 != omega_util_remove_file(file_path)) {
                 LOG_ERROR("removing file failed: " << strerror(errno));
                 return -7;
@@ -518,7 +528,22 @@ int omega_edit_save(const omega_session_t *session_ptr, const char *file_path, i
         LOG_ERROR("rename failed: " << strerror(errno));
         return -9;
     }
-    if (saved_file_path) { strncpy(saved_file_path, file_path, FILENAME_MAX); }
+    if (reset_session) {
+        assert(!session_ptr->models_.front()->file_ptr);
+        auto file_ptr = fopen(file_path, "rb");
+        if (!file_ptr) {
+            LOG_ERROR("fopen failed: " << strerror(errno));
+            return -10;
+        }
+        // session to point to the overwritten file, then suspend viewport callbacks, clear the changes, then resume the
+        // viewport callbacks.  A session clear event will be triggered in the clear changes call to indicate to the
+        // user that the edit history is cleared, affecting undo/redo.
+        session_ptr->models_.front()->file_ptr = file_ptr;
+        omega_session_pause_viewport_event_callbacks(session_ptr);
+        omega_edit_clear_changes(session_ptr);
+        omega_session_resume_viewport_event_callbacks(session_ptr);
+    }
+    if (saved_file_path) { omega_util_normalize_path(file_path, saved_file_path); }
     omega_session_notify(session_ptr, SESSION_EVT_SAVE, nullptr);
     return 0;
 }

src/lib/filesystem.cpp

@@ -32,6 +32,8 @@ int omega_util_remove_directory(char const *path) { return (fs::remove(path)) ?
 
 int64_t omega_util_file_size(char const *path) { return static_cast<int64_t>(fs::file_size(path)); }
 
+int omega_util_paths_equivalent(char const *path1, char const *path2) { return fs::equivalent(path1, path2) ? 1 : 0; }
+
 const char *omega_util_get_current_dir(char *buffer) {
     static char buff[FILENAME_MAX];//create string buffer to hold path
     if (!buffer) { buffer = buff; }

src/rpc/client/ts/package.json

@@ -1,6 +1,6 @@
 {
     "name": "omega-edit",
-    "version": "0.8.2",
+    "version": "0.8.3",
     "description": "OmegaEdit gRPC Client TypeScript Types",
     "publisher": "ctc-oss",
     "repository": {

src/tests/integration/data/session_save.expected.1.dat

@@ -0,0 +1 @@
+0123456789abcdefghijklmnopqrstuvwxyz

src/tests/integration/data/session_save.expected.2.dat

@@ -0,0 +1 @@
+0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ

src/tests/integration/omega_test.cpp

@@ -414,27 +414,27 @@ TEST_CASE("Model Tests", "[ModelTests]") {
     REQUIRE(0 == omega_edit_save(session_ptr, "data/model-test.actual.1.dat", 0, saved_filename));
     REQUIRE(0 != compare_files("data/model-test.dat", "data/model-test.actual.1.dat"));
     REQUIRE(0 == compare_files("data/model-test.expected.1.dat", "data/model-test.actual.1.dat"));
-    REQUIRE_THAT(saved_filename, Equals("data/model-test.actual.1.dat"));
+    REQUIRE(omega_util_paths_equivalent("data/model-test.actual.1.dat", saved_filename));
     omega_util_remove_file("data/model-test.actual.1-1.dat");
     REQUIRE(0 == omega_edit_save(session_ptr, "data/model-test.actual.1.dat", 0, saved_filename));
     REQUIRE(0 == compare_files("data/model-test.actual.1.dat", "data/model-test.actual.1-1.dat"));
-    REQUIRE(0 == strcmp("data/model-test.actual.1-1.dat", saved_filename));
+    REQUIRE(omega_util_paths_equivalent("data/model-test.actual.1-1.dat", saved_filename));
     omega_util_remove_file("data/model-test.actual.1-2.dat");
     REQUIRE(0 == omega_edit_save(session_ptr, "data/model-test.actual.1.dat", 0, saved_filename));
     REQUIRE(0 == compare_files("data/model-test.actual.1.dat", "data/model-test.actual.1-2.dat"));
-    REQUIRE(0 == strcmp("data/model-test.actual.1-2.dat", saved_filename));
+    REQUIRE(omega_util_paths_equivalent("data/model-test.actual.1-2.dat", saved_filename));
     REQUIRE(0 < omega_edit_insert_bytes(session_ptr, 10, reinterpret_cast<const omega_byte_t *>("0"), 1));
     file_size += 1;
     REQUIRE(omega_session_get_computed_file_size(session_ptr) == file_size);
     omega_util_remove_file("data/model-test.actual.2.dat");
     REQUIRE(0 == omega_edit_save(session_ptr, "data/model-test.actual.2.dat", 0, saved_filename));
     REQUIRE(0 == compare_files("data/model-test.expected.2.dat", "data/model-test.actual.2.dat"));
-    REQUIRE(0 == strcmp("data/model-test.actual.2.dat", saved_filename));
+    REQUIRE(omega_util_paths_equivalent("data/model-test.actual.2.dat", saved_filename));
     REQUIRE(0 < omega_edit_insert_bytes(session_ptr, 5, reinterpret_cast<const omega_byte_t *>("xxx"), 0));
     file_size += 3;
     REQUIRE(omega_session_get_computed_file_size(session_ptr) == file_size);
     REQUIRE(0 == omega_edit_save(session_ptr, "data/model-test.actual.3.dat", 1, saved_filename));
-    REQUIRE(0 == strcmp("data/model-test.actual.3.dat", saved_filename));
+    REQUIRE(omega_util_paths_equivalent("data/model-test.actual.3.dat", saved_filename));
     REQUIRE(0 == compare_files("data/model-test.expected.3.dat", "data/model-test.actual.3.dat"));
     auto num_changes = file_info.num_changes;
     REQUIRE(num_changes * -1 == omega_edit_undo_last_change(session_ptr));
@@ -449,7 +449,7 @@ TEST_CASE("Model Tests", "[ModelTests]") {
     REQUIRE(omega_session_get_computed_file_size(session_ptr) == file_size);
     REQUIRE(0 == omega_edit_save(session_ptr, "data/model-test.actual.4.dat", 1, saved_filename));
     REQUIRE(0 == compare_files("data/model-test.expected.4.dat", "data/model-test.actual.4.dat"));
-    REQUIRE(0 == strcmp("data/model-test.actual.4.dat", saved_filename));
+    REQUIRE(omega_util_paths_equivalent("data/model-test.actual.4.dat", saved_filename));
     REQUIRE(1 == omega_session_get_num_undone_changes(session_ptr));
     REQUIRE(0 < omega_edit_overwrite_string(session_ptr, 0, "-"));
     REQUIRE(0 == omega_session_get_num_undone_changes(session_ptr));
@@ -809,7 +809,7 @@ TEST_CASE("File Viewing", "[InitTests]") {
     omega_edit_destroy_viewport(viewport_ptr);
     REQUIRE(viewport_count - 1 == omega_session_get_num_viewports(session_ptr));
     omega_edit_destroy_session(session_ptr);
-    remove(file_name);
+    omega_util_remove_file(file_name);
 }
 
 TEST_CASE("Viewports", "[ViewportTests]") {
@@ -835,3 +835,74 @@ TEST_CASE("Viewports", "[ViewportTests]") {
     REQUIRE(0 == omega_session_get_num_viewports(session_ptr));
     omega_edit_destroy_session(session_ptr);
 }
+
+void session_save_test_session_cbk(const omega_session_t *session_ptr, omega_session_event_t session_event,
+                                   const omega_change_t *) {
+    auto count_ptr = reinterpret_cast<int *>(omega_session_get_user_data_ptr(session_ptr));
+    std::clog << "Session Event: " << session_event << std::endl;
+    ++*count_ptr;
+}
+
+void session_save_test_viewport_cbk(const omega_viewport_t *viewport_ptr, omega_viewport_event_t viewport_event,
+                                    const omega_change_t *) {
+    auto count_ptr = reinterpret_cast<int *>(omega_viewport_get_user_data_ptr(viewport_ptr));
+    std::clog << "Viewport Event: " << viewport_event << std::endl;
+    ++*count_ptr;
+}
+
+TEST_CASE("Session Save", "[SessionSaveTests]") {
+    char saved_filename[FILENAME_MAX];
+    int session_events_count = 0;
+    int viewport_events_count = 0;
+    auto session_ptr = omega_edit_create_session(nullptr, session_save_test_session_cbk, &session_events_count);
+    auto viewport_ptr =
+            omega_edit_create_viewport(session_ptr, 0, 100, session_save_test_viewport_cbk, &viewport_events_count, 0);
+    REQUIRE(1 == session_events_count); // SESSION_EVT_CREATE
+    REQUIRE(1 == viewport_events_count);// VIEWPORT_EVT_CREATE
+    omega_edit_insert_string(session_ptr, 0, "0123456789");
+    REQUIRE(2 == session_events_count); // SESSION_EVT_EDIT
+    REQUIRE(2 == viewport_events_count);// VIEWPORT_EVT_EDIT
+    omega_util_remove_file("data/session_save.1.dat");
+    omega_edit_save(session_ptr, "data/session_save.1.dat", 1, saved_filename);
+    REQUIRE(omega_util_paths_equivalent("data/session_save.1.dat", saved_filename));
+    REQUIRE(3 == session_events_count);// SESSION_EVT_SAVE
+    REQUIRE(2 == viewport_events_count);// no additional viewport events
+    omega_edit_destroy_session(session_ptr);
+    session_events_count = 0;
+    viewport_events_count = 0;
+    session_ptr =
+            omega_edit_create_session("data/session_save.1.dat", session_save_test_session_cbk, &session_events_count);
+    viewport_ptr =
+            omega_edit_create_viewport(session_ptr, 0, 100, session_save_test_viewport_cbk, &viewport_events_count, 0);
+    omega_edit_insert_string(session_ptr, omega_session_get_computed_file_size(session_ptr),
+                             "abcdefghijklmnopqrstuvwxyz");
+    REQUIRE(1 == omega_session_get_num_changes(session_ptr));
+    REQUIRE(2 == session_events_count);
+    REQUIRE(2 == viewport_events_count);
+    omega_edit_save(session_ptr, "data/session_save.1.dat", 1, saved_filename);
+    REQUIRE(omega_util_paths_equivalent("data/session_save.1.dat", saved_filename));
+    REQUIRE(0 == omega_session_get_num_changes(session_ptr));
+    REQUIRE(4 == session_events_count);// SESSION_EVT_CLEAR and SESSION_EVT_SAVE
+    REQUIRE(2 == viewport_events_count);// no additional viewport events
+    omega_edit_insert_string(session_ptr, omega_session_get_computed_file_size(session_ptr),
+                             "ABCDEFGHIJKLMNOPQRSTUVWXYZ");
+    REQUIRE(1 == omega_session_get_num_changes(session_ptr));
+    omega_util_remove_file("data/session_save.1-1.dat");
+    REQUIRE(5 == session_events_count);// SESSION_EVT_SAVE
+    REQUIRE(3 == viewport_events_count);// no additional viewport events
+    omega_edit_save(session_ptr, "data/session_save.1.dat", 0, saved_filename);
+    REQUIRE(6 == session_events_count);// SESSION_EVT_SAVE
+    REQUIRE(omega_util_paths_equivalent("data/session_save.1-1.dat", saved_filename));
+    REQUIRE(0 == compare_files("data/session_save.expected.2.dat", "data/session_save.1-1.dat"));
+    omega_util_remove_file("data/session_save.1-2.dat");
+    omega_edit_save(session_ptr, "data/session_save.1.dat", 0, saved_filename);
+    REQUIRE(7 == session_events_count);// SESSION_EVT_SAVE
+    REQUIRE(omega_util_paths_equivalent("data/session_save.1-2.dat", saved_filename));
+    REQUIRE(0 == compare_files("data/session_save.expected.2.dat", "data/session_save.1-2.dat"));
+    omega_util_remove_file("data/session_save.1-3.dat");
+    omega_edit_save(session_ptr, "data/session_save.1.dat", 0, saved_filename);
+    REQUIRE(8 == session_events_count);// SESSION_EVT_SAVE
+    REQUIRE(omega_util_paths_equivalent("data/session_save.1-3.dat", saved_filename));
+    REQUIRE(0 == compare_files("data/session_save.expected.2.dat", "data/session_save.1-3.dat"));
+    omega_edit_destroy_session(session_ptr);
+}