SERVER-53201 Check for removed commands in IDL compatibility checker script

8 files changed, 202 insertions, 33 deletions

diff --git a/buildscripts/idl/idl_check_compatibility.py b/buildscripts/idl/idl_check_compatibility.py
index 48754002f7b..824d80f7aa3 100644
--- a/buildscripts/idl/idl_check_compatibility.py
+++ b/buildscripts/idl/idl_check_compatibility.py
@@ -49,6 +49,7 @@ import idl_compatibility_errors
 
 def check_compatibility(old_idl_dir: str, new_idl_dir: str, import_directories: List[str]
                         ) -> idl_compatibility_errors.IDLCompatibilityErrorCollection:
+    # pylint: disable=too-many-locals,too-many-branches
     """Check IDL compatibility between old and new IDL commands."""
     ctxt = idl_compatibility_errors.IDLCompatibilityContext(
         old_idl_dir, new_idl_dir, idl_compatibility_errors.IDLCompatibilityErrorCollection())
@@ -79,9 +80,47 @@ def check_compatibility(old_idl_dir: str, new_idl_dir: str, import_directories:
                         ctxt.add_duplicate_command_name_error(new_cmd.command_name, new_idl_dir,
                                                               new_idl_file_path)
                         continue
-
                     new_commands[new_cmd.command_name] = new_cmd
 
+    # Check new commands' compatibility with old ones.
+    # Note, a command can be added to V1 at any time, it's ok if a
+    # new command has no corresponding old command.
+    old_commands: Dict[str, syntax.Command] = dict()
+    for dirpath, _, filenames in os.walk(old_idl_dir):
+        for old_filename in filenames:
+            old_idl_file_path = os.path.join(dirpath, old_filename)
+            with open(old_idl_file_path) as old_file:
+                old_idl_file = parser.parse(
+                    old_file, old_idl_file_path,
+                    CompilerImportResolver(import_directories + [old_idl_dir]))
+                if old_idl_file.errors:
+                    old_idl_file.errors.dump_errors()
+                    raise ValueError(f"Cannot parse {old_idl_file_path}")
+
+                for old_cmd in old_idl_file.spec.symbols.commands:
+                    if old_cmd.api_version == "":
+                        continue
+
+                    if old_cmd.api_version != "1":
+                        # We're not ready to handle future API versions yet.
+                        ctxt.add_command_invalid_api_version_error(
+                            old_cmd.command_name, old_cmd.api_version, old_idl_file_path)
+                        continue
+
+                    if old_cmd.command_name in old_commands:
+                        ctxt.add_duplicate_command_name_error(old_cmd.command_name, old_idl_dir,
+                                                              old_idl_file_path)
+                        continue
+
+                    old_commands[old_cmd.command_name] = old_cmd
+
+                    if old_cmd.command_name not in new_commands:
+                        # Can't remove a command from V1
+                        ctxt.add_command_removed_error(old_cmd.command_name, old_idl_file_path)
+                        continue
+
+                    new_cmd = new_commands[old_cmd.command_name]
+
     ctxt.errors.dump_errors()
     return ctxt.errors
 
diff --git a/buildscripts/idl/idl_compatibility_errors.py b/buildscripts/idl/idl_compatibility_errors.py
index 0b6470c537d..88f67dfba7a 100644
--- a/buildscripts/idl/idl_compatibility_errors.py
+++ b/buildscripts/idl/idl_compatibility_errors.py
@@ -43,6 +43,7 @@ from typing import List
 #
 ERROR_ID_COMMAND_INVALID_API_VERSION = "ID0001"
 ERROR_ID_DUPLICATE_COMMAND_NAME = "ID0002"
+ERROR_ID_REMOVED_COMMAND = "ID0003"
 
 
 class IDLCompatibilityCheckerError(Exception):
@@ -57,6 +58,7 @@ class IDLCompatibilityError(object):
 
     An IDLCompatibilityError consists of
     - error_id - IDxxxx where xxxx is a 0 leading number.
+    - command_name - a string, the command where the error occurred.
     - msg - a string describing an error.
     - old_idl_dir - a string, the directory containing the old IDL files.
     - new_idl_dir - a string, the directory containing the new IDL files.
@@ -64,10 +66,11 @@ class IDLCompatibilityError(object):
     """
 
     #pylint: disable=too-many-arguments
-    def __init__(self, error_id: str, msg: str, old_idl_dir: str, new_idl_dir: str,
-                 file: str) -> None:
+    def __init__(self, error_id: str, command_name: str, msg: str, old_idl_dir: str,
+                 new_idl_dir: str, file: str) -> None:
         """Construct an IDLCompatibility error."""
         self.error_id = error_id
+        self.command_name = command_name
         self.msg = msg
         self.old_idl_dir = old_idl_dir
         self.new_idl_dir = new_idl_dir
@@ -95,9 +98,11 @@ class IDLCompatibilityErrorCollection(object):
         self._errors: List[IDLCompatibilityError] = []
 
     #pylint: disable=too-many-arguments
-    def add(self, error_id: str, msg: str, old_idl_dir: str, new_idl_dir: str, file: str) -> None:
+    def add(self, error_id: str, command_name: str, msg: str, old_idl_dir: str, new_idl_dir: str,
+            file: str) -> None:
         """Add an error message with directory information."""
-        self._errors.append(IDLCompatibilityError(error_id, msg, old_idl_dir, new_idl_dir, file))
+        self._errors.append(
+            IDLCompatibilityError(error_id, command_name, msg, old_idl_dir, new_idl_dir, file))
 
     def has_errors(self) -> bool:
         """Have any errors been added to the collection?."""
@@ -114,6 +119,11 @@ class IDLCompatibilityErrorCollection(object):
             return error_id_list[0]
         return None
 
+    def get_error_by_command_name(self, command_name: str) -> IDLCompatibilityError:
+        """Get the first error in the error collection with the command command_name."""
+        command_name_list = [a for a in self._errors if a.command_name == command_name]
+        return next(iter(command_name_list), None)
+
     def to_list(self) -> List[str]:
         """Return a list of formatted error messages."""
         return [str(error) for error in self._errors]
@@ -150,20 +160,25 @@ class IDLCompatibilityContext(object):
         self.new_idl_dir = new_idl_dir
         self.errors = errors
 
-    def _add_error(self, error_id: str, msg: str, file: str) -> None:
+    def _add_error(self, error_id: str, command_name: str, msg: str, file: str) -> None:
         """Add an error with an error id and error message."""
-        self.errors.add(error_id, msg, self.old_idl_dir, self.new_idl_dir, file)
+        self.errors.add(error_id, command_name, msg, self.old_idl_dir, self.new_idl_dir, file)
 
     def add_command_invalid_api_version_error(self, command_name: str, api_version: str,
                                               file: str) -> None:
         """Add an error about a command with an invalid api version."""
-        self._add_error(ERROR_ID_COMMAND_INVALID_API_VERSION,
+        self._add_error(ERROR_ID_COMMAND_INVALID_API_VERSION, command_name,
                         "'%s' has an invalid API version '%s'" % (command_name, api_version), file)
 
+    def add_command_removed_error(self, command_name: str, file: str) -> None:
+        """Add an error about a command that was removed."""
+        self._add_error(ERROR_ID_REMOVED_COMMAND, command_name,
+                        "Old command '%s' was removed from new commands." % (command_name), file)
+
     def add_duplicate_command_name_error(self, command_name: str, dir_name: str, file: str) -> None:
         """Add an error about a duplicate command name within a directory."""
-        self._add_error(ERROR_ID_DUPLICATE_COMMAND_NAME,
-                        "'%s' has duplicate command name '%s'" % (dir_name, command_name), file)
+        self._add_error(ERROR_ID_DUPLICATE_COMMAND_NAME, command_name,
+                        "'%s' has duplicate command: '%s'" % (dir_name, command_name), file)
 
 
 def _assert_unique_error_messages() -> None:
diff --git a/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl b/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl
index a4660dee615..2645fef9e2b 100644
--- a/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl
+++ b/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl
@@ -33,21 +33,41 @@ imports:
     - "mongo/idl/basic_types.idl"
 
 commands:
-    invalidAPIVersion:
+    invalidAPIVersionNew:
         description: "new command fails because of invalid API version"
-        command_name: invalidAPIVersion
+        command_name: invalidAPIVersionNew
         namespace: ignored
-        cpp_name: invalidAPIVersion
+        cpp_name: invalidAPIVersionNew
         strict: true
         api_version: "whatever"
         reply_type: OkReply
 
-    duplicateCommand:
+    duplicateCommandNew:
         description: "duplicate command in new commands fails because a command with the same name
                       is in duplicate_command_fail_new.idl"
-        command_name: duplicateCommand
+        command_name: duplicateCommandNew
         namespace: ignored
-        cpp_name: duplicateCommand
+        cpp_name: duplicateCommandNew
+        strict: true
+        api_version: "1"
+        reply_type: OkReply
+
+    invalidAPIVersionOld:
+        description: "old version of this command in compatibility_test_fail_old.idl fails because
+                      of invalid API version"
+        command_name: invalidAPIVersionOld
+        namespace: ignored
+        cpp_name: invalidAPIVersionOld
+        strict: true
+        api_version: "1"
+        reply_type: OkReply
+
+    duplicateCommandOld:
+        description: "old version of this command in compatibility_test_fail_old.idl fails because
+                      a command with the same name is in duplicate_command_fail_old.idl"
+        command_name: duplicateCommandOld
+        namespace: ignored
+        cpp_name: duplicateCommandOld
         strict: true
         api_version: "1"
         reply_type: OkReply
\ No newline at end of file
diff --git a/buildscripts/idl/tests/compatibility_test_fail/new/duplicate_command_fail_new.idl b/buildscripts/idl/tests/compatibility_test_fail/new/duplicate_command_fail_new.idl
index 3498081ce6b..b507d68fa52 100644
--- a/buildscripts/idl/tests/compatibility_test_fail/new/duplicate_command_fail_new.idl
+++ b/buildscripts/idl/tests/compatibility_test_fail/new/duplicate_command_fail_new.idl
@@ -33,12 +33,12 @@ imports:
     - "mongo/idl/basic_types.idl"
 
 commands:    
-    duplicateCommand:
+    duplicateCommandNew:
         description: "duplicate command in new commands fails because a command with the same name
                       is in compatibility_test_fail_new.idl"
-        command_name: duplicateCommand
+        command_name: duplicateCommandNew
         namespace: ignored
-        cpp_name: duplicateCommand
+        cpp_name: duplicateCommandNew
         strict: true
         api_version: "1"
         reply_type: OkReply
\ No newline at end of file
diff --git a/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl b/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl
index 48a7e490618..5cfbf499254 100644
--- a/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl
+++ b/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl
@@ -33,12 +33,31 @@ imports:
     - "mongo/idl/basic_types.idl"
 
 commands:
-    invalidAPIVersion:
-        description: "old test command, the new version of this command will fail the IDL
-                      compatibility checker"
-        command_name: invalidAPIVersion
+    invalidAPIVersionOld:
+        description: "old command fails because of invalid API version"
+        command_name: invalidAPIVersionOld
         namespace: ignored
-        cpp_name: invalidAPIVersion
+        cpp_name: invalidAPIVersionOld
+        strict: true
+        api_version: "whatever"
+        reply_type: OkReply
+
+    duplicateCommandOld:
+        description: "duplicate command in old commands fails because a command with the same name
+                      is in duplicate_command_fail_old.idl"
+        command_name: duplicateCommandOld
+        namespace: ignored
+        cpp_name: duplicateCommandOld
+        strict: true
+        api_version: "1"
+        reply_type: OkReply
+
+    removedCommand:
+        description: "command will be removed from compatibility_test_fail/new which results 
+                      in an error"
+        command_name: removedCommand
+        namespace: ignored
+        cpp_name: removedCommand
         strict: true
         api_version: "1"
         reply_type: OkReply
\ No newline at end of file
diff --git a/buildscripts/idl/tests/compatibility_test_fail/old/duplicate_command_fail_old.idl b/buildscripts/idl/tests/compatibility_test_fail/old/duplicate_command_fail_old.idl
new file mode 100644
index 00000000000..82772b51df7
--- /dev/null
+++ b/buildscripts/idl/tests/compatibility_test_fail/old/duplicate_command_fail_old.idl
@@ -0,0 +1,44 @@
+# Copyright (C) 2021-present MongoDB, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the Server Side Public License, version 1,
+# as published by MongoDB, Inc.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# Server Side Public License for more details.
+#
+# You should have received a copy of the Server Side Public License
+# along with this program. If not, see
+# <http://www.mongodb.com/licensing/server-side-public-license>.
+#
+# As a special exception, the copyright holders give permission to link the
+# code of portions of this program with the OpenSSL library under certain
+# conditions as described in each individual source file and distribute
+# linked combinations including the program with the OpenSSL library. You
+# must comply with the Server Side Public License in all respects for
+# all of the code used other than as permitted herein. If you modify file(s)
+# with this exception, you may extend this exception to your version of the
+# file(s), but you are not obligated to do so. If you do not wish to do so,
+# delete this exception statement from your version. If you delete this
+# exception statement from all source files in the program, then also delete
+# it in the license file.
+#
+
+global:
+    cpp_namespace: "mongo"
+
+imports:
+    - "mongo/idl/basic_types.idl"
+
+commands:    
+    duplicateCommandOld:
+        description: "duplicate command in old commands fails because a command with the same name
+                      is in compatibility_test_fail_old.idl"
+        command_name: duplicateCommandOld
+        namespace: ignored
+        cpp_name: duplicateCommandOld
+        strict: true
+        api_version: "1"
+        reply_type: OkReply
\ No newline at end of file
diff --git a/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl b/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl
index edba5ad66d2..54c7c6502ae 100644
--- a/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl
+++ b/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl
@@ -41,3 +41,12 @@ commands:
         strict: true
         api_version: "1"
         reply_type: OkReply
+
+    removedCommandNotInAPIV1:
+        description: "removing a command not in API v1 passes"
+        command_name: removedCommandNotInAPIV1
+        namespace: ignored
+        cpp_name: removedCommandNotInAPIV1
+        strict: true
+        api_version: ""
+        reply_type: OkReply
diff --git a/buildscripts/idl/tests/test_compatibility.py b/buildscripts/idl/tests/test_compatibility.py
index 6a36255996e..ca4ba8c976a 100644
--- a/buildscripts/idl/tests/test_compatibility.py
+++ b/buildscripts/idl/tests/test_compatibility.py
@@ -61,17 +61,40 @@ class TestIDLCompatibilityChecker(unittest.TestCase):
             path.join(dir_path, "compatibility_test_fail/new"), ["src"])
 
         self.assertTrue(error_collection.has_errors())
-        self.assertTrue(error_collection.count() == 2)
+        self.assertTrue(error_collection.count() == 5)
 
-        invalid_api_version_error = error_collection.get_error(
-            idl_compatibility_errors.ERROR_ID_COMMAND_INVALID_API_VERSION)
-        self.assertIsNotNone(invalid_api_version_error)
-        self.assertRegex(str(invalid_api_version_error), "invalidAPIVersion")
+        invalid_api_version_new_error = error_collection.get_error_by_command_name(
+            "invalidAPIVersionNew")
+        self.assertIsNotNone(invalid_api_version_new_error)
+        self.assertTrue(invalid_api_version_new_error.error_id ==
+                        idl_compatibility_errors.ERROR_ID_COMMAND_INVALID_API_VERSION)
+        self.assertRegex(str(invalid_api_version_new_error), "invalidAPIVersionNew")
 
-        duplicate_command_error = error_collection.get_error(
-            idl_compatibility_errors.ERROR_ID_DUPLICATE_COMMAND_NAME)
-        self.assertIsNotNone(duplicate_command_error)
-        self.assertRegex(str(duplicate_command_error), "duplicateCommand")
+        duplicate_command_new_error = error_collection.get_error_by_command_name(
+            "duplicateCommandNew")
+        self.assertIsNotNone(duplicate_command_new_error)
+        self.assertTrue(duplicate_command_new_error.error_id ==
+                        idl_compatibility_errors.ERROR_ID_DUPLICATE_COMMAND_NAME)
+        self.assertRegex(str(duplicate_command_new_error), "duplicateCommandNew")
+
+        invalid_api_version_old_error = error_collection.get_error_by_command_name(
+            "invalidAPIVersionOld")
+        self.assertIsNotNone(invalid_api_version_old_error)
+        self.assertTrue(invalid_api_version_old_error.error_id ==
+                        idl_compatibility_errors.ERROR_ID_COMMAND_INVALID_API_VERSION)
+        self.assertRegex(str(invalid_api_version_old_error), "invalidAPIVersionOld")
+
+        duplicate_command_old_error = error_collection.get_error_by_command_name(
+            "duplicateCommandOld")
+        self.assertIsNotNone(duplicate_command_old_error)
+        self.assertTrue(duplicate_command_old_error.error_id ==
+                        idl_compatibility_errors.ERROR_ID_DUPLICATE_COMMAND_NAME)
+        self.assertRegex(str(duplicate_command_old_error), "duplicateCommandOld")
+
+        removed_command_error = error_collection.get_error(
+            idl_compatibility_errors.ERROR_ID_REMOVED_COMMAND)
+        self.assertIsNotNone(removed_command_error)
+        self.assertRegex(str(removed_command_error), "removedCommand")
 
 
 if __name__ == '__main__':