From c3e910616fb70be2337e5b4485e3c27cf603eef3 Mon Sep 17 00:00:00 2001 From: KrishDave1 Date: Fri, 11 Oct 2024 02:46:21 +0530 Subject: [PATCH 01/16] feat: Add customizable log level for 'username already exists' error --- spec/ParseUser.spec.js | 28 ++++++++++++++++++++++++++++ src/Options/docs.js | 6 ++++++ src/Options/index.js | 11 +++++++++++ src/RestWrite.js | 14 +++++++++++++- 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index b33b5f61f2..3191f28779 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4429,4 +4429,32 @@ describe('allowClientClassCreation option', () => { // Need to set it back to true to avoid other test fails defaultConfiguration.allowClientClassCreation = true; }); + + it('should respect custom log level for username already exists error', async () => { + const loggerController = { + error: jasmine.createSpy('error'), + warn: jasmine.createSpy('warn'), + info: jasmine.createSpy('info') + }; + await reconfigureServer({ + logEvents: { + usernameAlreadyExists: 'warn' + }, + loggerController + }); + const user = new Parse.User(); + user.setUsername('existingUser'); + user.setPassword('password'); + await user.signUp(); + const duplicateUser = new Parse.User(); + duplicateUser.setUsername('existingUser'); + duplicateUser.setPassword('password'); + try { + await duplicateUser.signUp(); + } catch (error) { + expect(error.code).toBe(Parse.Error.USERNAME_TAKEN); + expect(loggerController.warn).toHaveBeenCalled(); + expect(loggerController.error).not.toHaveBeenCalled(); + } + }); }); diff --git a/src/Options/docs.js b/src/Options/docs.js index 4c2883adaa..d3d7027334 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -62,6 +62,7 @@ * @property {Adapter} loggerAdapter Adapter module for the logging sub-system * @property {String} logLevel Sets the level for logs * @property {LogLevels} logLevels (Optional) Overrides the log levels used internally by Parse Server to log events. + * @property {LogEventsOptions} logEvents (Optional) Customize log levels for specific events. * @property {String} logsFolder Folder for the logs (defaults to './logs'); set to null to disable file based logging * @property {String} maintenanceKey (Optional) The maintenance key is used for modifying internal and read-only fields of Parse Server.

⚠️ This key is not intended to be used as part of a regular operation of Parse Server. This key is intended to conduct out-of-band changes such as one-time migrations or data correction tasks. Internal fields are not officially documented and may change at any time without publication in release changelogs. We strongly advice not to rely on internal fields as part of your regular operation and to investigate the implications of any planned changes *directly in the source code* of your current version of Parse Server. * @property {String[]} maintenanceKeyIps (Optional) Restricts the use of maintenance key permissions to a list of IP addresses or ranges.

This option accepts a list of single IP addresses, for example `['10.0.0.1', '10.0.0.2']`. You can also use CIDR notation to specify an IP address range, for example `['10.0.1.0/24']`.

Special scenarios:
- Setting an empty array `[]` means that the maintenance key cannot be used even in Parse Server Cloud Code. This value cannot be set via an environment variable as there is no way to pass an empty array to Parse Server via an environment variable.
- Setting `['0.0.0.0/0', '::0']` means to allow any IPv4 and IPv6 address to use the maintenance key and effectively disables the IP filter.

Considerations:
- IPv4 and IPv6 addresses are not compared against each other. Each IP version (IPv4 and IPv6) needs to be considered separately. For example, `['0.0.0.0/0']` allows any IPv4 address and blocks every IPv6 address. Conversely, `['::0']` allows any IPv6 address and blocks every IPv4 address.
- Keep in mind that the IP version in use depends on the network stack of the environment in which Parse Server runs. A local environment may use a different IP version than a remote environment. For example, it's possible that locally the value `['0.0.0.0/0']` allows the request IP because the environment is using IPv4, but when Parse Server is deployed remotely the request IP is blocked because the remote environment is using IPv6.
- When setting the option via an environment variable the notation is a comma-separated string, for example `"0.0.0.0/0,::0"`.
- IPv6 zone indices (`%` suffix) are not supported, for example `fe80::1%eth0`, `fe80::1%1` or `::1%lo`.

Defaults to `['127.0.0.1', '::1']` which means that only `localhost`, the server instance on which Parse Server runs, is allowed to use the maintenance key. @@ -255,3 +256,8 @@ * @property {String} triggerBeforeError Log level used by the Cloud Code Triggers `beforeSave`, `beforeDelete`, `beforeFind`, `beforeLogin` on error. Default is `error`. * @property {String} triggerBeforeSuccess Log level used by the Cloud Code Triggers `beforeSave`, `beforeDelete`, `beforeFind`, `beforeLogin` on success. Default is `info`. */ + +/** + * @interface LogEventsOptions + * @property {String|boolean} usernameAlreadyExists Log level for the "username already exists" error when trying to sign up. Possible values: 'info', 'error', 'warn', or false to disable logging. If not specified, the default behavior (logging as an error) will be used. + */ diff --git a/src/Options/index.js b/src/Options/index.js index 40e15afb27..18bf763f46 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -87,6 +87,9 @@ export interface ParseServerOptions { verbose: ?boolean; /* Sets the level for logs */ logLevel: ?string; + /* Options for customizing log levels for specific events + :DEFAULT: {} */ + logEvents: ?LogEventsOptions; /* (Optional) Overrides the log levels used internally by Parse Server to log events. :DEFAULT: {} */ logLevels: ?LogLevels; @@ -636,3 +639,11 @@ export interface LogLevels { */ cloudFunctionError: ?string; } + +export interface LogEventsOptions { + /* Log level for the "username already exists" error when trying to sign up. + Possible values: 'info', 'error', 'verbose', or false to disable logging. + If not specified, the default behavior (logging as an error) will be used. + :DEFAULT: undefined */ + usernameAlreadyExists: ?string | boolean; +} diff --git a/src/RestWrite.js b/src/RestWrite.js index 4e243e4418..bc5b3ee0f6 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1557,10 +1557,22 @@ RestWrite.prototype.runDatabaseOperation = function () { // Quick check, if we were able to infer the duplicated field name if (error && error.userInfo && error.userInfo.duplicated_field === 'username') { - throw new Parse.Error( + //Instead of directly throwing the error, we will give error based on logEvent + const usernameError = new Parse.Error( Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.' ); + + // Check for custom log level + const logLevel = this.config.logEvents && this.config.logEvents.usernameAlreadyExists; + if (logLevel) { + // Use custom log level + this.config.loggerController[logLevel](JSON.stringify(usernameError)); + } else if (logLevel !== false) { + // Use default error logging if not explicitly disabled + this.config.loggerController.error(JSON.stringify(usernameError)); + } + throw usernameError; } if (error && error.userInfo && error.userInfo.duplicated_field === 'email') { From a54e2c65da8df3d7d74a678927a569a5002769a3 Mon Sep 17 00:00:00 2001 From: KrishDave1 Date: Fri, 11 Oct 2024 05:58:04 +0530 Subject: [PATCH 02/16] feat: Made the suggested changes --- spec/ParseUser.spec.js | 18 +++++++++--------- src/Options/docs.js | 7 +------ src/Options/index.js | 15 +++++---------- src/RestWrite.js | 13 +++---------- 4 files changed, 18 insertions(+), 35 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 3191f28779..7677245a6f 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4431,30 +4431,30 @@ describe('allowClientClassCreation option', () => { }); it('should respect custom log level for username already exists error', async () => { - const loggerController = { - error: jasmine.createSpy('error'), - warn: jasmine.createSpy('warn'), - info: jasmine.createSpy('info') - }; + const logger = require('../lib/logger').logger; + const logSpy = spyOn(logger, 'warn').and.callThrough(); + await reconfigureServer({ logEvents: { - usernameAlreadyExists: 'warn' + usernameAlreadyExists: 'warn', }, - loggerController }); + const user = new Parse.User(); user.setUsername('existingUser'); user.setPassword('password'); await user.signUp(); + const duplicateUser = new Parse.User(); duplicateUser.setUsername('existingUser'); duplicateUser.setPassword('password'); + try { await duplicateUser.signUp(); } catch (error) { expect(error.code).toBe(Parse.Error.USERNAME_TAKEN); - expect(loggerController.warn).toHaveBeenCalled(); - expect(loggerController.error).not.toHaveBeenCalled(); + expect(logSpy).toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); } }); }); diff --git a/src/Options/docs.js b/src/Options/docs.js index d3d7027334..67330151cb 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -62,7 +62,6 @@ * @property {Adapter} loggerAdapter Adapter module for the logging sub-system * @property {String} logLevel Sets the level for logs * @property {LogLevels} logLevels (Optional) Overrides the log levels used internally by Parse Server to log events. - * @property {LogEventsOptions} logEvents (Optional) Customize log levels for specific events. * @property {String} logsFolder Folder for the logs (defaults to './logs'); set to null to disable file based logging * @property {String} maintenanceKey (Optional) The maintenance key is used for modifying internal and read-only fields of Parse Server.

⚠️ This key is not intended to be used as part of a regular operation of Parse Server. This key is intended to conduct out-of-band changes such as one-time migrations or data correction tasks. Internal fields are not officially documented and may change at any time without publication in release changelogs. We strongly advice not to rely on internal fields as part of your regular operation and to investigate the implications of any planned changes *directly in the source code* of your current version of Parse Server. * @property {String[]} maintenanceKeyIps (Optional) Restricts the use of maintenance key permissions to a list of IP addresses or ranges.

This option accepts a list of single IP addresses, for example `['10.0.0.1', '10.0.0.2']`. You can also use CIDR notation to specify an IP address range, for example `['10.0.1.0/24']`.

Special scenarios:
- Setting an empty array `[]` means that the maintenance key cannot be used even in Parse Server Cloud Code. This value cannot be set via an environment variable as there is no way to pass an empty array to Parse Server via an environment variable.
- Setting `['0.0.0.0/0', '::0']` means to allow any IPv4 and IPv6 address to use the maintenance key and effectively disables the IP filter.

Considerations:
- IPv4 and IPv6 addresses are not compared against each other. Each IP version (IPv4 and IPv6) needs to be considered separately. For example, `['0.0.0.0/0']` allows any IPv4 address and blocks every IPv6 address. Conversely, `['::0']` allows any IPv6 address and blocks every IPv4 address.
- Keep in mind that the IP version in use depends on the network stack of the environment in which Parse Server runs. A local environment may use a different IP version than a remote environment. For example, it's possible that locally the value `['0.0.0.0/0']` allows the request IP because the environment is using IPv4, but when Parse Server is deployed remotely the request IP is blocked because the remote environment is using IPv6.
- When setting the option via an environment variable the notation is a comma-separated string, for example `"0.0.0.0/0,::0"`.
- IPv6 zone indices (`%` suffix) are not supported, for example `fe80::1%eth0`, `fe80::1%1` or `::1%lo`.

Defaults to `['127.0.0.1', '::1']` which means that only `localhost`, the server instance on which Parse Server runs, is allowed to use the maintenance key. @@ -255,9 +254,5 @@ * @property {String} triggerAfter Log level used by the Cloud Code Triggers `afterSave`, `afterDelete`, `afterFind`, `afterLogout`. Default is `info`. * @property {String} triggerBeforeError Log level used by the Cloud Code Triggers `beforeSave`, `beforeDelete`, `beforeFind`, `beforeLogin` on error. Default is `error`. * @property {String} triggerBeforeSuccess Log level used by the Cloud Code Triggers `beforeSave`, `beforeDelete`, `beforeFind`, `beforeLogin` on success. Default is `info`. - */ - -/** - * @interface LogEventsOptions - * @property {String|boolean} usernameAlreadyExists Log level for the "username already exists" error when trying to sign up. Possible values: 'info', 'error', 'warn', or false to disable logging. If not specified, the default behavior (logging as an error) will be used. + * @property {String|boolean} usernameAlreadyExists Log level for the username already exists error when trying to sign up. Possible values: 'silent', 'info', 'error', 'warn', or false to disable logging. Default is `error`. */ diff --git a/src/Options/index.js b/src/Options/index.js index 18bf763f46..8ad9347b49 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -89,9 +89,6 @@ export interface ParseServerOptions { logLevel: ?string; /* Options for customizing log levels for specific events :DEFAULT: {} */ - logEvents: ?LogEventsOptions; - /* (Optional) Overrides the log levels used internally by Parse Server to log events. - :DEFAULT: {} */ logLevels: ?LogLevels; /* Maximum number of logs to keep. If not set, no logs will be removed. This can be a number of files or number of days. If using days, add 'd' as the suffix. (default: null) */ maxLogFiles: ?NumberOrString; @@ -638,12 +635,10 @@ export interface LogLevels { :DEFAULT: error */ cloudFunctionError: ?string; -} - -export interface LogEventsOptions { /* Log level for the "username already exists" error when trying to sign up. - Possible values: 'info', 'error', 'verbose', or false to disable logging. - If not specified, the default behavior (logging as an error) will be used. - :DEFAULT: undefined */ - usernameAlreadyExists: ?string | boolean; + Possible values: 'info', 'error', 'verbose', or false to disable logging. + If not specified, the default behavior (logging as an error) will be used. + :DEFAULT: undefined + */ + usernameAlreadyExists: 'silent' | 'error' | 'info' | 'verbose'; } diff --git a/src/RestWrite.js b/src/RestWrite.js index bc5b3ee0f6..76db6904df 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1563,16 +1563,9 @@ RestWrite.prototype.runDatabaseOperation = function () { 'Account already exists for this username.' ); - // Check for custom log level - const logLevel = this.config.logEvents && this.config.logEvents.usernameAlreadyExists; - if (logLevel) { - // Use custom log level - this.config.loggerController[logLevel](JSON.stringify(usernameError)); - } else if (logLevel !== false) { - // Use default error logging if not explicitly disabled - this.config.loggerController.error(JSON.stringify(usernameError)); - } - throw usernameError; + return this.config.loggerController[this.config.logLevels.usernameAlreadyExists]( + JSON.stringify(usernameError) + ); } if (error && error.userInfo && error.userInfo.duplicated_field === 'email') { From ae266473435bb98a6a54800962342464ef11fc32 Mon Sep 17 00:00:00 2001 From: KrishDave1 Date: Sat, 12 Oct 2024 08:23:46 +0530 Subject: [PATCH 03/16] fix: Added 2 log levels for Testing and followed new ParseServerConfiguration --- spec/ParseUser.spec.js | 53 +++++++++++++++++++++++++++++++++----- src/Options/Definitions.js | 6 +++++ src/Options/docs.js | 2 +- src/Options/index.js | 10 +++---- src/RestWrite.js | 1 - 5 files changed, 58 insertions(+), 14 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 7677245a6f..0fb5e08b41 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4429,13 +4429,27 @@ describe('allowClientClassCreation option', () => { // Need to set it back to true to avoid other test fails defaultConfiguration.allowClientClassCreation = true; }); +}); + +describe('custom log levels for username already exists error', () => { + const logger = require('../lib/logger').logger; + let warnSpy, errorSpy, infoSpy; + + beforeEach(() => { + warnSpy = spyOn(logger, 'warn').and.callThrough(); + errorSpy = spyOn(logger, 'error').and.callThrough(); + infoSpy = spyOn(logger, 'info').and.callThrough(); + }); - it('should respect custom log level for username already exists error', async () => { - const logger = require('../lib/logger').logger; - const logSpy = spyOn(logger, 'warn').and.callThrough(); + afterEach(() => { + warnSpy.calls.reset(); + errorSpy.calls.reset(); + infoSpy.calls.reset(); + }); + it('should respect warn log level for username already exists error', async () => { await reconfigureServer({ - logEvents: { + logLevels: { usernameAlreadyExists: 'warn', }, }); @@ -4453,8 +4467,35 @@ describe('allowClientClassCreation option', () => { await duplicateUser.signUp(); } catch (error) { expect(error.code).toBe(Parse.Error.USERNAME_TAKEN); - expect(logSpy).toHaveBeenCalled(); - expect(logger.error).not.toHaveBeenCalled(); + expect(warnSpy).toHaveBeenCalled(); + expect(errorSpy).not.toHaveBeenCalled(); + expect(infoSpy).not.toHaveBeenCalled(); + } + }); + + it('should respect info log level for username already exists error', async () => { + await reconfigureServer({ + logLevels: { + usernameAlreadyExists: 'info', + }, + }); + + const user = new Parse.User(); + user.setUsername('anotherExistingUser'); + user.setPassword('password'); + await user.signUp(); + + const duplicateUser = new Parse.User(); + duplicateUser.setUsername('anotherExistingUser'); + duplicateUser.setPassword('password'); + + try { + await duplicateUser.signUp(); + } catch (error) { + expect(error.code).toBe(Parse.Error.USERNAME_TAKEN); + expect(infoSpy).toHaveBeenCalled(); + expect(warnSpy).not.toHaveBeenCalled(); + expect(errorSpy).not.toHaveBeenCalled(); } }); }); diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index a1da6c09a7..825fb84a6d 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -1117,4 +1117,10 @@ module.exports.LogLevels = { 'Log level used by the Cloud Code Triggers `beforeSave`, `beforeDelete`, `beforeFind`, `beforeLogin` on success. Default is `info`.', default: 'info', }, + usernameAlreadyExists: { + env: 'PARSE_SERVER_LOG_LEVELS_USERNAME_ALREADY_EXISTS', + help: + 'Log level for the username already exists error when trying to sign up.Default is `error`.', + default: 'error', + }, }; diff --git a/src/Options/docs.js b/src/Options/docs.js index 67330151cb..c839834f05 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -254,5 +254,5 @@ * @property {String} triggerAfter Log level used by the Cloud Code Triggers `afterSave`, `afterDelete`, `afterFind`, `afterLogout`. Default is `info`. * @property {String} triggerBeforeError Log level used by the Cloud Code Triggers `beforeSave`, `beforeDelete`, `beforeFind`, `beforeLogin` on error. Default is `error`. * @property {String} triggerBeforeSuccess Log level used by the Cloud Code Triggers `beforeSave`, `beforeDelete`, `beforeFind`, `beforeLogin` on success. Default is `info`. - * @property {String|boolean} usernameAlreadyExists Log level for the username already exists error when trying to sign up. Possible values: 'silent', 'info', 'error', 'warn', or false to disable logging. Default is `error`. + * @property {String} usernameAlreadyExists Log level for the username already exists error when trying to sign up.Default is `error`. */ diff --git a/src/Options/index.js b/src/Options/index.js index 8ad9347b49..93c73371e9 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -87,7 +87,7 @@ export interface ParseServerOptions { verbose: ?boolean; /* Sets the level for logs */ logLevel: ?string; - /* Options for customizing log levels for specific events + /* (Optional) Overrides the log levels used internally by Parse Server to log events. :DEFAULT: {} */ logLevels: ?LogLevels; /* Maximum number of logs to keep. If not set, no logs will be removed. This can be a number of files or number of days. If using days, add 'd' as the suffix. (default: null) */ @@ -635,10 +635,8 @@ export interface LogLevels { :DEFAULT: error */ cloudFunctionError: ?string; - /* Log level for the "username already exists" error when trying to sign up. - Possible values: 'info', 'error', 'verbose', or false to disable logging. - If not specified, the default behavior (logging as an error) will be used. - :DEFAULT: undefined + /* Log level for the username already exists error when trying to sign up.Default is `error`. + :DEFAULT: error */ - usernameAlreadyExists: 'silent' | 'error' | 'info' | 'verbose'; + usernameAlreadyExists: ?string; } diff --git a/src/RestWrite.js b/src/RestWrite.js index 76db6904df..08b718b091 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1557,7 +1557,6 @@ RestWrite.prototype.runDatabaseOperation = function () { // Quick check, if we were able to infer the duplicated field name if (error && error.userInfo && error.userInfo.duplicated_field === 'username') { - //Instead of directly throwing the error, we will give error based on logEvent const usernameError = new Parse.Error( Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.' From da1785cc2828531eec0f569f5a85d103c8ef0074 Mon Sep 17 00:00:00 2001 From: KrishDave1 Date: Sat, 12 Oct 2024 14:53:09 +0530 Subject: [PATCH 04/16] fix: Added a loop in test to remove redundant code --- spec/ParseUser.spec.js | 76 +++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 45 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 0fb5e08b41..64e2efd56e 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4447,55 +4447,41 @@ describe('custom log levels for username already exists error', () => { infoSpy.calls.reset(); }); - it('should respect warn log level for username already exists error', async () => { - await reconfigureServer({ - logLevels: { - usernameAlreadyExists: 'warn', - }, - }); + const testCases = [ + { logLevel: 'warn', expectedSpy: 'warnSpy' }, + { logLevel: 'info', expectedSpy: 'infoSpy' }, + ]; + + testCases.forEach(({ logLevel, expectedSpy }) => { + it(`should respect ${logLevel} log level for username already exists error`, async () => { + await reconfigureServer({ + logLevels: { + usernameAlreadyExists: logLevel, + }, + }); - const user = new Parse.User(); - user.setUsername('existingUser'); - user.setPassword('password'); - await user.signUp(); + const username = `existingUser_${logLevel}`; - const duplicateUser = new Parse.User(); - duplicateUser.setUsername('existingUser'); - duplicateUser.setPassword('password'); + const user = new Parse.User(); + user.setUsername(username); + user.setPassword('password'); + await user.signUp(); - try { - await duplicateUser.signUp(); - } catch (error) { - expect(error.code).toBe(Parse.Error.USERNAME_TAKEN); - expect(warnSpy).toHaveBeenCalled(); - expect(errorSpy).not.toHaveBeenCalled(); - expect(infoSpy).not.toHaveBeenCalled(); - } - }); + const duplicateUser = new Parse.User(); + duplicateUser.setUsername(username); + duplicateUser.setPassword('password'); - it('should respect info log level for username already exists error', async () => { - await reconfigureServer({ - logLevels: { - usernameAlreadyExists: 'info', - }, + try { + await duplicateUser.signUp(); + } catch (error) { + expect(error.code).toBe(Parse.Error.USERNAME_TAKEN); + expect(this[expectedSpy]).toHaveBeenCalled(); + ['warnSpy', 'errorSpy', 'infoSpy'].forEach(spy => { + if (spy !== expectedSpy) { + expect(this[spy]).not.toHaveBeenCalled(); + } + }); + } }); - - const user = new Parse.User(); - user.setUsername('anotherExistingUser'); - user.setPassword('password'); - await user.signUp(); - - const duplicateUser = new Parse.User(); - duplicateUser.setUsername('anotherExistingUser'); - duplicateUser.setPassword('password'); - - try { - await duplicateUser.signUp(); - } catch (error) { - expect(error.code).toBe(Parse.Error.USERNAME_TAKEN); - expect(infoSpy).toHaveBeenCalled(); - expect(warnSpy).not.toHaveBeenCalled(); - expect(errorSpy).not.toHaveBeenCalled(); - } }); }); From 63b4f9b885d9944b82e25cef1dcb334af1c4ea86 Mon Sep 17 00:00:00 2001 From: KrishDave1 Date: Sat, 12 Oct 2024 22:30:39 +0530 Subject: [PATCH 05/16] feat: Added the both the tests inside one testcase --- spec/ParseUser.spec.js | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 64e2efd56e..28a8ce3f9d 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4447,13 +4447,10 @@ describe('custom log levels for username already exists error', () => { infoSpy.calls.reset(); }); - const testCases = [ - { logLevel: 'warn', expectedSpy: 'warnSpy' }, - { logLevel: 'info', expectedSpy: 'infoSpy' }, - ]; + it('should respect custom log levels for username already exists error', async () => { + const logLevels = ['warn', 'info']; - testCases.forEach(({ logLevel, expectedSpy }) => { - it(`should respect ${logLevel} log level for username already exists error`, async () => { + for (const logLevel of logLevels) { await reconfigureServer({ logLevels: { usernameAlreadyExists: logLevel, @@ -4473,15 +4470,23 @@ describe('custom log levels for username already exists error', () => { try { await duplicateUser.signUp(); + fail('Should have thrown an error'); } catch (error) { expect(error.code).toBe(Parse.Error.USERNAME_TAKEN); - expect(this[expectedSpy]).toHaveBeenCalled(); - ['warnSpy', 'errorSpy', 'infoSpy'].forEach(spy => { - if (spy !== expectedSpy) { - expect(this[spy]).not.toHaveBeenCalled(); - } - }); + if (logLevel === 'warn') { + expect(warnSpy).toHaveBeenCalled(); + expect(infoSpy).not.toHaveBeenCalled(); + } else if (logLevel === 'info') { + expect(infoSpy).toHaveBeenCalled(); + expect(warnSpy).not.toHaveBeenCalled(); + } + expect(errorSpy).not.toHaveBeenCalled(); } - }); + + // Reset spies for the next iteration + warnSpy.calls.reset(); + errorSpy.calls.reset(); + infoSpy.calls.reset(); + } }); }); From d582e56069a375c5ebc8d99ce2bc4b0dc977fede Mon Sep 17 00:00:00 2001 From: KrishDave1 Date: Sat, 12 Oct 2024 23:13:56 +0530 Subject: [PATCH 06/16] Simplified the code further --- spec/ParseUser.spec.js | 50 ++++++++++++------------------------------ 1 file changed, 14 insertions(+), 36 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 28a8ce3f9d..082236bbce 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4431,23 +4431,13 @@ describe('allowClientClassCreation option', () => { }); }); -describe('custom log levels for username already exists error', () => { - const logger = require('../lib/logger').logger; - let warnSpy, errorSpy, infoSpy; - - beforeEach(() => { - warnSpy = spyOn(logger, 'warn').and.callThrough(); - errorSpy = spyOn(logger, 'error').and.callThrough(); - infoSpy = spyOn(logger, 'info').and.callThrough(); - }); - - afterEach(() => { - warnSpy.calls.reset(); - errorSpy.calls.reset(); - infoSpy.calls.reset(); - }); - +describe('custom log levels for user errors', () => { it('should respect custom log levels for username already exists error', async () => { + const logger = require('../lib/logger').logger; + const warnSpy = spyOn(logger, 'warn').and.callThrough(); + const errorSpy = spyOn(logger, 'error').and.callThrough(); + const infoSpy = spyOn(logger, 'info').and.callThrough(); + const logLevels = ['warn', 'info']; for (const logLevel of logLevels) { @@ -4458,32 +4448,20 @@ describe('custom log levels for username already exists error', () => { }); const username = `existingUser_${logLevel}`; - - const user = new Parse.User(); - user.setUsername(username); - user.setPassword('password'); - await user.signUp(); - - const duplicateUser = new Parse.User(); - duplicateUser.setUsername(username); - duplicateUser.setPassword('password'); + await Parse.User.signUp(username, 'password'); try { - await duplicateUser.signUp(); - fail('Should have thrown an error'); + await Parse.User.signUp(username, 'password'); } catch (error) { expect(error.code).toBe(Parse.Error.USERNAME_TAKEN); - if (logLevel === 'warn') { - expect(warnSpy).toHaveBeenCalled(); - expect(infoSpy).not.toHaveBeenCalled(); - } else if (logLevel === 'info') { - expect(infoSpy).toHaveBeenCalled(); - expect(warnSpy).not.toHaveBeenCalled(); - } - expect(errorSpy).not.toHaveBeenCalled(); + expect(logger[logLevel]).toHaveBeenCalled(); + ['warn', 'error', 'info'].forEach(level => { + if (level !== logLevel) { + expect(logger[level]).not.toHaveBeenCalled(); + } + }); } - // Reset spies for the next iteration warnSpy.calls.reset(); errorSpy.calls.reset(); infoSpy.calls.reset(); From 6c21038cf2b5a69be7420453aebf923e4595791b Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sun, 13 Oct 2024 03:09:51 +0200 Subject: [PATCH 07/16] refactor --- spec/ParseUser.spec.js | 42 ++++++++++++++++++------------------------ spec/helper.js | 1 + src/RestWrite.js | 20 ++++++++++++++------ 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 082236bbce..04cbd9e670 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4431,15 +4431,10 @@ describe('allowClientClassCreation option', () => { }); }); -describe('custom log levels for user errors', () => { - it('should respect custom log levels for username already exists error', async () => { - const logger = require('../lib/logger').logger; - const warnSpy = spyOn(logger, 'warn').and.callThrough(); - const errorSpy = spyOn(logger, 'error').and.callThrough(); - const infoSpy = spyOn(logger, 'info').and.callThrough(); - - const logLevels = ['warn', 'info']; +fdescribe('log levels', () => { + const logLevels = ['warn', 'info', 'error']; + it_id('bd3929eb-85dd-4955-ac1d-5ba59ab1b9a3')(it)('should use log level for username already exists error', async () => { for (const logLevel of logLevels) { await reconfigureServer({ logLevels: { @@ -4447,24 +4442,23 @@ describe('custom log levels for user errors', () => { }, }); - const username = `existingUser_${logLevel}`; - await Parse.User.signUp(username, 'password'); + const logger = require('../lib/logger').logger; + spyOn(logger, 'warn').and.callFake(() => {}); + spyOn(logger, 'error').and.callFake(() => {}); + spyOn(logger, 'info').and.callFake(() => {}); - try { - await Parse.User.signUp(username, 'password'); - } catch (error) { - expect(error.code).toBe(Parse.Error.USERNAME_TAKEN); - expect(logger[logLevel]).toHaveBeenCalled(); - ['warn', 'error', 'info'].forEach(level => { - if (level !== logLevel) { - expect(logger[level]).not.toHaveBeenCalled(); - } - }); - } + await Parse.User.signUp('user', 'pass'); + await expectAsync(Parse.User.signUp('user', 'pass')).toBeRejectedWith( + new Parse.Error( + Parse.Error.USERNAME_TAKEN, + 'Account already exists for this username.' + ) + ); - warnSpy.calls.reset(); - errorSpy.calls.reset(); - infoSpy.calls.reset(); + logLevels.forEach(level => { + expect(logger[level]).toHaveBeenCalledTimes(level === logLevel ? 1 : 0); + logger[level].calls.reset(); + }); } }); }); diff --git a/spec/helper.js b/spec/helper.js index c09a0c4eb4..5702ac48f0 100644 --- a/spec/helper.js +++ b/spec/helper.js @@ -150,6 +150,7 @@ if (silent) { triggerAfter: 'silent', triggerBeforeError: 'silent', triggerBeforeSuccess: 'silent', + usernameAlreadyExists: 'silent', }; } diff --git a/src/RestWrite.js b/src/RestWrite.js index 08b718b091..7250a4463c 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -739,10 +739,14 @@ RestWrite.prototype._validateUserName = function () { ) .then(results => { if (results.length > 0) { - throw new Parse.Error( + const usernameError = new Parse.Error( Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.' ); + if (this.config.logLevels.usernameAlreadyExists !== 'silent') { + logger[this.config.logLevels.usernameAlreadyExists](JSON.stringify(usernameError)); + } + throw usernameError; } return; }); @@ -1561,10 +1565,10 @@ RestWrite.prototype.runDatabaseOperation = function () { Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.' ); - - return this.config.loggerController[this.config.logLevels.usernameAlreadyExists]( - JSON.stringify(usernameError) - ); + if (this.config.logLevels.usernameAlreadyExists !== 'silent') { + logger[this.config.logLevels.usernameAlreadyExists](JSON.stringify(usernameError)); + } + throw usernameError; } if (error && error.userInfo && error.userInfo.duplicated_field === 'email') { @@ -1589,10 +1593,14 @@ RestWrite.prototype.runDatabaseOperation = function () { ) .then(results => { if (results.length > 0) { - throw new Parse.Error( + const usernameError = new Parse.Error( Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.' ); + if (this.config.logLevels.usernameAlreadyExists !== 'silent') { + logger[this.config.logLevels.usernameAlreadyExists](JSON.stringify(usernameError)); + } + throw usernameError; } return this.config.database.find( this.className, From a557e78ff20bbdcadd336abaed81bf78b38a09cf Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sun, 13 Oct 2024 03:24:47 +0200 Subject: [PATCH 08/16] consider default level --- spec/ParseUser.spec.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 04cbd9e670..50ac0f2a26 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4432,7 +4432,7 @@ describe('allowClientClassCreation option', () => { }); fdescribe('log levels', () => { - const logLevels = ['warn', 'info', 'error']; + const logLevels = [undefined, 'silent', 'info', 'error']; it_id('bd3929eb-85dd-4955-ac1d-5ba59ab1b9a3')(it)('should use log level for username already exists error', async () => { for (const logLevel of logLevels) { @@ -4456,8 +4456,12 @@ fdescribe('log levels', () => { ); logLevels.forEach(level => { - expect(logger[level]).toHaveBeenCalledTimes(level === logLevel ? 1 : 0); - logger[level].calls.reset(); + if (level == 'silent') { + return; + } + const levelOrDefault = level || 'error'; + expect(logger[levelOrDefault]).toHaveBeenCalledTimes(level === logLevel ? 1 : 0); + logger[levelOrDefault].calls.reset(); }); } }); From 0a5aa4238bf7bdbfd1640de4015ab0ff2128459a Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sun, 13 Oct 2024 03:37:55 +0200 Subject: [PATCH 09/16] fix typo --- src/Options/Definitions.js | 2 +- src/Options/docs.js | 2 +- src/Options/index.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index 825fb84a6d..936a4cea5d 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -1120,7 +1120,7 @@ module.exports.LogLevels = { usernameAlreadyExists: { env: 'PARSE_SERVER_LOG_LEVELS_USERNAME_ALREADY_EXISTS', help: - 'Log level for the username already exists error when trying to sign up.Default is `error`.', + 'Log level for the username already exists error when trying to sign up. Default is `error`.', default: 'error', }, }; diff --git a/src/Options/docs.js b/src/Options/docs.js index c839834f05..ee351eaa8f 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -254,5 +254,5 @@ * @property {String} triggerAfter Log level used by the Cloud Code Triggers `afterSave`, `afterDelete`, `afterFind`, `afterLogout`. Default is `info`. * @property {String} triggerBeforeError Log level used by the Cloud Code Triggers `beforeSave`, `beforeDelete`, `beforeFind`, `beforeLogin` on error. Default is `error`. * @property {String} triggerBeforeSuccess Log level used by the Cloud Code Triggers `beforeSave`, `beforeDelete`, `beforeFind`, `beforeLogin` on success. Default is `info`. - * @property {String} usernameAlreadyExists Log level for the username already exists error when trying to sign up.Default is `error`. + * @property {String} usernameAlreadyExists Log level for the username already exists error when trying to sign up. Default is `error`. */ diff --git a/src/Options/index.js b/src/Options/index.js index 93c73371e9..68fa0fbca2 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -635,7 +635,7 @@ export interface LogLevels { :DEFAULT: error */ cloudFunctionError: ?string; - /* Log level for the username already exists error when trying to sign up.Default is `error`. + /* Log level for the username already exists error when trying to sign up. Default is `error`. :DEFAULT: error */ usernameAlreadyExists: ?string; From f537c2c03cb5bbbb3794c1fe132f724c5cc1eadb Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Sun, 13 Oct 2024 10:09:47 +0200 Subject: [PATCH 10/16] add warnjs Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com> --- spec/ParseUser.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 50ac0f2a26..9d73a0935e 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4432,7 +4432,7 @@ describe('allowClientClassCreation option', () => { }); fdescribe('log levels', () => { - const logLevels = [undefined, 'silent', 'info', 'error']; + const logLevels = [undefined, 'silent', 'info', 'warn', 'error']; it_id('bd3929eb-85dd-4955-ac1d-5ba59ab1b9a3')(it)('should use log level for username already exists error', async () => { for (const logLevel of logLevels) { From 5ec7c820fe460634f2f60625673a53e6185878f1 Mon Sep 17 00:00:00 2001 From: KrishDave1 Date: Tue, 15 Oct 2024 11:06:58 +0530 Subject: [PATCH 11/16] Removed the extra logger from RestWrite.js --- src/RestWrite.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/RestWrite.js b/src/RestWrite.js index 7250a4463c..b885954876 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -743,9 +743,6 @@ RestWrite.prototype._validateUserName = function () { Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.' ); - if (this.config.logLevels.usernameAlreadyExists !== 'silent') { - logger[this.config.logLevels.usernameAlreadyExists](JSON.stringify(usernameError)); - } throw usernameError; } return; From 386d2c13b13900c550a2759174109673e2e28e14 Mon Sep 17 00:00:00 2001 From: KrishDave1 Date: Wed, 16 Oct 2024 12:55:33 +0530 Subject: [PATCH 12/16] fix: Usernames will always be unique for testing --- spec/ParseUser.spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 9d73a0935e..1fe19cfea1 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4446,9 +4446,10 @@ fdescribe('log levels', () => { spyOn(logger, 'warn').and.callFake(() => {}); spyOn(logger, 'error').and.callFake(() => {}); spyOn(logger, 'info').and.callFake(() => {}); + const uniqueUsername = `user_${Date.now()}`; - await Parse.User.signUp('user', 'pass'); - await expectAsync(Parse.User.signUp('user', 'pass')).toBeRejectedWith( + await Parse.User.signUp(uniqueUsername, 'pass'); + await expectAsync(Parse.User.signUp(uniqueUsername, 'pass')).toBeRejectedWith( new Parse.Error( Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.' From 3587fb802cc305b229e2c6f606f7d9e816479865 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Wed, 16 Oct 2024 12:34:34 +0200 Subject: [PATCH 13/16] un-fdescribe Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com> --- spec/ParseUser.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 1fe19cfea1..4c1e4195bc 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4431,7 +4431,7 @@ describe('allowClientClassCreation option', () => { }); }); -fdescribe('log levels', () => { +describe('log levels', () => { const logLevels = [undefined, 'silent', 'info', 'warn', 'error']; it_id('bd3929eb-85dd-4955-ac1d-5ba59ab1b9a3')(it)('should use log level for username already exists error', async () => { From 9f3c82a27a418769929cb2ebd7c3a47d25b7a7a1 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Wed, 16 Oct 2024 15:11:46 +0200 Subject: [PATCH 14/16] fit --- spec/ParseUser.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 4c1e4195bc..16a00375a8 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4434,7 +4434,7 @@ describe('allowClientClassCreation option', () => { describe('log levels', () => { const logLevels = [undefined, 'silent', 'info', 'warn', 'error']; - it_id('bd3929eb-85dd-4955-ac1d-5ba59ab1b9a3')(it)('should use log level for username already exists error', async () => { + it_id('bd3929eb-85dd-4955-ac1d-5ba59ab1b9a3')(fit)('should use log level for username already exists error', async () => { for (const logLevel of logLevels) { await reconfigureServer({ logLevels: { From f1bc586d1440f3fd8b590ca7c42a51120955fa53 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Wed, 16 Oct 2024 15:45:46 +0200 Subject: [PATCH 15/16] refactor --- spec/ParseUser.spec.js | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 16a00375a8..41a9a5d441 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -4432,22 +4432,23 @@ describe('allowClientClassCreation option', () => { }); describe('log levels', () => { - const logLevels = [undefined, 'silent', 'info', 'warn', 'error']; + const logLevels = ['info', 'warn', 'error']; + const testLogLevels = ['info'];//[undefined, 'silent', 'info', 'warn', 'error']; it_id('bd3929eb-85dd-4955-ac1d-5ba59ab1b9a3')(fit)('should use log level for username already exists error', async () => { - for (const logLevel of logLevels) { + for (const testLogLevel of testLogLevels) { await reconfigureServer({ logLevels: { - usernameAlreadyExists: logLevel, + usernameAlreadyExists: testLogLevel, }, }); + // Set up logger spies const logger = require('../lib/logger').logger; - spyOn(logger, 'warn').and.callFake(() => {}); - spyOn(logger, 'error').and.callFake(() => {}); - spyOn(logger, 'info').and.callFake(() => {}); - const uniqueUsername = `user_${Date.now()}`; + logLevels.forEach(level => spyOn(logger, level).and.callFake(() => {})); + // Invoke error + const uniqueUsername = `user_${Date.now()}`; await Parse.User.signUp(uniqueUsername, 'pass'); await expectAsync(Parse.User.signUp(uniqueUsername, 'pass')).toBeRejectedWith( new Parse.Error( @@ -4456,13 +4457,11 @@ describe('log levels', () => { ) ); + // Verify log outputs logLevels.forEach(level => { - if (level == 'silent') { - return; - } - const levelOrDefault = level || 'error'; - expect(logger[levelOrDefault]).toHaveBeenCalledTimes(level === logLevel ? 1 : 0); - logger[levelOrDefault].calls.reset(); + const levelOrDefault = testLogLevel || 'error'; + expect(logger[level]).toHaveBeenCalledTimes(level === levelOrDefault ? 1 : 0); + logger[level].calls.reset(); }); } }); From 2a03838236a3edbecbbd61ec6fc823bd3d4be01b Mon Sep 17 00:00:00 2001 From: KrishDave1 Date: Wed, 16 Oct 2024 19:42:37 +0530 Subject: [PATCH 16/16] Check again for info --- src/RestWrite.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/RestWrite.js b/src/RestWrite.js index b885954876..0446b96b1e 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1562,8 +1562,12 @@ RestWrite.prototype.runDatabaseOperation = function () { Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.' ); - if (this.config.logLevels.usernameAlreadyExists !== 'silent') { - logger[this.config.logLevels.usernameAlreadyExists](JSON.stringify(usernameError)); + if (this.config.logLevels.usernameAlreadyExists === 'info') { + logger.info(JSON.stringify(usernameError)); + } else if (this.config.logLevels.usernameAlreadyExists === 'warn') { + logger.warn(JSON.stringify(usernameError)); + } else if (this.config.logLevels.usernameAlreadyExists === 'error') { + logger.error(JSON.stringify(usernameError)); } throw usernameError; }