Add field number checks for required fields

Author: Tobias Hafner

doc-generator/config.py

@@ -10,24 +10,26 @@
 # This file must be inside the PROTO_DIR also specified in this config.
 RPC_DEFINITION_FILE = 'protointerface.proto'
 
-# These fields must be present in the top level request message. If on of them is missing, the documentation will not
-# generate and an exception will be thrown.
+# These fields with their corresponding message number must be present in the top level request message. If on of them
+# is missing, the documentation will not generate and an exception will be thrown.
+# Fields are specified in the forma (field_name, field_number)
 REQUIRED_REQUEST_FIELDS = [
-    'id',
-    'dbms_version_request',
-    'connection_request',
-    'disconnect_request'
+    ('id', 1),
+    ('dbms_version_request', 2),
+    ('connection_request', 19),
+    ('disconnect_request', 21)
 ]
 
-# These fields must be present in the top level response message. If on of them is missing, the documentation will not
-# generate and an exception will be thrown.
+# These fields with their corresponding message number must be present in the top level request message. If on of them
+# is missing, the documentation will not generate and an exception will be thrown.
+# Fields are specified in the forma (field_name, field_number)
 REQUIRED_RESPONSE_FIELDS = [
-    'id',
-    'last',
-    'error_response',
-    'dbms_version_response',
-    'connection_response',
-    'disconnect_response'
+    ('id', 1),
+    ('last', 2),
+    ('error_response', 256),
+    ('dbms_version_response', 3),
+    ('connection_response', 12),
+    ('disconnect_response', 13)
 ]
 
 # The common suffix of all rpc request messages in the top level request message.
@@ -57,8 +59,7 @@
 DESCRIPTOR_SET_OUT = 'descriptor_set.bin'
 
 # The directory from which the imports in the .proto files are resolved.
-IMPORT_BASE_DIR = '../' #../org/polypheny/prism'
-
+IMPORT_BASE_DIR = '../'  # ../org/polypheny/prism'
 
 # ----------------------------------------------------------------------------------
 # DOCUMENTATION
@@ -68,4 +69,4 @@
 README_FILE = '../README.md'
 
 # The directory in which the generated documentation should be put. The directory is created if it does not exist.
-OUTPUT_DIR = '../doc'
+OUTPUT_DIR = '../doc'

doc-generator/enums.py

@@ -4,7 +4,8 @@
 import utils as utils
 import proto_utils as putils
 
-def get_enums_from_file_descriptor(file_descriptor, repo_path, tree_ish):
+
+def _get_enums_from_file_descriptor(file_descriptor, repo_path, tree_ish):
     content = []
     if not hasattr(file_descriptor, 'source_code_info'):
         print('ERROR: No source code info!')
@@ -48,6 +49,6 @@ def enums(repo_path, tree_ish):
         putils.compile_file(file_path)
         descriptor_set = putils.load_descriptor_set()
         for descriptor in descriptor_set.file:
-            content.extend(get_enums_from_file_descriptor(descriptor, repo_path, tree_ish))
+            content.extend(_get_enums_from_file_descriptor(descriptor, repo_path, tree_ish))
     putils.remove_descriptor_set()
     return ''.join(content)

doc-generator/files.py

@@ -11,10 +11,10 @@
 
 def proto_files(repo_path, branch_name, directory=config.PROTO_DIR):
     file_names = putils.get_proto_file_names(directory)
-    return generate_file_section(directory, file_names, repo_path, branch_name)
+    return _generate_file_section(directory, file_names, repo_path, branch_name)
 
 
-def generate_file_section(directory, file_names, repo_path, tree_ish):
+def _generate_file_section(directory, file_names, repo_path, tree_ish):
     processed = []
     content = []
     short_directory = directory.replace("../", "", 1)
@@ -25,7 +25,7 @@ def generate_file_section(directory, file_names, repo_path, tree_ish):
             responses_file = file.replace(REQUESTS_SUFFIX, RESPONSES_SUFFIX)
             if responses_file in file_names:
                 category = file.replace(REQUESTS_SUFFIX, '')
-                description = get_file_description(directory, file)
+                description = _get_file_description(directory, file)
                 file_link = utils.generate_link(repo_path, tree_ish, file, short_directory)
                 responses_file_link = utils.generate_link(repo_path, tree_ish, responses_file, short_directory)
                 content.append(gen.generate_paired_file_entry(category, file, responses_file, description, file_link, responses_file_link))
@@ -36,14 +36,14 @@ def generate_file_section(directory, file_names, repo_path, tree_ish):
                 raise FileExistsError(f"No matching responses file found for {file}.")
         elif not file.endswith(REQUESTS_SUFFIX) and not file.endswith(RESPONSES_SUFFIX):
             category = file.replace('.proto', '')
-            description = get_file_description(directory, file)
+            description = _get_file_description(directory, file)
             file_link = utils.generate_link(repo_path, tree_ish, file, short_directory)
             content.append(gen.generate_single_file_entry(category, file, description, file_link))
             processed.append(file)
     return ''.join(content)
 
 
-def get_file_description(directory, file_name):
+def _get_file_description(directory, file_name):
     path = os.path.join(directory, file_name)
     try:
         with open(path, 'r') as file:

doc-generator/generator.py

@@ -165,18 +165,18 @@ def generate_single_file_entry(category, file, description, file_link=None):
     return entry
 
 
-def create_directory(directory):
+def _create_directory(directory):
     if not os.path.exists(directory):
         os.makedirs(directory)
 
 
-def write_to_file(file, text):
+def _write_to_file(file, text):
     with open(file, 'w') as file:
         file.write(text)
 
 
 def main(repo_path, tree_ish):
-    create_directory(config.OUTPUT_DIR)
+    _create_directory(config.OUTPUT_DIR)
 
     tasks = [
         generate_overview_doc,
@@ -186,7 +186,7 @@ def main(repo_path, tree_ish):
     for task in tasks:
         file, text = task(repo_path, tree_ish)
         path = os.path.join(config.OUTPUT_DIR, file)
-        write_to_file(path, text)
+        _write_to_file(path, text)
 
 
 if __name__ == '__main__':

doc-generator/messages.py

@@ -8,7 +8,7 @@
 import utils as utils
 
 
-def get_messages_from_file_descriptor(file_descriptor, repo_path, tree_ish):
+def _get_messages_from_file_descriptor(file_descriptor, repo_path, tree_ish):
     content = []
     if not hasattr(file_descriptor, 'source_code_info'):
         print('ERROR: No source code info!')
@@ -46,6 +46,6 @@ def messages(repo_path, branch_name):
         putils.compile_file(file_path)
         descriptor_set = putils.load_descriptor_set()
         for descriptor in descriptor_set.file:
-            content.extend(get_messages_from_file_descriptor(descriptor, repo_path, branch_name))
+            content.extend(_get_messages_from_file_descriptor(descriptor, repo_path, branch_name))
     putils.remove_descriptor_set()
     return ''.join(content)

doc-generator/proto_utils.py

@@ -1,9 +1,9 @@
 import os
 import subprocess
-import config
-from google.protobuf import descriptor_pb2
 
+from google.protobuf import descriptor_pb2
 
+import config
 
 
 def get_proto_file_names(directory):
@@ -48,7 +48,7 @@ def get_description_for_location(source_code_info, path):
     return ''
 
 
-def get_source_from_span(span, file_path):
+def _get_source_from_span(span, file_path):
     start_line, start_column, length_in_chars = span
     length_in_chars -= 2
     end_position = start_column + length_in_chars
@@ -61,7 +61,7 @@ def get_source_from_span(span, file_path):
         if i < start_line:
             continue
         if i == start_line + 1:
-            line = line[start_column:start_column+length_in_chars]
+            line = line[start_column:start_column + length_in_chars]
         if current_char_count + len(line) >= end_position:
             segment += line[:end_position - current_char_count]
             break
@@ -75,7 +75,7 @@ def get_field_type(source_code_info, location_path, file_path):
     location_path.append(4)
     for location in source_code_info.location:
         if list(location.path) == location_path:
-            return get_source_from_span(location.span, file_path)
+            return _get_source_from_span(location.span, file_path)
     return ''
 
 

doc-generator/rpc.py

@@ -6,9 +6,9 @@
 import os
 import config
 
-
 RPC_DEFINITION_FILE = os.path.join(config.PROTO_DIR, config.RPC_DEFINITION_FILE)
 
+
 def get_rpcs_from_sets(descriptor, matches, single_responses, single_requests, repo_path, tree_ish, request_path,
                        response_path):
     content = []
@@ -22,13 +22,15 @@ def get_rpcs_from_sets(descriptor, matches, single_responses, single_requests, r
 
         request_type = request[1].type_name.split('.')[-1]
         line = putils.get_line_number_for_location(descriptor.source_code_info, message_path)
-        request_link = utils.generate_link(repo_path, tree_ish, os.path.basename(RPC_DEFINITION_FILE), short_directory, line)
+        request_link = utils.generate_link(repo_path, tree_ish, os.path.basename(RPC_DEFINITION_FILE), short_directory,
+                                           line)
         content.append(gen.generate_rpc_message(request_type, request[1].name, True, request_link))
 
         response_type = response[1].type_name.split('.')[-1]
         message_path = [4, response_path, 2, response[0]]
         line = putils.get_line_number_for_location(descriptor.source_code_info, message_path)
-        response_link = utils.generate_link(repo_path, tree_ish, os.path.basename(RPC_DEFINITION_FILE), short_directory, line)
+        response_link = utils.generate_link(repo_path, tree_ish, os.path.basename(RPC_DEFINITION_FILE), short_directory,
+                                            line)
         content.append(gen.generate_rpc_message(response_type, response[1].name, False, response_link))
 
     for single_request in single_requests:
@@ -39,7 +41,8 @@ def get_rpcs_from_sets(descriptor, matches, single_responses, single_requests, r
 
         request_type = single_request[1].type_name.split('.')[-1]
         line = putils.get_line_number_for_location(descriptor.source_code_info, message_path)
-        request_link = utils.generate_link(repo_path, tree_ish, os.path.basename(RPC_DEFINITION_FILE), short_directory, line)
+        request_link = utils.generate_link(repo_path, tree_ish, os.path.basename(RPC_DEFINITION_FILE), short_directory,
+                                           line)
         content.append(gen.generate_rpc_message(request_type, single_request[1].name, False, request_link))
 
     for single_response in single_responses:
@@ -50,7 +53,8 @@ def get_rpcs_from_sets(descriptor, matches, single_responses, single_requests, r
 
         request_type = single_response[1].type_name.split('.')[-1]
         line = putils.get_line_number_for_location(descriptor.source_code_info, message_path)
-        request_link = utils.generate_link(repo_path, tree_ish, os.path.basename(RPC_DEFINITION_FILE), short_directory, line)
+        request_link = utils.generate_link(repo_path, tree_ish, os.path.basename(RPC_DEFINITION_FILE), short_directory,
+                                           line)
         content.append(gen.generate_rpc_message(request_type, single_response[1].name, True, request_link))
 
     return content
@@ -62,17 +66,17 @@ def rpc(repo_path, tree_ish):
     descriptor_set = putils.load_descriptor_set()
     for descriptor in descriptor_set.file:
         check_required_fields(descriptor)
-        matches, single_responses, single_requests = get_rpc_sets_from_file_descriptor(descriptor)
-        requests_path, _ = get_message_descriptor(descriptor, config.REQUESTS_MESSAGE_NAME)
-        responses_path, _ = get_message_descriptor(descriptor, config.RESPONSES_MESSAGE_NAME)
+        matches, single_responses, single_requests = _get_rpc_sets_from_file_descriptor(descriptor)
+        requests_path, _ = _get_message_descriptor(descriptor, config.REQUESTS_MESSAGE_NAME)
+        responses_path, _ = _get_message_descriptor(descriptor, config.RESPONSES_MESSAGE_NAME)
         content.extend(get_rpcs_from_sets(descriptor, matches, single_responses, single_requests, repo_path, tree_ish,
                                           requests_path, responses_path))
     return ''.join(content)
 
 
-def get_message_field_descriptors(file_descriptor, message_name):
+def _get_message_field_descriptors(file_descriptor, message_name):
     message_descriptors = []
-    _, message_descriptor = get_message_descriptor(file_descriptor, message_name)
+    _, message_descriptor = _get_message_descriptor(file_descriptor, message_name)
     if not message_descriptor:
         raise Exception(f'No message with name "{message_name}" found.')
     for field_id, field in enumerate(message_descriptor.field):
@@ -82,29 +86,29 @@ def get_message_field_descriptors(file_descriptor, message_name):
     return message_descriptors
 
 
-def get_message_descriptor(file_descriptor, message_name):
+def _get_message_descriptor(file_descriptor, message_name):
     for message_index, message in enumerate(file_descriptor.message_type):
         if message.name == message_name:
             return message_index, message
     return None
 
 
-def is_request_present(file_descriptor):
-    return get_message_descriptor(file_descriptor, config.REQUESTS_MESSAGE_NAME) is not None
+def _is_request_present(file_descriptor):
+    return _get_message_descriptor(file_descriptor, config.REQUESTS_MESSAGE_NAME) is not None
 
 
-def is_response_resent(file_descriptor):
-    return get_message_descriptor(file_descriptor, config.RESPONSES_MESSAGE_NAME) is not None
+def _is_response_resent(file_descriptor):
+    return _get_message_descriptor(file_descriptor, config.RESPONSES_MESSAGE_NAME) is not None
 
 
-def get_rpc_sets_from_file_descriptor(file_descriptor):
-    if not is_request_present(file_descriptor):
+def _get_rpc_sets_from_file_descriptor(file_descriptor):
+    if not _is_request_present(file_descriptor):
         raise Exception('Request message not present.')
-    if not is_response_resent(file_descriptor):
+    if not _is_response_resent(file_descriptor):
         raise Exception('Response message not present.')
-    request_messages = get_message_field_descriptors(file_descriptor, config.REQUESTS_MESSAGE_NAME)
-    response_messages = get_message_field_descriptors(file_descriptor, config.RESPONSES_MESSAGE_NAME)
-    matches, leftover_requests, leftover_responses = get_matches(request_messages, response_messages)
+    request_messages = _get_message_field_descriptors(file_descriptor, config.REQUESTS_MESSAGE_NAME)
+    response_messages = _get_message_field_descriptors(file_descriptor, config.RESPONSES_MESSAGE_NAME)
+    matches, leftover_requests, leftover_responses = _get_matches(request_messages, response_messages)
     for left_request in leftover_requests:
         if left_request[1].name.endswith(config.REQUESTS_MESSAGE_NAME):
             raise Exception(f'No matching response for {left_request.name}')
@@ -114,7 +118,7 @@ def get_rpc_sets_from_file_descriptor(file_descriptor):
     return matches, leftover_requests, leftover_responses
 
 
-def get_matches(requests, responses):
+def _get_matches(requests, responses):
     matches = []
     leftover_requests = []
     leftover_responses = responses.copy()
@@ -134,16 +138,18 @@ def get_matches(requests, responses):
 
 
 def check_required_fields(file_descriptor):
-    _, request_descriptor = get_message_descriptor(file_descriptor, config.REQUESTS_MESSAGE_NAME)
-    request_fields = [field.name for field in request_descriptor.field]
-    for required_field in config.REQUIRED_REQUEST_FIELDS:
-        if required_field in request_fields:
-            continue
-        raise Exception(f'Required field "{required_field}" not present in request message.')
+    _, request_descriptor = _get_message_descriptor(file_descriptor, config.REQUESTS_MESSAGE_NAME)
+    request_fields = {field.name: field.number for field in request_descriptor.field}
+    _check_for_missing_fields(request_fields, config.REQUIRED_REQUEST_FIELDS)
 
-    _, response_descriptor = get_message_descriptor(file_descriptor, config.RESPONSES_MESSAGE_NAME)
-    response_fields = [field.name for field in response_descriptor.field]
-    for required_field in config.REQUIRED_RESPONSE_FIELDS:
-        if required_field in response_fields:
-            continue
-        raise Exception(f'Required field "{required_field}" not present in response message.')
+    _, response_descriptor = _get_message_descriptor(file_descriptor, config.RESPONSES_MESSAGE_NAME)
+    response_fields = {field.name: field.number for field in response_descriptor.field}
+    _check_for_missing_fields(response_fields, config.REQUIRED_RESPONSE_FIELDS)
+
+
+def _check_for_missing_fields(existing_fields, required_fields):
+    for required_field_name, required_field_number in required_fields:
+        if required_field_name not in existing_fields:
+            raise Exception(f'Required field "{required_field_name}" not present in response message.')
+        if existing_fields[required_field_name] != required_field_number:
+            raise Exception(f'Required field "{required_field_name}" in response message has incorrect number. Expected {required_field_number}, got {existing_fields[required_field_name]}.')