Made cfbs add reject invalid modules

cfbs/cfbs_config.py

@@ -1,4 +1,19 @@
 #!/usr/bin/env python3
+"""cfbs_config.py - Logic for manipulating a project / cfbs.json config file.
+
+TODOs:
+- A lot of the code which is currently inside some long commands in commands.py
+  should be moved here.
+- CFBSConfig.add() logic needs a good refactoring. It has a lot more code than
+  necessary duplicated for slightly different cases (URL vs index vs provides
+  vs dependencies). It should be rewritten / unified, and split up into a few
+  discrete steps:
+  1. Collect modules to add (and filter)
+  2. Validate modules and abort if the new modules fail validation
+  3. Add modules and make commits
+"""
+
+
 import os
 import copy
 import glob
@@ -23,6 +38,7 @@
 from cfbs.cfbs_json import CFBSJson
 from cfbs.module import Module, is_module_added_manually
 from cfbs.prompts import prompt_user, YES_NO_CHOICES
+from cfbs.validate import validate_single_module
 
 
 # Legacy; do not use. Use the 'Result' namedtuple instead.
@@ -333,6 +349,11 @@ def _add_without_dependencies(self, modules):
                 del module["subdirectory"]
             if self.index.custom_index is not None:
                 module["index"] = self.index.custom_index
+            # TODO: This validation could probably be done in a better place,
+            #       after we refactor the add logic:
+            validate_single_module(
+                context="build", name=name, module=module, config=None, local_check=True
+            )
             self["build"].append(module)
             self._handle_local_module(module)
 

cfbs/cfbs_json.py

@@ -1,3 +1,15 @@
+"""cfbs_json.py - Simple read-only abstraction of a cfbs.json file
+
+Don't put code which makes changes to cfbs.json here, that should
+go in cfbs_config.py.
+
+TODOs:
+1. Code related to validation should be moved to validate.py.
+   For example warn_about_unknown_keys(). Validation code
+   should just take "raw data" (JSON dicts) and not be coupled
+   with cfbs_json.py nor cfbs_config.py.
+"""
+
 from collections import OrderedDict
 from copy import deepcopy
 import logging as log

cfbs/commands.py

@@ -1,6 +1,45 @@
 """
+commands.py - Entry points for each cfbs command.
+
+Each cfbs command has a corresponding function named <command>_command.
 Functions ending in "_command" are dynamically included in the list of commands
 in main.py for -h/--help/help.
+
+At a high level, each command should do something like this:
+1. Open the cfbs.json file using CFBSConfig.get_instance().
+2. Validate arguments and cfbs.json data before we start editing.
+3. Perform the necessary operations using the CFBSConfig methods.
+4. Validate cfbs.json again after editing.
+5. Save changes and make commits.
+6. Return an appropriate exit code.
+   0 for success, 1 for failure.
+   Raising exceptions is also an option and often preferred, see main.py for exception handling code.
+
+Note that most of the business logic (step 2) should be implemented in CFBSConfig.
+
+The command function (in this file) should have parameters which
+map closely to the command line arguments.
+
+For example, cfbs status does not take any arguments, so the signature should be:
+
+def status_command() -> int:
+
+On the other hand, cfbs search takes a list of search terms:
+
+cfbs search <terms>
+
+So, the appropriate signature is:
+
+def search_command(terms: List[str]) -> int:
+
+Todos:
+1. Some of these functions are getting too long, business logic should be moved into
+   CFBSConfig in cfbs_config.py. Commands should generally not call other commands,
+   instead they should call the correct CFBSConfig method(s).
+2. Decorators, like the git ones, should not change the parameters nor types of
+   the functions they decorate. This makes them much harder to read, and
+   type checkers don't understand it either. It applies to both types and values of
+   parameters and returns.
 """
 
 import os

cfbs/validate.py

@@ -17,6 +17,17 @@
 Thus, for example, the common parts needed by both build.py
 and validate.py, should be in utils.py or validate.py,
 not in build.py.
+
+TODOs:
+1. Although we don't import anything from cfbs_config.py
+   nor cfbs_json.py, we still depend on them by calling
+   certain methods on the config object, such as
+   config.warn_about_unknown_keys() and config.find_module().
+   We should decouple this so that validate.py is more pure
+   and can be called and tested with just simple JSON data
+   / dicts. This should make it easer to write unit tests,
+   use stricter type checking, and maintain all the validation
+   code in one place.
 """
 
 import logging as log