too many service comparisons

[ryanfkeepers]

Issue

One of the results of the repo's organic growth up to this point is the proliferation of service-specific switches and conditionals. While Corso only covered two services, this was largely a non issue. After three services we ignored the problem. Now in the process of adding Groups and Teams we're seeing just how many places in the code we need to acknowledge the existence of the service in order for the support to behave as expected.

This is a death-by-1000-papercuts maintenance issue. It requires multiple tiny changes to implement high level concepts, increases cognitive load and code familiarity requirements, and will scale poorly with future feature additions. Further, this problem creates a barrier for code contribution: the domain knowledge required for large-scale additions already races against contributor enthusiasm. Adding a hundred points of minutia to update will antagonize the experience.

The various implementations of this issue appear as three basic design smells:

1. Domain-isolated data containers
2. Reactive handling to generic structures
3. One-off service conditionals

Across the solution suggestions for these three issues you'll see a couple common design principles:

• For behavior and categorization, interfaces should be prioritized to ensure service implementations can self-describe their state.
• For actual data, generic data structures should be prioritized over Getter interfaces.
• Packages with clear knowledge and ownership of the utilization of common structures should also own the transformations required by common endpoints.
• Proactive is better than reactive. New implementation should comply with expected, identified conditions to minimize the risk of missing a needed implementation.

Domain-isolated data containers

Example: Details ItemInfo

Domain isolation occurs when we juggle multiple structs to identify expected metadata, as opposed to a generic data structure (such as a map) and/or a single interface value.

In the example above, data is split acros five different DomainInfo entries (four for services, one for folders). In order to utilize an ItemInfo struct, all code must first check which of these structs is actually populated (if any), either by comparing each value or by utilizing some external identifier such as path service type. In addition, many of these types express overlapping data (ex: four of the five contain DriveID and DriveName values). The result of this is not only poor scaling and code duplication, but also potential gotchas around constraints like having multiple populated types or seeing different in property keys between structs for similar values.

An ideal solution would reduce the ItemInfo to a single DomainInfo interface. The interface could describe common data categorization checks (WhatCategory, WhatService, IsFolder, IsDriveItem, etc), and should be able to produce a common container of data with well-known property keys (ie: either map or single, standardized struct).

Reactive handling to generic structures

Example: Paths Transformers

Reactive handling occurs when transformations that could be owned by the caller get passed downstream into the generic function instead.

This issue can be fairly nuanced as there's a fine line between elegant generic code and this smell. The key issue here occurs when the caller is an opinionated package with clear knowledge about how it utilizes a generic structure, but still passes off those utilization details onto the centralized function. This punt distributes design ownership between the two packages, and requires maintainers to remember to update both pacakges for a successful change.

By identifying the places where opinionated packages expect generic functions to understand their design, we can pull those transformations back into the owning package to centralize that design specification within a single location. Future implementations should naturally recognize when they're being asked to own these transformations by the nature of the func parameters and comments. This priority also ensures a minimum of churn within generic packages, and thus the smallest likelihood of unintentional changes to other packages that share the code space.

One-off service conditionals

Example: m365 backup

One offs are the most insidious of the issues, and also the most difficult to tackle. By their nature, there's no large-scale solution. We've allowed them to proliferate because each occurrence seems benign at the time of its implementation. It's only now that we've gone back to recount all of the places we have to update a one-off (two-off, I suppose?) that the burden of maintenance becomes clear.

The best solution here is a change in accepted code standards: prevent the adoption of one-offs in the first place. It's always a case-by-case basis. Every case will be unique, and the contributor will need to ask themselves, "how do we avoid a conditional or switch here?". Any effort we put in up-front to avoid a one-off will ensure more stable code with less maintenance and contribution whiplash in the future.

Required Changes

These problems are too large and too distributed to address in a single triage. Changes will need to be divided into smaller chunks, each with their own ticket. After each one, we can revisit this parent issue to check whether the changes are sufficient to close it out.