Merge e45d6fc into f804e4f

CHANGELOG.md

@@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Unreleased
 
+### Fixed
+- merging of baseProperties with customProperties causing inconsistent microservices' env Variables schema
+
 ### Added
 
 - add optional metrics on request duration. The metrics collection is enabled by mean of the variable `ENABLE_HTTP_CLIENT_METRICS` (default: false)

index.js

@@ -94,30 +94,36 @@ const baseSchema = {
   },
 }
 
-function mergeJsonSchemas(schema, otherSchema) {
-  const { properties: schemaProperties, required: requiredSchema = [], ...schemaRemainingProperties } = schema
-  const {
-    properties: otherSchemaProperties,
-    required: requiredOtherSchema = [],
-    ...otherSchemaRemainingProperties
-  } = otherSchema
-  const mergedSchema = {
-    type: 'object',
-    properties: {
-      ...schemaProperties,
-      ...otherSchemaProperties,
-    },
-    required: [...requiredSchema, ...requiredOtherSchema],
-    allOf: [
-      schemaRemainingProperties,
-      otherSchemaRemainingProperties,
-    ],
-    additionalProperties: false,
+function mergeObjectOrArrayProperty(toBeMergedValues, alreadyMergedValues, isArray) {
+  return isArray ? [
+    ...toBeMergedValues ?? [],
+    ...alreadyMergedValues,
+  ] : {
+    ...toBeMergedValues ?? {},
+    ...alreadyMergedValues,
   }
-  return mergedSchema
+}
+
+// WARNING: including any first level properties other than the ones already declared
+// may have undesired effects on the result of the merge
+function mergeWithDefaultJsonSchema(schema) {
+  const defaultSchema = {
+    ...baseSchema,
+  }
+
+  Object.keys(schema).forEach(key => {
+    defaultSchema[key] = typeof schema[key] === 'object'
+      ? mergeObjectOrArrayProperty(defaultSchema[key], schema[key], Array.isArray(schema[key]))
+      : schema[key]
+  })
+
+  return defaultSchema
 }
 
 function getOverlappingKeys(properties, otherProperties) {
+  if (!otherProperties) {
+    return []
+  }
   const propertiesNames = Object.keys(properties)
   const otherPropertiesNames = Object.keys(otherProperties)
   const overlappingProperties = propertiesNames.filter(propertyName =>
@@ -315,16 +321,17 @@ async function decorateRequestAndFastifyInstance(fastify, { asyncInitFunction, s
   })
 }
 
-const defaultSchema = { type: 'object', required: [], properties: {} }
 
-function initCustomServiceEnvironment(envSchema = defaultSchema) {
+function initCustomServiceEnvironment(envSchema) {
   return function customService(asyncInitFunction, serviceOptions) {
     async function index(fastify, opts) {
-      const overlappingPropertiesNames = getOverlappingKeys(baseSchema.properties, envSchema.properties)
+      const overlappingPropertiesNames = getOverlappingKeys(baseSchema.properties, envSchema?.properties)
       if (overlappingPropertiesNames.length > 0) {
         throw new Error(`The provided Environment JSON Schema includes properties declared in the Base JSON Schema of the custom-plugin-lib, please remove them from your schema. The properties to remove are: ${overlappingPropertiesNames.join(', ')}`)
       }
-      fastify.register(fastifyEnv, { schema: mergeJsonSchemas(baseSchema, envSchema), data: opts })
+
+      const schema = envSchema ? mergeWithDefaultJsonSchema(envSchema) : baseSchema
+      fastify.register(fastifyEnv, { schema, data: opts })
       fastify.register(fastifyFormbody)
       fastify.register(fp(decorateRequestAndFastifyInstance), { asyncInitFunction, serviceOptions })
     }

tests/environment.test.js

@@ -230,5 +230,27 @@ tap.test('Test Environment variables', test => {
     }
   })
 
+  test.test('Should pass since all of the required fields are present', async assert => {
+    const env = {
+      ...baseEnv,
+      MY_REQUIRED_ENV_VAR: 'value',
+    }
+
+    await assert.resolves(async() => {
+      await setupFastify('./tests/services/advanced-first-level-properties-service.js', env)
+    })
+  })
+
+  test.test('Should fail since one of the required fields is not present', async assert => {
+    // NOTE: use try catch instead of assert.reject to customize error message assertion
+    assert.plan(1)
+    try {
+      await setupFastify('./tests/services/advanced-first-level-properties-service.js', baseEnv)
+    } catch (error) {
+      const errorMessage = 'env must have required property \'MY_REQUIRED_ENV_VAR\''
+      assert.strictSame(error.message, errorMessage)
+    }
+  })
+
   test.end()
 })

tests/index.test.js

@@ -28,6 +28,7 @@ const GROUPS_HEADER_KEY = 'groups-header-key'
 const CLIENTTYPE_HEADER_KEY = 'clienttype-header-key'
 const BACKOFFICE_HEADER_KEY = 'backoffice-header-key'
 const MICROSERVICE_GATEWAY_SERVICE_NAME = 'microservice-gateway'
+const MY_REQUIRED_ENV_VAR = 'value'
 
 const baseEnv = {
   USERID_HEADER_KEY,
@@ -329,7 +330,7 @@ tap.test('Advanced Custom Service', test => {
 
   test.test('Require some environment variables', async assert => {
     const MY_AWESOME_ENV = 'foobar'
-    const fastify = await setupFastify({ ...baseEnv, MY_AWESOME_ENV })
+    const fastify = await setupFastify({ ...baseEnv, MY_AWESOME_ENV, MY_REQUIRED_ENV_VAR })
     const response = await fastify.inject({
       method: 'GET',
       url: '/env',
@@ -344,7 +345,7 @@ tap.test('Advanced Custom Service', test => {
   test.test('Decorate fastify with custom functionalities', async assert => {
     const MY_AWESOME_ENV = 'foobar'
     const payload = { hello: 'world' }
-    const fastify = await setupFastify({ ...baseEnv, MY_AWESOME_ENV })
+    const fastify = await setupFastify({ ...baseEnv, MY_AWESOME_ENV, MY_REQUIRED_ENV_VAR })
     const response = await fastify.inject({
       method: 'POST',
       url: '/custom',
@@ -359,7 +360,7 @@ tap.test('Advanced Custom Service', test => {
   })
 
   test.test('default env var', async assert => {
-    const fastify = await setupFastify(baseEnv)
+    const fastify = await setupFastify({ ...baseEnv, MY_REQUIRED_ENV_VAR })
     const response = await fastify.inject({
       method: 'GET',
       url: '/env',
@@ -478,3 +479,38 @@ tap.test('Service with API formats', t => {
 
   t.end()
 })
+
+tap.test('baseEnv with first level properties', t => {
+  async function setupFastify(envVariables) {
+    const fastify = await lc39('./tests/services/if-then-else-env-validation-custom-service.js', {
+      logLevel: 'silent',
+      envVariables,
+    })
+    return fastify
+  }
+
+  t.test('it should fail the build', async t => {
+    try {
+      await setupFastify({
+        ...baseEnv,
+        if: {
+          properties: {
+            field: {
+              const: 'value',
+            },
+          },
+        },
+        then: {
+          required: [
+            'field1',
+          ],
+        },
+      })
+      t.fail()
+    } catch (error) {
+      t.ok(error)
+    }
+  })
+
+  t.end()
+})

tests/services/advanced-custom-service.js

@@ -17,9 +17,10 @@
 'use strict'
 const envSchema = {
   type: 'object',
-  required: ['MY_AWESOME_ENV'],
+  required: ['MY_AWESOME_ENV', 'MY_REQUIRED_ENV_VAR'],
   properties: {
     MY_AWESOME_ENV: { type: 'string', default: 'the default value' },
+    MY_REQUIRED_ENV_VAR: { type: 'string' },
   },
 }
 

tests/services/advanced-first-level-properties-service.js

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2023 Mia srl
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+*/
+
+'use strict'
+const envSchema = {
+  type: 'object',
+  required: ['MY_AWESOME_ENV', 'MY_REQUIRED_ENV_VAR'],
+  properties: {
+    MY_AWESOME_ENV: { type: 'string', default: 'the default value' },
+    MY_REQUIRED_ENV_VAR: { type: 'string' },
+  },
+  patternProperties: {
+    '^S_': { 'type': 'string' },
+    '^I_': { 'type': 'number' },
+  },
+  minProperties: 1,
+  additionalProperties: true,
+}
+
+const customService = require('../../index')(envSchema)
+
+module.exports = customService(async function clientGroups(service) {
+  async function handlerApiRest(_, reply) {
+    reply.send(this.config.required)
+  }
+
+  service.addRawCustomPlugin('GET', '/api-rest', handlerApiRest)
+})