fix: address comment simplify and remove BaseCacheReader

libs/ui/dataviz/src/lib/data-table/data-table.component.ts

@@ -64,7 +64,7 @@ export class DataTableComponent implements OnInit, AfterViewInit, OnChanges {
   @Input() set dataset(value: BaseReader) {
     this.properties$.next(null)
     this.dataset_ = value
-    this.dataset_.load()
+    this.dataset_.load() // TODO: add cacheActive
     this.dataset_.properties.then((properties) =>
       this.properties$.next(properties.map((p) => p.name))
     )
@@ -136,7 +136,7 @@ export class DataTableComponent implements OnInit, AfterViewInit, OnChanges {
     )
     this.dataset_.select(...propsWithoutGeom)
     try {
-      await this.dataSource.showData(this.dataset_.read())
+      await this.dataSource.showData(this.dataset_.read()) // TODO: add cacheActive
       this.error = null
     } catch (error) {
       this.handleError(error as FetchError | Error)

libs/util/data-fetcher/src/lib/data-fetcher.ts

@@ -5,7 +5,6 @@ import { GeojsonReader } from './readers/geojson'
 import { ExcelReader } from './readers/excel'
 import { DataItem, DatasetHeaders, FetchError, SupportedType } from './model'
 import { inferDatasetType } from './utils'
-import { BaseReader } from './readers/base'
 import { GmlReader } from './readers/gml'
 import { WfsVersion } from '@camptocamp/ogc-client'
 import { WfsReader } from './readers/wfs'
@@ -19,40 +18,39 @@ export async function openDataset(
     wfsFeatureType?: string
   },
   cacheActive?: boolean
-): Promise<BaseReader> {
+): Promise<
+  CsvReader | JsonReader | GeojsonReader | ExcelReader | GmlReader | WfsReader
+> {
   const fileType = await inferDatasetType(url, typeHint)
-  let reader: BaseReader
+  let reader:
+    | CsvReader
+    | JsonReader
+    | GeojsonReader
+    | ExcelReader
+    | GmlReader
+    | WfsReader
   try {
     switch (fileType) {
       case 'csv':
-        reader = new CsvReader(url, cacheActive)
+        reader = new CsvReader(url)
         break
       case 'json':
-        reader = new JsonReader(url, cacheActive)
+        reader = new JsonReader(url)
         break
       case 'geojson':
-        reader = new GeojsonReader(url, cacheActive)
+        reader = new GeojsonReader(url)
         break
       case 'excel':
-        reader = new ExcelReader(url, cacheActive)
+        reader = new ExcelReader(url)
         break
       case 'gml':
-        reader = new GmlReader(
-          url,
-          options.namespace,
-          options.wfsVersion,
-          cacheActive
-        )
+        reader = new GmlReader(url, options.namespace, options.wfsVersion)
         break
       case 'wfs':
-        reader = await WfsReader.createReader(
-          url,
-          options.wfsFeatureType,
-          cacheActive
-        )
+        reader = await WfsReader.createReader(url, options.wfsFeatureType)
         break
     }
-    reader.load()
+    reader.load(cacheActive)
     return reader
   } catch (e: any) {
     throw FetchError.parsingFailed(e.message)

libs/util/data-fetcher/src/lib/readers/base-file.ts

@@ -2,22 +2,21 @@ import { BaseReader } from './base'
 import { DataItem, DatasetInfo, PropertyInfo } from '../model'
 import { getJsonDataItemsProxy, jsonToGeojsonFeature } from '../utils'
 import { generateSqlQuery } from '../sql-utils'
-import { BaseCacheReader } from './base-cache'
 
 type ParseResult = {
   items: DataItem[]
   properties: PropertyInfo[]
 }
 
-export class BaseFileReader extends BaseCacheReader {
+export class BaseFileReader extends BaseReader {
   private parseResult_: Promise<ParseResult>
 
-  protected getData(): Promise<ParseResult> {
+  protected getData(cacheActive: boolean): Promise<ParseResult> {
     throw new Error('not implemented')
   }
 
-  load() {
-    this.parseResult_ = this.getData()
+  load(cacheActive?: boolean) {
+    this.parseResult_ = this.getData(cacheActive)
   }
 
   get properties(): Promise<PropertyInfo[]> {

libs/util/data-fetcher/src/lib/readers/csv.spec.ts

@@ -382,11 +382,8 @@ describe('CSV parsing', () => {
           sendAsJson: false,
         }
       )
-      reader = new CsvReader(
-        'http://localfile/fixtures/rephytox.csv',
-        cacheActive
-      )
-      reader.load()
+      reader = new CsvReader('http://localfile/fixtures/rephytox.csv')
+      reader.load(cacheActive)
     })
     afterEach(() => {
       fetchMock.reset()

libs/util/data-fetcher/src/lib/readers/csv.ts

@@ -46,7 +46,7 @@ export function parseCsv(text: string): {
 }
 
 export class CsvReader extends BaseFileReader {
-  getData() {
-    return fetchDataAsText(this.url, this.cacheActive).then(parseCsv)
+  getData(cacheActive: boolean) {
+    return fetchDataAsText(this.url, cacheActive).then(parseCsv)
   }
 }

libs/util/data-fetcher/src/lib/readers/excel.spec.ts

@@ -4134,11 +4134,8 @@ describe('Excel parsing', () => {
           sendAsJson: false,
         }
       )
-      reader = new ExcelReader(
-        'http://localfile/fixtures/ENS_CG02.xls',
-        cacheActive
-      )
-      reader.load()
+      reader = new ExcelReader('http://localfile/fixtures/ENS_CG02.xls')
+      reader.load(cacheActive)
     })
     afterEach(() => {
       fetchMock.reset()

libs/util/data-fetcher/src/lib/readers/excel.ts

@@ -27,7 +27,7 @@ export function parseExcel(buffer: ArrayBuffer): Promise<{
 }
 
 export class ExcelReader extends BaseFileReader {
-  getData() {
-    return fetchDataAsArrayBuffer(this.url, this.cacheActive).then(parseExcel)
+  getData(cacheActive: boolean) {
+    return fetchDataAsArrayBuffer(this.url, cacheActive).then(parseExcel)
   }
 }

libs/util/data-fetcher/src/lib/readers/geojson.spec.ts

@@ -402,10 +402,9 @@ describe('geojson parsing', () => {
         }
       )
       reader = new GeojsonReader(
-        'http://localfile/fixtures/perimetre-des-epci-concernes-par-un-contrat-de-ville.geojson',
-        cacheActive
+        'http://localfile/fixtures/perimetre-des-epci-concernes-par-un-contrat-de-ville.geojson'
       )
-      reader.load()
+      reader.load(cacheActive)
     })
     afterEach(() => {
       fetchMock.reset()

libs/util/data-fetcher/src/lib/readers/geojson.ts

@@ -23,7 +23,7 @@ export function parseGeojson(text: string): {
 }
 
 export class GeojsonReader extends BaseFileReader {
-  getData() {
-    return fetchDataAsText(this.url, this.cacheActive).then(parseGeojson)
+  getData(cacheActive: boolean) {
+    return fetchDataAsText(this.url, cacheActive).then(parseGeojson)
   }
 }

libs/util/data-fetcher/src/lib/readers/gml.spec.ts

@@ -293,10 +293,9 @@ describe('Gml parsing', () => {
       reader = new GmlReader(
         'http://localfile/fixtures/wfs-gml.xml',
         'ms:n_mat_eolien_p_r32',
-        '2.0.0',
-        cacheActive
+        '2.0.0'
       )
-      reader.load()
+      reader.load(cacheActive)
     })
     afterEach(() => {
       fetchMock.reset()

libs/util/data-fetcher/src/lib/readers/gml.ts

@@ -40,14 +40,13 @@ export class GmlReader extends BaseFileReader {
   constructor(
     protected url: string,
     protected namespace: string,
-    protected version: WfsVersion,
-    protected cacheActive: boolean
+    protected version: WfsVersion
   ) {
-    super(url, cacheActive)
+    super(url)
   }
 
-  protected getData() {
-    return fetchDataAsText(this.url, this.cacheActive).then((text) =>
+  protected getData(cacheActive: boolean) {
+    return fetchDataAsText(this.url, cacheActive).then((text) =>
       parseGml(text, this.namespace, this.version)
     )
   }

libs/util/data-fetcher/src/lib/readers/json.spec.ts

@@ -246,10 +246,9 @@ describe('json parsing', () => {
         }
       )
       reader = new JsonReader(
-        'http://localfile/fixtures/perimetre-des-epci-concernes-par-un-contrat-de-ville.json',
-        cacheActive
+        'http://localfile/fixtures/perimetre-des-epci-concernes-par-un-contrat-de-ville.json'
       )
-      reader.load()
+      reader.load(cacheActive)
     })
     afterEach(() => {
       fetchMock.reset()

libs/util/data-fetcher/src/lib/readers/json.ts

@@ -22,7 +22,7 @@ export function parseJson(text: string): {
 }
 
 export class JsonReader extends BaseFileReader {
-  getData() {
-    return fetchDataAsText(this.url, this.cacheActive).then(parseJson)
+  getData(cacheActive: boolean) {
+    return fetchDataAsText(this.url, cacheActive).then(parseJson)
   }
 }

libs/util/data-fetcher/src/lib/readers/wfs.spec.ts

@@ -6,7 +6,7 @@ import path from 'path'
 import fs from 'fs/promises'
 import { WfsReader } from './wfs'
 import { WfsEndpoint, useCache } from '@camptocamp/ogc-client'
-import { GeojsonReader, parseGeojson } from './geojson'
+import { GeojsonReader } from './geojson'
 import { GmlReader } from './gml'
 
 const urlGeojson =
@@ -176,8 +176,7 @@ describe('WfsReader', () => {
           sendAsJson: false,
         }
       )
-      reader = new WfsReader(urlGeojson, wfsEndpoint, 'epci', cacheActive)
-      reader.load()
+      reader = new WfsReader(urlGeojson, wfsEndpoint, 'epci')
     })
     afterEach(() => {
       fetchMock.reset()
@@ -313,7 +312,6 @@ describe('WfsReader', () => {
       )
       const wfsEndpoint = new WfsEndpoint(urlGml)
       reader = new WfsReader(urlGml, wfsEndpoint, 'ms:n_mat_eolien_p_r32')
-      reader.load()
     })
     afterEach(() => {
       fetchMock.reset()
@@ -379,7 +377,6 @@ describe('WfsReader', () => {
 
   describe('#createReader', () => {
     let reader: WfsReader
-    let cacheActive = true
     let GmlReaderSpy: jest.SpyInstance
     beforeEach(() => {
       GmlReaderSpy = jest.spyOn({ GmlReader }, 'GmlReader')
@@ -400,13 +397,7 @@ describe('WfsReader', () => {
         }
       )
       const wfsEndpoint = new WfsEndpoint(urlGml)
-      reader = new WfsReader(
-        urlGml,
-        wfsEndpoint,
-        'ms:n_mat_eolien_p_r32',
-        cacheActive
-      )
-      reader.load()
+      reader = new WfsReader(urlGml, wfsEndpoint, 'ms:n_mat_eolien_p_r32')
     })
     afterEach(() => {
       fetchMock.reset()
@@ -418,41 +409,34 @@ describe('WfsReader', () => {
       ).resolves.toBeInstanceOf(WfsReader)
     })
     it('returns an instance of GeojsonReader', async () => {
-      await WfsReader.createReader(
-        urlGeojsonLegacy,
-        urlGeojsonLegacy,
-        cacheActive
-      )
+      await WfsReader.createReader(urlGeojsonLegacy, urlGeojsonLegacy)
       expect(GeojsonReader).toHaveBeenCalledWith(
-        'https://mygeojsonreader.edu?1=1&STARTINDEX=undefined&MAXFEATURES=undefined',
-        true
+        'https://mygeojsonreader.edu?1=1&STARTINDEX=undefined&MAXFEATURES=undefined'
       )
     })
     it('returns an instance of GmlReader', async () => {
-      await WfsReader.createReader(urlGmlLegacy, urlGmlLegacy, cacheActive)
+      await WfsReader.createReader(urlGmlLegacy, urlGmlLegacy)
       expect(GmlReader).toHaveBeenCalledWith(
         'https://mygmlreader.edu?1=1&STARTINDEX=undefined&MAXFEATURES=undefined',
         'any',
-        '1.0.0',
-        true
+        '1.0.0'
       )
     })
 
     describe('When cache should be used', () => {
       it('uses the cache', async () => {
         const useCacheSpy = jest.spyOn({ useCache }, 'useCache')
-        await reader.read()
+        await reader.read(true)
         expect(useCacheSpy).toHaveBeenCalledTimes(1)
       })
     })
     describe('When cache should not be used', () => {
       beforeAll(() => {
         jest.clearAllMocks()
-        cacheActive = false
       })
       it('does not use the cache', async () => {
         const useCacheSpy = jest.spyOn({ useCache }, 'useCache')
-        await reader.read()
+        await reader.read(false)
         expect(useCacheSpy).not.toHaveBeenCalled()
       })
     })