fix: resolve export bugs (#300)

* Fix xls export * Refactor resolveProjection in httpInterface * Fix export * Fix PR and changelog * Fix changelog --------- Co-authored-by: Paola Nicosia <[email protected]>
mia-platform · May 3, 2024 · a95961d · a95961d
1 parent 0911448
commit a95961d
Show file tree

Hide file tree

Showing 12 changed files with 292 additions and 431 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Fixed 
 
+- fixed `GET /export` with `csv` format: `csv` files contain all the requested fields as columns without omissions
+- fixed `GET /export` with `excel` format: `excel` files contain all the requested fields as columns instead of containing always all the collection fields
 - allow running raw queries with `_q` over fields containing a digit in their name
 
 ## 7.0.1 - 2024-04-08

diff --git a/lib/CrudService.js b/lib/CrudService.js
@@ -112,10 +112,9 @@ class CrudService {
   findAll(context, query, projection, sort, skip, limit, _states, isTextSearchQuery) {
     const stateQuery = getStateQuery(_states)
     const isQueryValid = query && Object.keys(query).length > 0
-    const mongoProjection = getProjection(projection)
 
     if (isTextSearchQuery) {
-      addTextScoreProjection(mongoProjection)
+      addTextScoreProjection(projection)
     }
 
     const sortConfig = getSorting({
@@ -129,7 +128,7 @@ class CrudService {
 
     let cursor = this._mongoCollection
       .find(searchQuery, options)
-      .project(mongoProjection)
+      .project(projection)
 
     if (sortConfig !== undefined && sortConfig !== null) { cursor = cursor.sort(sortConfig) }
     if (skip !== undefined && skip !== null) { cursor = cursor.skip(skip) }
@@ -144,28 +143,26 @@ class CrudService {
     const searchQuery = isQueryValid
       ? { $and: [query, { _id: id }, stateQuery] }
       : { $and: [{ _id: id }, stateQuery] }
-    const mongoProjection = getProjection(projection)
     const options = getQueryOptions(this._options)
 
     context.log.debug({ query: searchQuery, projection, options }, 'findById operation requested')
 
     return this._mongoCollection.findOne(searchQuery, {
-      projection: mongoProjection, ...options,
+      projection, ...options,
     })
   }
 
   aggregate(context, query, projection, sort, skip, limit, _states, isTextSearchQuery) {
     const stateQuery = getStateQuery(_states)
     const isQueryValid = query && Object.keys(query).length > 0
-    const mongoProjection = getProjection(projection)
 
     const sortConfig = getSorting({ defaultSorting: this._defaultSorting, sort, isTextSearchQuery })
     let textSearchQuery = {}
     if (isTextSearchQuery) {
       const [path] = JSONPath({ json: query, resultType: 'pointer', path: '$..[?(@property === "$text")]' }).map(getPathFromPointer)
       textSearchQuery = { $text: lget(query, path) }
       lunset(query, path)
-      addTextScoreProjection(mongoProjection)
+      addTextScoreProjection(projection)
     }
 
     const searchQuery = isQueryValid ? { $and: [query, stateQuery] } : stateQuery
@@ -175,7 +172,7 @@ class CrudService {
 
     const pipeline = [
       { $match: { ...textSearchQuery, ...stateQuery } },
-      { $project: mongoProjection },
+      { $project: projection },
       { $match: query },
     ]
     if (sortConfig !== undefined && sortConfig !== null) { pipeline.push({ $sort: sortConfig }) }
@@ -296,15 +293,12 @@ class CrudService {
     commands.$set[UPDATERID] = context.userId
     commands.$set[UPDATEDAT] = context.now
 
-    const mongoProjection = getProjection(projection)
-
-
     const writeOpResult = await this._mongoCollection.findOneAndUpdate(
       searchQuery,
       commands,
       {
         returnDocument: 'after',
-        projection: mongoProjection,
+        projection,
       }
     )
 
@@ -350,15 +344,13 @@ class CrudService {
     commands.$setOnInsert[CREATEDAT] = context.now
     commands.$setOnInsert[__STATE__] = this._stateOnInsert
 
-    const mongoProjection = getProjection(projection)
-
     const writeOpResult = await this._mongoCollection.findOneAndUpdate(
       searchQuery,
       commands,
       {
         returnDocument: 'after',
         upsert: true,
-        projection: mongoProjection,
+        projection,
       }
     )
 
@@ -585,21 +577,6 @@ class CrudService {
   }
 }
 
-function getProjection(projection) {
-  // In case of empty projection, we project only the _id
-  if (!projection?.length) { return { _id: 1 } }
-
-  return projection.reduce((acc, val) => {
-    const propertiesToInclude = typeof val === 'string'
-      // a string represents the name of a field to be projected
-      ? { [val]: 1 }
-      // an object represents a raw projection to be passed as it is
-      : val
-
-    return { ...acc, ...propertiesToInclude }
-  }, {})
-}
-
 function addTextScoreProjection(projection) {
   if (!Object.keys(projection).includes('score')) {
     projection.score = textScore

diff --git a/lib/httpInterface.js b/lib/httpInterface.js
@@ -409,6 +409,14 @@ async function handleCollectionImport(request, reply) {
   return reply.code(returnCode).send({ message: 'File uploaded successfully' })
 }
 
+function getExportColumns(projection) {
+  const columns = Object.keys(projection).filter(key => projection[key] !== 0)
+  if (!columns.includes('_id') && projection['_id'] !== 0) {
+    columns.unshift('_id')
+  }
+  return columns
+}
+
 // eslint-disable-next-line max-statements
 async function handleGetListLookup(request, reply) {
   if (this.customMetrics) {
@@ -445,14 +453,29 @@ async function handleGetListLookup(request, reply) {
     '',
     log
   )
+  delete projection._id
 
-  projection = this.lookupProjection.filter(proj => projection.includes(Object.keys(proj)[0]))
-  if (projection.length === 0) {
+  projection = this.lookupProjection.reduce((acc, proj) => {
+    if (projection[Object.keys(proj)[0]]) {
+      return { ...acc, ...proj }
+    }
+    return acc
+  }, {})
+  if (Object.keys(projection).length === 0) {
     reply.getHttpError(BAD_REQUEST_ERROR_STATUS_CODE, 'No allowed colums')
   }
 
-  const LookupProjectionFieldsToOmit = this.lookupProjection.filter(field => Object.values(field).shift() === 0)
-  projection.push(...LookupProjectionFieldsToOmit)
+  const lookupProjectionFieldsToOmit = this.lookupProjection.reduce((acc, field) => {
+    if (Object.values(field).shift() === 0) {
+      return { ...acc, ...field }
+    }
+    return acc
+  },
+  {})
+  projection = {
+    ...projection,
+    ...lookupProjectionFieldsToOmit,
+  }
 
   const isTextSearchQuery = query._q && this.queryParser.isTextSearchQuery(JSON.parse(query._q))
   const mongoQuery = resolveMongoQuery(this.queryParser, clientQueryString, acl_rows, otherParams, isTextSearchQuery)
@@ -480,7 +503,7 @@ async function handleGetListLookup(request, reply) {
         .stream(),
       this.castResultsAsStream(),
       streamValidator(),
-      ...responseStringifiers({ fields: this.allFieldNames }),
+      ...responseStringifiers({ fields: getExportColumns(projection) }),
       reply.raw
     )
   } catch (error) {
@@ -492,6 +515,7 @@ async function handleGetListLookup(request, reply) {
   }
 }
 
+// eslint-disable-next-line max-statements
 async function handleGetList(request, reply) {
   if (this.customMetrics) {
     this.customMetrics.collectionInvocation.inc({
@@ -555,7 +579,7 @@ async function handleGetList(request, reply) {
         .stream(),
       this.castResultsAsStream(),
       streamValidator(),
-      ...responseStringifiers({ fields: this.allFieldNames }),
+      ...responseStringifiers({ fields: getExportColumns(projection) }),
       reply.raw
     )
   } catch (error) {
@@ -1042,13 +1066,31 @@ function resolveProjection(clientProjectionString, aclColumns, allFieldNames, ra
       'Use of both _rawp and _p parameter is not allowed')
   }
 
+  let projection
   if (!clientProjectionString && !rawProjection) {
-    return removeAclColumns(allFieldNames, acls)
+    projection = removeAclColumns(allFieldNames, acls)
   } else if (rawProjection) {
-    return resolveRawProjectionString(rawProjection, acls, allFieldNames, log)
+    projection = resolveRawProjectionString(rawProjection, acls, allFieldNames, log)
   } else if (clientProjectionString) {
-    return resolveClientProjectionString(clientProjectionString, acls)
+    projection = resolveClientProjectionString(clientProjectionString, acls)
   }
+
+  return getProjection(projection)
+}
+
+function getProjection(projection) {
+  // In case of empty projection, we project only the _id
+  if (!projection?.length) { return { _id: 1 } }
+
+  return projection.reduce((acc, val) => {
+    const propertiesToInclude = typeof val === 'string'
+      // a string represents the name of a field to be projected
+      ? { [val]: 1 }
+      // an object represents a raw projection to be passed as it is
+      : val
+
+    return { ...acc, ...propertiesToInclude }
+  }, {})
 }
 
 function resolveClientProjectionString(clientProjectionString, _acls) {

diff --git a/lib/transformers/csv.js b/lib/transformers/csv.js
@@ -19,13 +19,14 @@ const csvParse = require('csv-parse')
 const csvStringify = require('csv-stringify')
 
 module.exports = (parsingOptions) => ({
-  stringifier: () => [csvStringify.stringify({
+  stringifier: ({ fields }) => [csvStringify.stringify({
     encoding: 'utf8',
     delimiter: ',',
     escape: '\\',
     header: true,
     quote: false,
     ...parsingOptions,
+    columns: fields,
     cast: {
       object: (value) => {
         try {

diff --git a/tests/CrudService.special.test.js b/tests/CrudService.special.test.js
@@ -30,9 +30,10 @@ const {
   BOOKS_COLLECTION_NAME,
   defaultSorting,
   fixtures,
+  getProjectionFromObject,
 } = require('./utils')
 
-const ALL_FIELDS = Object.keys(publicFixtures[0])
+const ALL_FIELDS = getProjectionFromObject(publicFixtures[0])
 
 const [firstPublicFixture] = publicFixtures
 const OBJECT_ID = firstPublicFixture._id
@@ -42,6 +43,7 @@ const context = {
   userId: 'my-user-id',
   now: new Date('2018-02-08'),
 }
+const DEFAULT_PROJECTION = { _id: 1 }
 
 tap.test('allowDiskUse', async t => {
   const databaseName = getMongoDatabaseName()
@@ -78,7 +80,7 @@ tap.test('allowDiskUse', async t => {
     const crudService = new CrudService(collection, STATES.PUBLIC, null, { allowDiskUse: true })
 
     t.test('in findAll', async t => {
-      await crudService.findAll(context, { price: { $gt: 20 } }).toArray()
+      await crudService.findAll(context, { price: { $gt: 20 } }, DEFAULT_PROJECTION).toArray()
       t.strictSame(commandFailedEvents, [])
       t.match(commandStartedEvents, [{ commandName: 'find', command: { allowDiskUse: true } }])
     })
@@ -156,7 +158,7 @@ tap.test('allowDiskUse', async t => {
 
     const defaultAscendendFixtures = getSortedFixtures((a, b) => a.name.localeCompare(b.name))
     t.test('if isn\'t defined, defaultSorting is applied', async t => {
-      const result = await crudService.findAll(context, {}).toArray()
+      const result = await crudService.findAll(context, {}, DEFAULT_PROJECTION).toArray()
       t.strictSame(
         result,
         defaultAscendendFixtures
@@ -171,7 +173,7 @@ tap.test('allowDiskUse', async t => {
 
       t.notSame(ascendedFixturesByPrice, defaultAscendendFixtures)
 
-      const result = await crudService.findAll(context, {}, null, sort).toArray()
+      const result = await crudService.findAll(context, {}, DEFAULT_PROJECTION, sort).toArray()
 
       t.strictSame(
         result,