[gparted] Encapsulate operation merging inside the Operation* classes (#755214)

[Curtis Gedak <gedakc src gnome org>]

commit 7f4ffd28d539a772258fe31cd5027dcd28706f20
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Sat Sep 12 14:59:40 2015 +0100

    Encapsulate operation merging inside the Operation* classes (#755214)
    
    Win_GParted::Merge_Operations() method was modifying the internals of
    Operation* objects; in particular the partition_new member variable.
    This is breaking data hiding and encapsulation tenant of object oriented
    programming.
    
    Implement exactly the same operation merge semantics, but hide the
    manipulation of the internals of the Operation* objects within the
    Operation* classes themselves.
    
    Bug 755214 - Refactor operation merging

 include/Operation.h                |    1 +
 include/OperationChangeUUID.h      |    1 +
 include/OperationCheck.h           |    1 +
 include/OperationCopy.h            |    1 +
 include/OperationCreate.h          |    1 +
 include/OperationDelete.h          |    1 +
 include/OperationFormat.h          |    1 +
 include/OperationLabelFileSystem.h |    1 +
 include/OperationNamePartition.h   |    1 +
 include/OperationResizeMove.h      |    3 +-
 src/OperationChangeUUID.cc         |   19 ++++++++++
 src/OperationCheck.cc              |   11 +++++-
 src/OperationCopy.cc               |    6 +++-
 src/OperationCreate.cc             |    6 +++-
 src/OperationDelete.cc             |    8 +++-
 src/OperationFormat.cc             |   14 +++++++-
 src/OperationLabelFileSystem.cc    |   13 +++++++
 src/OperationNamePartition.cc      |   13 +++++++
 src/OperationResizeMove.cc         |   13 +++++++
 src/Win_GParted.cc                 |   71 +-----------------------------------
 20 files changed, 109 insertions(+), 77 deletions(-)
---
diff --git a/include/Operation.h b/include/Operation.h
index a377a2c..4a81493 100644
--- a/include/Operation.h
+++ b/include/Operation.h
@@ -45,6 +45,7 @@ public:
        
        virtual void apply_to_visual( std::vector<Partition> & partitions ) = 0 ;
        virtual void create_description() = 0 ;
+       virtual bool merge_operations( const Operation & candidate ) = 0;
 
        //public variables
        Device device ;
diff --git a/include/OperationChangeUUID.h b/include/OperationChangeUUID.h
index 5d2c93b..e34a0fd 100644
--- a/include/OperationChangeUUID.h
+++ b/include/OperationChangeUUID.h
@@ -34,6 +34,7 @@ void apply_to_visual( std::vector<Partition> & partitions ) ;
 
 private:
        void create_description() ;
+       bool merge_operations( const Operation & candidate );
 } ;
 
 } //GParted
diff --git a/include/OperationCheck.h b/include/OperationCheck.h
index 65603d1..1d62ed2 100644
--- a/include/OperationCheck.h
+++ b/include/OperationCheck.h
@@ -31,6 +31,7 @@ public:
 
 private:
        void create_description() ;
+       bool merge_operations( const Operation & candidate );
 } ;
 
 } //GParted
diff --git a/include/OperationCopy.h b/include/OperationCopy.h
index 71b3bd1..43bbed4 100644
--- a/include/OperationCopy.h
+++ b/include/OperationCopy.h
@@ -36,6 +36,7 @@ public:
 
 private:
        void create_description() ;
+       bool merge_operations( const Operation & candidate );
 } ;
 
 } //GParted
diff --git a/include/OperationCreate.h b/include/OperationCreate.h
index 62023b6..223b10a 100644
--- a/include/OperationCreate.h
+++ b/include/OperationCreate.h
@@ -33,6 +33,7 @@ public:
 
 private:
        void create_description() ;
+       bool merge_operations( const Operation & candidate );
 } ;
 
 } //GParted
diff --git a/include/OperationDelete.h b/include/OperationDelete.h
index c1155cc..333b919 100644
--- a/include/OperationDelete.h
+++ b/include/OperationDelete.h
@@ -31,6 +31,7 @@ public:
                
 private:
        void create_description() ;
+       bool merge_operations( const Operation & candidate );
        void remove_original_and_adjacent_unallocated( std::vector<Partition> & partitions, int index_orig ) ;
 } ;
 
diff --git a/include/OperationFormat.h b/include/OperationFormat.h
index 22732ce..afc4d5b 100644
--- a/include/OperationFormat.h
+++ b/include/OperationFormat.h
@@ -33,6 +33,7 @@ public:
 
 private:
        void create_description() ;
+       bool merge_operations( const Operation & candidate );
 } ;
 
 } //GParted
diff --git a/include/OperationLabelFileSystem.h b/include/OperationLabelFileSystem.h
index 996f4a0..fa3459d 100644
--- a/include/OperationLabelFileSystem.h
+++ b/include/OperationLabelFileSystem.h
@@ -33,6 +33,7 @@ public:
 
 private:
        void create_description() ;
+       bool merge_operations( const Operation & candidate );
 } ;
 
 } //GParted
diff --git a/include/OperationNamePartition.h b/include/OperationNamePartition.h
index 955fbaf..1ac822b 100644
--- a/include/OperationNamePartition.h
+++ b/include/OperationNamePartition.h
@@ -33,6 +33,7 @@ public:
 
 private:
        void create_description();
+       bool merge_operations( const Operation & candidate );
 };
 
 } //GParted
diff --git a/include/OperationResizeMove.h b/include/OperationResizeMove.h
index a625179..96bd06d 100644
--- a/include/OperationResizeMove.h
+++ b/include/OperationResizeMove.h
@@ -33,7 +33,8 @@ public:
 
 private:
        void create_description() ;
-       
+       bool merge_operations( const Operation & candidate );
+
        void apply_normal_to_visual( std::vector<Partition> & partitions ) ;
        void apply_extended_to_visual( std::vector<Partition> & partitions ) ;
        
diff --git a/src/OperationChangeUUID.cc b/src/OperationChangeUUID.cc
index 71e4577..e05d404 100644
--- a/src/OperationChangeUUID.cc
+++ b/src/OperationChangeUUID.cc
@@ -69,4 +69,23 @@ void OperationChangeUUID::create_description()
        }
 }
 
+bool OperationChangeUUID::merge_operations( const Operation & candidate )
+{
+       if ( candidate.type == OPERATION_CHANGE_UUID        &&
+            partition_new  == candidate.partition_original    )
+       {
+               // Changing half the UUID must not override changing all of it
+               if ( candidate.partition_new.uuid != UUID_RANDOM_NTFS_HALF )
+               {
+                       partition_new.uuid = candidate.partition_new.uuid;
+                       create_description();
+               }
+               // Else no change required to this operation to merge candidate
+
+               return true;
+       }
+
+       return false;
+}
+
 } //GParted
diff --git a/src/OperationCheck.cc b/src/OperationCheck.cc
index 80c3cc6..b3c5132 100644
--- a/src/OperationCheck.cc
+++ b/src/OperationCheck.cc
@@ -39,5 +39,14 @@ void OperationCheck::create_description()
                                        partition_original .get_path() ) ;
 }
 
-} //GParted
+bool OperationCheck::merge_operations( const Operation & candidate )
+{
+       if ( candidate.type     == OPERATION_CHECK              &&
+            partition_original == candidate.partition_original    )
+               // No steps required to merge this operation
+               return true;
 
+       return false;
+}
+
+} //GParted
diff --git a/src/OperationCopy.cc b/src/OperationCopy.cc
index 7b530e3..528d4bf 100644
--- a/src/OperationCopy.cc
+++ b/src/OperationCopy.cc
@@ -90,5 +90,9 @@ void OperationCopy::create_description()
        }
 }
 
-} //GParted
+bool OperationCopy::merge_operations( const Operation & candidate )
+{
+       return false;  // Never merge copy operations
+}
 
+} //GParted
diff --git a/src/OperationCreate.cc b/src/OperationCreate.cc
index b561c00..2c3a648 100644
--- a/src/OperationCreate.cc
+++ b/src/OperationCreate.cc
@@ -92,5 +92,9 @@ void OperationCreate::create_description()
                                        device .get_path() ) ;
 }
 
-} //GParted
+bool OperationCreate::merge_operations( const Operation & candidate )
+{
+       return false;  // Never merge create operations
+}
 
+} //GParted
diff --git a/src/OperationDelete.cc b/src/OperationDelete.cc
index 2b293d1..a04c5f0 100644
--- a/src/OperationDelete.cc
+++ b/src/OperationDelete.cc
@@ -84,7 +84,12 @@ void OperationDelete::create_description()
                                        Utils::format_size( partition_original .get_sector_length(), 
partition_original .sector_size ),
                                        partition_original .device_path ) ;
 }
-       
+
+bool OperationDelete::merge_operations( const Operation & candidate )
+{
+       return false;  // Can't merge with an already deleted partition
+}
+
 void OperationDelete::remove_original_and_adjacent_unallocated( std::vector<Partition> & partitions, int 
index_orig ) 
 {
        //remove unallocated space following the original partition
@@ -101,4 +106,3 @@ void OperationDelete::remove_original_and_adjacent_unallocated( std::vector<Part
 }
 
 } //GParted
-
diff --git a/src/OperationFormat.cc b/src/OperationFormat.cc
index 8dec4bd..3df797d 100644
--- a/src/OperationFormat.cc
+++ b/src/OperationFormat.cc
@@ -74,5 +74,17 @@ void OperationFormat::create_description()
                                        Utils::get_filesystem_string( partition_new .filesystem ) ) ;
 }
 
-} //GParted
+bool OperationFormat::merge_operations( const Operation & candidate )
+{
+       if ( candidate.type == OPERATION_FORMAT             &&
+            partition_new  == candidate.partition_original    )
+       {
+               partition_new = candidate.partition_new;
+               create_description();
+               return true;
+       }
 
+       return false;
+}
+
+} //GParted
diff --git a/src/OperationLabelFileSystem.cc b/src/OperationLabelFileSystem.cc
index 12d1015..9a07426 100644
--- a/src/OperationLabelFileSystem.cc
+++ b/src/OperationLabelFileSystem.cc
@@ -65,4 +65,17 @@ void OperationLabelFileSystem::create_description()
        }
 }
 
+bool OperationLabelFileSystem::merge_operations( const Operation & candidate )
+{
+       if ( candidate.type == OPERATION_LABEL_FILESYSTEM   &&
+            partition_new  == candidate.partition_original    )
+       {
+               partition_new.set_filesystem_label( candidate.partition_new.get_filesystem_label() );
+               create_description();
+               return true;
+       }
+
+       return false;
+}
+
 } //GParted
diff --git a/src/OperationNamePartition.cc b/src/OperationNamePartition.cc
index edda1ec..84aca51 100644
--- a/src/OperationNamePartition.cc
+++ b/src/OperationNamePartition.cc
@@ -68,4 +68,17 @@ void OperationNamePartition::create_description()
        }
 }
 
+bool OperationNamePartition::merge_operations( const Operation & candidate )
+{
+       if ( candidate.type == OPERATION_NAME_PARTITION     &&
+            partition_new  == candidate.partition_original    )
+       {
+               partition_new.name = candidate.partition_new.name;
+               create_description();
+               return true;
+       }
+
+       return false;
+}
+
 } //GParted
diff --git a/src/OperationResizeMove.cc b/src/OperationResizeMove.cc
index fe31312..16982f6 100644
--- a/src/OperationResizeMove.cc
+++ b/src/OperationResizeMove.cc
@@ -208,4 +208,17 @@ void OperationResizeMove::remove_adjacent_unallocated( std::vector<Partition> &
                partitions .erase( partitions .begin() + ( index_orig -1 ) ) ;
 }
 
+bool OperationResizeMove::merge_operations( const Operation & candidate )
+{
+       if ( candidate.type == OPERATION_RESIZE_MOVE        &&
+            partition_new  == candidate.partition_original    )
+       {
+               partition_new = candidate.partition_new;
+               create_description();
+               return true;
+       }
+
+       return false;
+}
+
 } //GParted
diff --git a/src/Win_GParted.cc b/src/Win_GParted.cc
index 97b6b10..aa762e9 100644
--- a/src/Win_GParted.cc
+++ b/src/Win_GParted.cc
@@ -746,78 +746,9 @@ bool Win_GParted::Merge_Operations( unsigned int first, unsigned int second )
        if( first >= operations .size() || second >= operations .size() )
                return false;
 
-       // Two resize operations of the same partition
-       if ( operations[ first ]->type == OPERATION_RESIZE_MOVE &&
-            operations[ second ]->type == OPERATION_RESIZE_MOVE &&
-            operations[ first ]->partition_new == operations[ second ]->partition_original
-          )
-       {
-               operations[ first ]->partition_new = operations[ second ]->partition_new;
-               operations[ first ]->create_description() ;
-               remove_operation( second );
-
-               return true;
-       }
-       // Two label change operations on the same partition
-       else if ( operations[ first ]->type == OPERATION_LABEL_FILESYSTEM &&
-                 operations[ second ]->type == OPERATION_LABEL_FILESYSTEM &&
-                 operations[ first ]->partition_new == operations[ second ]->partition_original
-               )
-       {
-               operations[ first ]->partition_new.set_filesystem_label(
-                                       operations[ second ]->partition_new.get_filesystem_label() );
-               operations[ first ]->create_description() ;
-               remove_operation( second );
-
-               return true;
-       }
-       // Two name change operations on the same partition
-       else if ( operations[first]->type == OPERATION_NAME_PARTITION &&
-                 operations[second]->type == OPERATION_NAME_PARTITION &&
-                 operations[first]->partition_new == operations[second]->partition_original
-               )
-       {
-               operations[first]->partition_new.name = operations[second]->partition_new.name;
-               operations[first]->create_description();
-               remove_operation( second );
-
-               return true;
-       }
-       // Two change-uuid change operations on the same partition
-       else if ( operations[ first ]->type == OPERATION_CHANGE_UUID &&
-                 operations[ second ]->type == OPERATION_CHANGE_UUID &&
-                 operations[ first ]->partition_new == operations[ second ]->partition_original
-               )
-       {
-               // Changing half the UUID should not override changing all of it
-               if ( operations[ first ]->partition_new.uuid == UUID_RANDOM_NTFS_HALF ||
-                    operations[ second ]->partition_new.uuid == UUID_RANDOM )
-                       operations[ first ]->partition_new.uuid = operations[ second ]->partition_new.uuid;
-               operations[ first ]->create_description() ;
-               remove_operation( second );
-
-               return true;
-       }
-       // Two check operations of the same partition
-       else if ( operations[ first ]->type == OPERATION_CHECK &&
-                 operations[ second ]->type == OPERATION_CHECK &&
-                 operations[ first ]->partition_original == operations[ second ]->partition_original
-               )
-       {
-               remove_operation( second );
-
-               return true;
-       }
-       // Two format operations of the same partition
-       else if ( operations[ first ]->type == OPERATION_FORMAT &&
-                 operations[ second ]->type == OPERATION_FORMAT &&
-                 operations[ first ]->partition_new == operations[ second ]->partition_original
-               )
+       if ( operations[first]->merge_operations( *operations[second] ) )
        {
-               operations[ first ]->partition_new = operations[ second ]->partition_new;
-               operations[ first ]->create_description() ;
                remove_operation( second );
-
                return true;
        }