Require top-level config values while allowing to override config file

[jescalada]

Currently, top-level config values are not required, which means that the generated config types are optional. This forces us to use fallbacks, check if values are present or cast them:

app.use(
  session({
    store: db.getSessionStore(),
    secret: config.getCookieSecret() as string,
    resave: false,
    saveUninitialized: false,
    cookie: {
      secure: 'auto',
      httpOnly: true,
      maxAge: (config.getSessionMaxAgeHours() || DEFAULT_SESSION_MAX_AGE_HOURS) * 60 * 60 * 1000,
    },
  }),
);

Describe the solution you'd like
Adding the parameters to the "required" list will break tests that override the config entirely since it errors out if the entries aren't present. The best solution seems to be to post-process the generated config types so that the top-level properties are wrapped in Required types. This allows stronger typing as well as overriding.

We should also consider how we can set default values for the config - see the related discussion: #1202 (comment)

Additional context
#1202 (comment)
#1202 (comment)

[kriswest]

I believe we can use the Required<subtype> utility TypeScript type to give us type safety without post processing the generated types. That type simply makes all the immediate properties of the parent type required. Were a function to return Required<GitProxyConfig> then it would need to check those properties are set before returning (throw an error if not all are set), but then everything downstream of that function can safely assume those top-level properties are set (allowing us to remove checks or overrides (e.g. !).

We've done actual post processing of generated types (i.e. modifying and re-saving the generated TS types with ts-morph) in the FDC3 project, but it brings in more complexity and something else that can break. Just using the Required utility type to wrap the generated one doesn't require actual post-processing, making life much easier!

I think we're in agreement on this - I just wanted to clarify 'post-processing'.

We should also consider how we can set default values for the config - see the related discussion: #1202 (comment)

We currently do this by merging the default config file with the user supplied overrides (by calling mergeConfigurations() - defaults do NOT come from the schema - it takes work to pull defaults from a schema, but it is possible). We do this in src\config\index.ts loadFullConfiguration(), it is then used in the accessor functions for the various parts of the config. We can change the return type of that function to Required<GitProxyConfig> and run checks there that we have set all default values. Afterwards we can reduce accessors like:

// Get commit related configuration
export const getCommitConfig = () => {
  const config = loadFullConfiguration();
  return config.commitConfig || {};
};

to

// Get commit related configuration
export const getCommitConfig = () => {
  return loadFullConfiguration().commitConfig;
};

or ditch them as unnecessary (they were really just correcting the optional type with a false default). There are lots of other places with overrides (e.g. !) that can also be corrected thereafter.

Personally, I'm not hugely bothered where we set defaults - as long as its one place and easily found! We currently have several places where its happening (e.g. both the default config and the accessor functions above, probably also where config is used in some cases) - and as long as we are testing the defaults (are present/work as expected).

I do not know interactions with the configLoader well yet - but do note that reloadConfiguration does call loadFullConfiguration.