Formatting Refactor Discussion

[Waidhoferj]

Objective

Reduce the scope of the database wrapper to interacting with the database.

Proposal

Require that all Entities have a validator and formatter class attached. By updating this class, we can make our error checking extensible and move most of the logic out of the database_wrapper.py file.

We have a top level initialization static method on each class called from_dict(data). All of the validation/formatting code in the database_wrapper could be reduced to the following:

try:
   entity = entity_type.from_dict(data)
except:
  #Handle errors
#Perform insert/update/delete

Internally this method would incorporate the following:

 validator = EntityTypeValidator()
formatter = EntityTypeFormatter()
  validator.validate(data) #Checks data for critical errors, raises exceptions handled above.
 formatted_data = formatter.format(data) #Casts data to correct type and fills in missing replaceable fields.
entity = EnitityType()
for key in formatted_data:
    setattr(entity, key, formatted_data[key])
return entity

Examples of validators and formatters can be found the the ./modules folder.

Benefits

• Upholds single responsibility principle: database_wrapper is focused on interacting with database. Formatters format. Validators validate
• Extensible: allows formatting and validating to be abstracted from the wrapper, so adding new features doesn't mean pushing breaking changes.
• Modularized: Allows code to be divided into different files, making the codebase more readable.
• More performant? We're loading k/v pairs for specific use cases every time we init the server. If this were abstracted, we wouldn't have to load overhead for functionalities outside of the scope of a specific REST API endpoint.

Details

Currently the database has formatting and validation functionality baked into the wrapper. The code rigidly enforces that each Entity adheres to the keys defined in EXPECTED_KEYS_BY_ENTITY, but there are cases for the Nimbus Validator App where an ID needs to be passed instead of automatically generated. If I were to change the code in the validate_and_format_entity_data() function, there would likely be other breaking changes.

The proposed format would allow me to edit the QuestionAnswerPair formatter in isolation in order to field this new use case.

[Waidhoferj]

What do you all think about this new format?

[Jason-Ku]

In all seriousness, putting the validation/formatting burden on each Entity itself seems like a MUCH smarter move than trying to externally enforce it. Having an Entity interface that defines a validate and format method to call validators/formatters sounds like a really really smart move to me

[Waidhoferj]

It appears that SQLAlchemy doesn't make use of the constructor when it does its magic, so we can just use the entity constructor instead of a static from_dict() call. Gonna use this implementation in my next PR