Dal Prototipo alla Produzione: Refactoring delle tue API Interne

Intro

Questo post è stato originariamente scritto per il mio blog refactormycode.dev

Molti progetti interni iniziano con API costruite in fretta che possono diventare difficili da gestire nel tempo. Questo articolo fornisce consigli pratici sul refactoring di queste API “prototipo”, trasformandole in soluzioni stabili ed efficienti, adatte ad ambienti di produzione.

Avviso: Tutti gli esempi di codice forniti sono stati rielaborati per brevità

Come è iniziato il disastro

Qualche anno fa, lavoravo per una piccola azienda italiana nel settore della formazione per lo sviluppo professionale. Il codice in questo articolo proviene da un’applicazione web interna necessaria per archiviare e organizzare qualsiasi corrispondenza inviata e ricevuta che potesse essere assegnata a un progetto esistente. Questo includeva preventivi, contratti, fatture, ricevute di pagamento, ecc.

Quando è arrivata la richiesta era proprio quella che hai letto sopra, piuttosto semplice, no?

Tutte le risorse interne erano già sature con altri progetti dei clienti, una situazione molto comune nelle piccole aziende, e questo progetto aveva una priorità più bassa. Quindi si decise di commissionarlo a un team offshore, almeno per la fase iniziale, per poi passarlo al mio team.

La mia responsabilità era mettere insieme un documento di specifiche per un MVP (Prodotto Minimo Funzionante), trovare collaboratori esterni ed eseguire un po’ di project management. Non ero coinvolto nella programmazione o nelle revisioni. Anche se non avevo molto tempo dedicato a questo progetto, ho cercato di essere diligente nel raccogliere i requisiti, nel parlare con il team amministrativo per capire le loro reali esigenze, e anche nel trovare il team offshore giusto con cui lavorare.

Un approccio ingenuo

Insieme al team esterno, abbiamo deciso di usare Firebase, che sembrava adattarsi perfettamente al nostro caso d’uso:

• Piattaforma serverless, quindi nessun sovraccarico devops
• Facile da distribuire
• Integrazione fluida di un front end Angular con Google Cloud Function per il nostro backend API

Dopo il meeting iniziale con il team lead, tutti i requisiti sembravano chiari ed erano pronti per iniziare.

Non fui coinvolto in nessuna revisione del codice poiché loro avevano “apparentemente” un team completo al lavoro, incluso un team lead senior, così mi coordinavo con lui ogni paio di giorni o quando servivano chiarimenti. Sembravano professionali, quindi mi fidai.

La grande palla di fango

Dopo 4-5 settimane, quando la prima versione beta fu pronta per una demo interna, diedi un’occhiata al codice ed era già molto… “destrutturato”, eccone un esempio:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72

export async function create(req: Request, res: Response) {
	try {
		let document:any = null
		const valid = await documentSchema.noUnknown(true).isValid(req.body);
		if (!valid) return res.status(400).send(await documentSchema.validate(req.body));
		const validDate = await documentSchema.noUnknown(true).validate(req.body);
		document = validDate

        if([3,6].includes(document.docType) && document.sourceType === 2){
            return res.status(400).send({  message: 'Invalid - Document Type' })
        }

        if([1,2,3,4,6].includes(document.docType) && !document.expDate){
            return res.status(400).send({  message: 'undefined - expDate is Required' })
        }

        if(document.docType !== 4){
            document.docRef = undefined;
        }

        if(document.docRef) {
            const doc = (await db.collection("documents")
                .doc(document.docRef).get()).data();
            if(!doc) {
                return res.status(403).send({
                    message:"undefiend - docRef points to an invalid document"
                })
            }
        }

        if(document.projectRef){
            const project = (await db.collection("projects")
                .doc(`${document.projectRef}`).get()).data();
            if(!project) {
                return res.status(403).send({
                    message:"undefiend - projectRef points to an invalid project"
                })
            }
        }

        //Condizioni basate su sourceType
        if(document.sourceType === 1 && !document.sender){
            return res.status(400).send({ message: 'undefined - sender is required' })
        }

        //Condizioni basate su sourceType
        if(document.sourceType === 2 && !document.recipients){
            return res.status(400).send({ message: 'undefined - recipients is required' })
        }

        if(document.sourceType === 1 && document.recipients){
            document.recipients = undefined;
        }

        if(document.sourceType === 2 && document.sender){
            document.sender = undefined;
        }

        if(![1,2,3,4,6].includes(document.docType) && document.expDate){
            document.expDate = undefined;
        }

        [...codice omesso qui]

        const docRef = await db.collection('documents').doc(code).set(document)


		return res.status(201).send({ id: docRef.id })
	} catch (err) {
		return handleError(res, err)
	}
}

Si supponeva che questo fosse un controller per l’API nodejs, ma ha finito per essere un “contenitore per tutto”: gestisce la convalida dei dati, la logica di business, l’accesso al database e molto altro. Il resto dell’API seguiva lo stesso pattern.

Questo codice non è facile da testare (ammesso che sia possibile), il che lo renderà anche molto difficile da mantenere e fare debug.

Scomponiamo i problemi

Innanzitutto, se leggi il codice sopra, riesci a capirne la logica di business? Non molta, vero?

Credo che ogni volta che un nuovo sviluppatore si unisce al team, dovrebbe essere in grado di capire almeno lo scopo principale di una determinata porzione di codice. Esaminiamo quello che ritengo sia l’approccio corretto per un refactoring graduale che porti anche valore aziendale, in modo che il tuo manager/capo non si arrabbi.

1. Rendi il codice più leggibile
2. Separa gradualmente il codice definendo le “responsabilità”
3. Scrivi i test

1. Rendi il codice più leggibile

Il primo problema che vedo è non usare alcuna costante per document.docType o document.sourceType

1
2
3

if([3,6].includes(document.docType) && document.sourceType === 2){
    return res.status(400).send({  message: 'Invalid - Document Type' })
}

Che tipi di docType sono 3 e 6 e perché non possono avere un sourceType 2? Proviamo a sistemarlo mappando docType e sourceType e usando stringhe predefinite per gli errori di validazione:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18

export enum DocumentType {
    INVOICE = 1,
    RECEIPT = 2,
    SURETY = 3,
    PAYMENT = 4,
    COMMUNICATION = 5,
    PAYMENT_NOTICE = 6
}

export enum DocumentSource {
    INCOMING = 1,
    OUTGOING = 2
}

const ValidationErrors = {
    INVALID_DOCUMENT_TYPE: 'Invalid document type',
    [...aggiungi qui tutti gli errori di validazione]
};

quindi il codice precedente diventa:

1
2
3
4

if ([DocumentType.SURETY, DocumentType.PAYMENT_NOTICE].includes(document.docType) &&
    document.sourceType === DocumentSource.OUTGOING) {
    return res.status(400).send({ message: ValidationErrors.INVALID_DOCUMENT_TYPE });
}

ora dovrebbe essere più chiaro che un documento OUTGOING (In uscita) non può essere un SURETY (Fideiussione) o un PAYMENT_NOTICE (Avviso di pagamento). Inoltre, l’aggiunta di commenti è sempre una buona pratica, soprattutto in situazioni come questa in cui la logica di business è sparsa per il codice.

2. Separa gradualmente il codice definendo le “responsabilità”

Un controller dovrebbe contenere logica di business o codice di accesso al database? Hmm… Non credo.

Sono un grande fan della clean architecture (architettura pulita) e cerco di usarla ogni volta che inizio un nuovo progetto di media o grande complessità.

In un refactoring come questo, però, non è pratico semplicemente buttare via il codice esistente per ricostruire l’intera base di codice usando l’architettura “ideale”.
Stiamo cercando di portare valore aziendale e migliorare il codice allo stesso tempo, senza farci sgridare dal nostro manager, ricordi?

Quindi per questa fase iniziale di refactoring, credo che il codice dovrebbe essere strutturato così:

• Controller:
  • elaborano le richieste in arrivo ed eseguono la convalida dei dati di base (nessuna logica di business)
  • chiamano i servizi per validare la logica di business
  • formattano la risposta
• Servizi:
  • validano la logica aziendale
  • chiamano i repository per le operazioni di database
• Repository:
  • lettura/scrittura sul database

Implementiamo questi cambiamenti nel nostro codice ora.

La prima parte del controller eseguiva solo alcune convalide di base sui dati usando yup e non lo cambieremo per ora. Abbiamo solo modificato alcuni nomi di variabili per renderli più significativi e indentato il codice affinché sia più leggibile.

Un miglioramento notevole è la creazione di un’interfaccia per la richiesta in ingresso: in questo modo noi (e chiunque altro lavorerà su questo progetto) sapremo esattamente quale struttura dati si aspetta questo endpoint.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34

export interface CreateDocumentRequest {
  organizationId: string;
  sourceType: DocumentSource;
  sender?: string;
  recipients?: Array<string>;
  docType: DocumentType;
  amount?: number;
  docRef: string;
  notes?: string;
  projectRef: string;
}

export async function create(req: Request, res: Response) {
	try {
        // convalida dei dati
		let parsedDocument: CreateDocumentRequest;
		const isValid = await documentSchema.noUnknown(true).isValid(req.body);
		if (!isValid)  {
            return res.status(400).send(await documentSchema.validate(req.body));
        }

		parsedDocument = await documentSchema.noUnknown(true).validate(req.body);

        // convalidiamo la logica di business e salviamo nel db
        const documentId = await documentService.create(parsedDocument)
        
        // formattazione della risposta
		return res.status(201).send({ id: documentId })

    } catch (err) {
        // tutti gli errori di convalida verranno intercettati qui
		return handleError(res, err)
	}
}

La logica di business è stata spostata in una classe separata chiamata DocumentService, che gestirà l’intera logica di business e chiamerà il repository per archiviare i nostri documenti nel database.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17

export class DocumentService {

  constructor(private documentRepository: DocumentRepository) {}

  async create(documentRequest: CreateDocumentRequest): Promise<string> {

       if ([DocumentType.SURETY, DocumentType.PAYMENT_NOTICE].includes(document.docType) &&
            document.sourceType === DocumentSource.OUTGOING) {
            throw ValidationErrors.INVALID_DOCUMENT_TYPE;
        }

       [...codice omesso qui]

       return this.documentRepository.save(document);

  }
}

L’ultimo pezzo consiste nell’archiviazione del documento nel database, ed è gestita dal DocumentRepository. Mi piace molto il pattern repository perché ti permette di scindere l’accesso ai dati dal resto del codice. In questo modo puoi simulare facilmente il livello del database nei tuoi test unitari.

Creiamo una semplice interfaccia in modo che il nostro servizio non sia strettamente accoppiato all’implementazione Firestore e possiamo facilmente passare a un altro database o farne un mock (simulazione) per i nostri test unitari.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15

export interface DocumentRepository {
    save(document: Document): Promise<string>;
}

export class DocumentFirestoreRepository implements DocumentRepository {
    constructor(private readonly firestore: admin.firestore.Firestore) {}
    
    async save(document: Document): Promise<string> {

        const newDocumentRef = await this.firestore.collection('documents').add(document);

        return newDocumentRef.id;
    }
}
		

Qual è il valore aziendale nel ristrutturare semplicemente il codice? Ci si potrebbe chiedere…

La risposta è semplice: un codice più pulito è più facile da testare. Questo ridurrà il numero di bug e anche il tempo di debug quando si verifica un difetto.

Di conseguenza, meno tempo di sviluppo viene speso per correggere i bug e più tempo per sviluppare nuove funzionalità che porteranno maggiori entrate alla tua azienda.

3. Scrivi i test

In realtà questa dovrebbe essere la prima cosa che un software developer professionale dovrebbe fare, se segui l’approccio TDD, come suggerito da Uncle Bob nel suo libro Clean Coder che ti consiglio caldamente.

Ma nessuno è perfetto e sappiamo tutti che molti team non seguono (o non possono seguire) questo approccio per vari motivi. Quindi per ora non siamo troppo romantici su questo e iniziamo a scrivere i test.

Penso che la prima cosa da testare in questo caso sarebbe la logica di business, quindi dobbiamo scrivere un test per la nostra classe DocumentService. Dal momento che ho omesso la maggior parte delle regole di business, creeremo un test per i 2 possibili fallimenti di validazione che abbiamo nel nostro servizio, oltre all’“happy path” (percorso felice) dove un documento viene creato con successo.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51

describe('Document', function () {
    describe('falls quando si cerca di creare un documento', function () {
        it('tipo di documento OUTGOING non valido PAYMENT_NOTICE', function () {
            const request: CreateDocumentRequest = {
                organizationId: '123456',
                sourceType: DocumentSource.INCOMING,
                sender: '1',
                docType: DocumentType.PAYMENT_NOTICE,
                amount: 100,
                docRef: '1',
                projectRef: '1'
            };

            assert.throws(() => documentService.create(request), ValidationErrors.INVALID_DOCUMENT_TYPE);
        });

        it('tipo di documento OUTGOING non valido SURETY', function () {
            const request: CreateDocumentRequest = {
                organizationId: '123456',
                sourceType: DocumentSource.INCOMING,
                sender: '1',
                docType: DocumentType.SURETY,
                amount: 100,
                docRef: '1',
                projectRef: '1'
            };

            assert.throws(() => documentService.create(request), ValidationErrors.INVALID_DOCUMENT_TYPE);
        });
    });

    describe('ha successo quando si cerca di creare un documento', function () {
        it('dovrebbe creare una fattura valida', function () {
            const request: CreateDocumentRequest = {
                organizationId: '123456',
                sourceType: DocumentSource.INCOMING,
                sender: '1',
                docType: DocumentType.INVOICE,
                amount: 100,
                docRef: '1',
                projectRef: '1'
            };

            const id = await documentService.create(request);

            expect(typeof id).to.equal('string');
            expect(id.length).not.to.equal(0);

        });
    });
});

Finito!

Beh… più o meno. Ci sono molti altri miglioramenti che possiamo apportare, come definire diverse entità per ogni tipo di documento con le loro proprietà e logiche, usare delle factory per generare la giusta entità basata sul suo tipo, per esempio.

Conclusione

In questo post, abbiamo preso una web app progettata non molto bene e l’abbiamo gradualmente migliorata per renderla più leggibile, manutenibile e testabile. Abbiamo cercato di utilizzare diversi principi di progettazione e applicarli in modo pratico per assicurarci che il tempo del refactoring producesse un buon ritorno di investimento per la tua azienda.

Link Importanti

• TDD
• Repository Pattern
• Clean Architecture
• Uncle Bob
• The Clean Coder