final-hill / cathedral

Requirements Engineering
GNU Affero General Public License v3.0
0 stars 0 forks source link

Refactor Versioning, Mapping, Repositories #56

Closed mlhaufe closed 6 months ago

mlhaufe commented 6 months ago

Currently all the mappers have the following form:

export interface FooJson extends BaseJson {
    ...props
}

export default class FooToJsonMapper extends BaseToJsonMapper {
    override mapFrom(target: FooJson): Foo {
        const version = target.serializationVersion ?? '{undefined}';

        if (version.startsWith('0.3.'))
            return new Foo(target);
        //... additional versions

        throw new Error(`Unsupported serialization version: ${version}`);
    }

    override mapTo(source: Foo): FooJson {
        return {
            ...super.mapTo(source),
            // ..additionalProps
        };
    }
}

Most repositories are of the form:

export default class FooRepository extends BaseRepository<Foo> {
    constructor() { super('foo', new FooToJsonMapper(pkg.version)); }
}

There is also a LocalStorageRepository specialized for the storage type:

export class LocalStorageRepository<E extends Entity> extends Repository<E> {
    constructor(readonly storageKey: string, mapper: Mapper<E, EntityJson>) {
        super(mapper);
    }

    get storage(): Storage {
        return localStorage;
    }

    get(id: E['id']): Promise<E | undefined> { ... }

    getAll(filter: (entity: E) => boolean = _ => true): Promise<E[]> { ... }

    add(item: E): Promise<void> { ... }

    clear(): Promise<void> { ... }

    update(item: E): Promise<void> { ... }

    delete(id: E['id']): Promise<void> { ... }
}

Note the storage member which was defined to support unit testing:

const fakeStorage: Storage = new DomStorage(null, { strict: true });

class TestLocalStorageRepository<F extends Foo> extends LocalStorageRepository<F> {
    override get storage() {
        return fakeStorage;
    }
}

This smells and has significant duplication. This needs to be done better