Merge pull request #1110 from geonetwork/backport/1106-to-2.3.x

[Backport 2.3.x] Datahub: fix some issues in query URL generation form

apps/datahub-e2e/src/e2e/datasetDetailPage.cy.ts

@@ -706,7 +706,7 @@ describe('api form', () => {
     cy.get('@secondInput').type('87')
 
     cy.get('@apiForm').find('gn-ui-dropdown-selector').as('dropdown')
-    cy.get('@dropdown').eq(0).selectDropdownOption('geojson')
+    cy.get('@dropdown').eq(0).selectDropdownOption('application/geo+json')
 
     cy.get('@apiForm')
       .find('gn-ui-copy-text-button')
@@ -727,7 +727,7 @@ describe('api form', () => {
       .find('gn-ui-copy-text-button')
       .find('input')
       .invoke('val')
-      .should('include', 'f=json&limit=-1')
+      .should('include', 'f=application%2Fjson&limit=-1')
   })
   it('should close the panel on click', () => {
     cy.get('gn-ui-record-api-form').prev().find('button').click()

libs/ui/elements/src/lib/record-api-form/record-api-form.component.spec.ts

@@ -14,9 +14,16 @@ const mockDatasetServiceDistribution: DatasetServiceDistribution = {
 
 jest.mock('@camptocamp/ogc-client', () => ({
   OgcApiEndpoint: class {
+    collections_ = [{ name: 'uniqueCollection' }]
     constructor(private url) {}
     get allCollections() {
-      return Promise.resolve([{ name: 'feature1' }])
+      if (this.url.toString().includes('multiple')) {
+        return Promise.resolve([
+          { name: 'firstCollection' },
+          { name: 'otherCollection' },
+        ])
+      }
+      return Promise.resolve(this.collections_)
     }
     getCollectionInfo(collectionId) {
       return Promise.resolve({
@@ -90,58 +97,79 @@ describe('RecordApiFormComponent', () => {
       expect(component.apiBaseUrl).toBe('https://api.example.com/data')
       expect(component.offset$.getValue()).toBe('')
       expect(component.limit$.getValue()).toBe('-1')
-      expect(component.format$.getValue()).toBe('json')
+      expect(component.format$.getValue()).toBe('application/json')
       const url = await firstValueFrom(component.apiQueryUrl$)
       expect(url).toBe(
-        'https://api.example.com/data/collections/feature1/items?limit=-1&f=json'
+        'https://api.example.com/data/collections/uniqueCollection/items?limit=-1&f=application%2Fjson'
       )
     })
   })
   describe('When URL params are changed', () => {
     it('should update query URL correctly when setting offset, limit, and format', async () => {
       const mockOffset = '10'
       const mockLimit = '20'
-      const mockFormat = 'json'
+      const mockFormat = 'text/csv'
       component.setOffset(mockOffset)
       component.setLimit(mockLimit)
       component.setFormat(mockFormat)
       const url = await firstValueFrom(component.apiQueryUrl$)
       expect(url).toBe(
-        `https://api.example.com/data/collections/feature1/items?limit=${mockLimit}&offset=${mockOffset}&f=${mockFormat}`
+        `https://api.example.com/data/collections/uniqueCollection/items?limit=${mockLimit}&offset=${mockOffset}&f=${encodeURIComponent(
+          mockFormat
+        )}`
       )
     })
     it('should remove the param in url if value is null', async () => {
       const mockOffset = '0'
       const mockLimit = '20'
-      const mockFormat = 'json'
+      const mockFormat = 'application/json'
       component.setOffset(mockOffset)
       component.setLimit(mockLimit)
       component.setFormat(mockFormat)
       const url = await firstValueFrom(component.apiQueryUrl$)
       expect(url).toBe(
-        `https://api.example.com/data/collections/feature1/items?limit=${mockLimit}&offset=${mockOffset}&f=${mockFormat}`
+        `https://api.example.com/data/collections/uniqueCollection/items?limit=${mockLimit}&offset=${mockOffset}&f=${encodeURIComponent(
+          mockFormat
+        )}`
       )
     })
     it('should remove the param in url if value is zero', async () => {
       const mockOffset = '10'
       const mockLimit = '0'
-      const mockFormat = 'json'
+      const mockFormat = 'application/json'
       component.setOffset(mockOffset)
       component.setLimit(mockLimit)
       component.setFormat(mockFormat)
       const url = await firstValueFrom(component.apiQueryUrl$)
       expect(url).toBe(
-        `https://api.example.com/data/collections/feature1/items?limit=${mockLimit}&offset=${mockOffset}&f=${mockFormat}`
+        `https://api.example.com/data/collections/uniqueCollection/items?limit=${mockLimit}&offset=${mockOffset}&f=${encodeURIComponent(
+          mockFormat
+        )}`
       )
     })
+
+    describe('when multiple collections available', () => {
+      it('uses the link name', async () => {
+        component.apiLink = {
+          ...mockDatasetServiceDistribution,
+          url: new URL('https://api.example.com/multiple'),
+          name: 'myCollection',
+        }
+        fixture.detectChanges()
+        const url = await firstValueFrom(component.apiQueryUrl$)
+        expect(url).toBe(
+          `https://api.example.com/multiple/collections/myCollection/items?limit=-1&f=application%2Fjson`
+        )
+      })
+    })
   })
 
   describe('#resetUrl', () => {
     it('should reset URL to default parameters', () => {
       component.resetUrl()
       expect(component.offset$.getValue()).toBe('')
       expect(component.limit$.getValue()).toBe('-1')
-      expect(component.format$.getValue()).toBe('json')
+      expect(component.format$.getValue()).toBe('application/json')
     })
   })
 
@@ -153,9 +181,9 @@ describe('RecordApiFormComponent', () => {
     it('should parse the returned formats', () => {
       component.parseOutputFormats()
       expect(component.outputFormats).toEqual([
-        { value: 'csv', label: 'CSV' },
-        { value: 'geojson', label: 'GEOJSON' },
-        { value: 'json', label: 'JSON' },
+        { value: 'text/csv', label: 'CSV' },
+        { value: 'application/geo+json', label: 'GEOJSON' },
+        { value: 'application/json', label: 'JSON' },
       ])
     })
   })
@@ -174,10 +202,19 @@ describe('RecordApiFormComponent', () => {
       expect(component.accessServiceProtocol).toBe('wfs')
       expect(component.offset$.getValue()).toBe('')
       expect(component.limit$.getValue()).toBe('-1')
-      expect(component.format$.getValue()).toBe('json')
+      expect(component.format$.getValue()).toBe('application/json')
+      const url = await firstValueFrom(component.apiQueryUrl$)
+      expect(url).toBe(
+        `https://api.example.com/data?type=mockFeatureType&options={"outputFormat":"application/json"}`
+      )
+    })
+
+    it('sets maxFeatures if a limit is set', async () => {
+      component.setLimit('12')
+      expect(component.limit$.getValue()).toBe('12')
       const url = await firstValueFrom(component.apiQueryUrl$)
       expect(url).toBe(
-        `https://api.example.com/data?type=mockFeatureType&options={"outputFormat":"json","limit":-1}`
+        `https://api.example.com/data?type=mockFeatureType&options={"outputFormat":"application/json","maxFeatures":12}`
       )
     })
   })

libs/ui/elements/src/lib/record-api-form/record-api-form.component.ts

@@ -6,16 +6,12 @@ import {
 } from '@geonetwork-ui/common/domain/model/record'
 import { mimeTypeToFormat } from '@geonetwork-ui/util/shared'
 import { BehaviorSubject, combineLatest, filter, map, switchMap } from 'rxjs'
+import { DropdownChoice } from '@geonetwork-ui/ui/inputs'
 
 const DEFAULT_PARAMS = {
   OFFSET: '',
   LIMIT: '-1',
-  FORMAT: 'json',
-}
-
-interface OutputFormats {
-  itemFormats?: any[]
-  outputFormats?: any[]
+  FORMAT: 'application/json',
 }
 
 @Component({
@@ -26,7 +22,7 @@ interface OutputFormats {
 })
 export class RecordApiFormComponent {
   @Input() set apiLink(value: DatasetServiceDistribution) {
-    this.outputFormats = [{ value: 'json', label: 'JSON' }]
+    this.outputFormats = [{ value: 'application/json', label: 'JSON' }]
     this.accessServiceProtocol = value ? value.accessServiceProtocol : undefined
     this.apiFeatureType = value ? value.name : undefined
     if (value) {
@@ -46,9 +42,10 @@ export class RecordApiFormComponent {
   apiFeatureType: string
   supportOffset = true
   accessServiceProtocol: ServiceProtocol | undefined
-  outputFormats = [{ value: 'json', label: 'JSON' }]
+  outputFormats: DropdownChoice[] = [
+    { value: 'application/json', label: 'JSON' },
+  ]
   endpoint: WfsEndpoint | OgcApiEndpoint | undefined
-  firstCollection: string | undefined
 
   apiQueryUrl$ = combineLatest([
     this.offset$,
@@ -89,42 +86,34 @@ export class RecordApiFormComponent {
 
   async parseOutputFormats() {
     if (!this.endpoint) return
-    const apiUrl = this.apiBaseUrl.endsWith('?')
-      ? this.apiBaseUrl.slice(0, -1)
-      : this.apiBaseUrl
-    const outputFormats = await this.getOutputFormats(apiUrl)
-
-    const formatsList = outputFormats.itemFormats
-      ? this.mapFormats(outputFormats.itemFormats)
-      : this.mapFormats(outputFormats.outputFormats || [])
+    const outputFormats = (await this.getOutputFormats()).map(
+      this.mimeTypeToFormatName
+    )
 
     this.outputFormats = this.outputFormats
-      .concat(formatsList.filter(Boolean))
+      .concat(outputFormats.filter(Boolean))
       .filter(
         (format, index, self) =>
           index === self.findIndex((t) => t.value === format.value)
       )
       .sort((a, b) => a.label.localeCompare(b.label))
   }
 
-  mapFormats(formats: any[]) {
-    return formats.map((format) => {
-      const normalizedFormat = mimeTypeToFormat(format)
-      return normalizedFormat
-        ? { label: normalizedFormat.toUpperCase(), value: normalizedFormat }
-        : null
-    })
+  mimeTypeToFormatName(mimeType: string): DropdownChoice | null {
+    const formatName = mimeTypeToFormat(mimeType)
+    return formatName
+      ? { label: formatName.toUpperCase(), value: mimeType }
+      : null
   }
 
-  async getOutputFormats(url: string): Promise<OutputFormats> {
-    if (!this.endpoint) return {}
+  async getOutputFormats(): Promise<string[]> {
+    if (!this.endpoint) return []
     if (this.endpoint instanceof WfsEndpoint) {
       this.supportOffset = this.endpoint.supportsStartIndex()
-      return this.endpoint.getServiceInfo() as OutputFormats
+      return this.endpoint.getServiceInfo().outputFormats
     } else {
-      return (await this.endpoint.getCollectionInfo(
-        this.firstCollection
-      )) as OutputFormats
+      return (await this.endpoint.getCollectionInfo(this.apiFeatureType))
+        .itemFormats
     }
   }
 
@@ -135,7 +124,11 @@ export class RecordApiFormComponent {
       await (this.endpoint as WfsEndpoint).isReady()
     } else {
       this.endpoint = new OgcApiEndpoint(this.apiBaseUrl)
-      this.firstCollection = (await this.endpoint.allCollections)[0].name
+      const collections = await this.endpoint.allCollections
+      // if there's only one collection, use this instead of the name given in the link.
+      if (collections.length === 1) {
+        this.apiFeatureType = collections[0].name
+      }
     }
     this.endpoint$.next(this.endpoint)
   }
@@ -151,16 +144,17 @@ export class RecordApiFormComponent {
       outputFormat: format,
       startIndex: offset ? Number(offset) : undefined,
       maxFeatures: limit !== '-1' ? Number(limit) : undefined,
-      limit: limit !== '-1' ? Number(limit) : limit === '-1' ? -1 : undefined,
+      limit: limit !== '-1' ? Number(limit) : -1,
       offset: offset !== '' ? Number(offset) : undefined,
     }
 
     if (this.endpoint instanceof WfsEndpoint) {
+      delete options.limit
       options.maxFeatures = limit !== '-1' ? Number(limit) : undefined
       return this.endpoint.getFeatureUrl(this.apiFeatureType, options)
     } else {
       return await this.endpoint.getCollectionItemsUrl(
-        this.firstCollection,
+        this.apiFeatureType,
         options
       )
     }