-
Notifications
You must be signed in to change notification settings - Fork 18
Apigee (new) #579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Apigee (new) #579
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a new Apigee integration package, including authentication, service metadata, quota management, and multiple components for managing Apigee organizations, environments, key-value maps, and IP block entries. It also adds supporting utility libraries, component definitions, and test suites. Minor adjustments are made to Apify authentication and dataset-fetching components. Changes
Suggested labels
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 21
🧹 Nitpick comments (17)
src/appmixer/apigee/lib.generated.js (1)
25-31
: Duplicate comment – minor cleanupLine 25 repeats “One by one.” twice; trim the redundancy.
src/appmixer/apify/auth.js (1)
26-34
: Good switch fromuri
→url
; consider moving token to headerChange aligns with common Axios/fetch options – nice.
Security note: the API token is still sent as a query parameter, which can be logged by proxies. If Apify supports it, prefer
Authorization: Bearer {{apiToken}}
to avoid leaking credentials.src/appmixer/apigee/README.md (1)
1-4
: Wrap bare URL in markdown to silence MD034 and improve readability
Markdown-lint flags the naked URL. Enclose it in angle brackets or make it an explicit link.-TEST_SERVER_URL=http://localhost:2200 appmixer … +TEST_SERVER_URL=<http://localhost:2200> appmixer …🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1-1: Bare URL used
null(MD034, no-bare-urls)
1-1: Bare URL used
null(MD034, no-bare-urls)
src/appmixer/apigee/core/ListKeyValueMaps/ListKeyValueMaps.js (2)
6-7
: Validate required parameters before firing the request
org
orenv
being undefined will currently yield a hard-to-debug 404 from Apigee. Guard early and surface a clear error.-const { org, env } = context.messages.in.content; +const { org, env } = context.messages.in.content || {}; +if (!org || !env) { + throw new Error('Both "org" and "env" parameters are required.'); +}
12-14
: Header-key quoting inconsistent with sibling modules
Here the key is unquoted while inListEnvironments.js
it is quoted. Pick one style across the package for consistency.- headers: { - Authorization: `Bearer ${context.auth.accessToken}` - } + headers: { + 'Authorization': `Bearer ${context.auth.accessToken}` + }src/appmixer/apigee/core/ListEnvironments/ListEnvironments.js (3)
6-7
: Parameter presence check recommended
Same rationale as forListKeyValueMaps
; fail fast whenorg
is missing to avoid opaque remote errors.
9-15
: Potential pagination not handled
The environments list also supports pagination (pageSize
,pageToken
). Decide whether to auto-paginate or at least document the limitation.
12-14
: Quote the Authorization header for uniformity
Match the quoting style used in other files.test/apigee/listEnvironments.test.js (1)
30-32
:sendJson
mock should mirror actual signature
Production signature is(data, port)
. Include theport
parameter to avoid surprises if logic branches on it later.-context.sendJson = (data) => { +context.sendJson = (data, _port) => { result = data; };src/appmixer/apify/crawlers/FetchDatasets/FetchDatasets.js (2)
1-1
: Insert'use strict';
for consistencyAll other Appmixer components start with the directive and guidelines explicitly state it is desirable.
Add it before the first statement to keep the style uniform.+'use strict'; + const lib = require('../../lib.generated');
4-4
:desc
is extracted but never usedLeaving an unused variable will trigger linters and confuses readers.
- const { desc } = context.messages.in.content; + // No input parameters required at the momentsrc/appmixer/apigee/core/FindEntries/FindEntries.js (1)
4-4
: Unusedlib
import
lib
is required but never referenced; drop it to avoid dead code.-const lib = require('../../lib.generated');
test/apigee/listOrganizations.test.js (1)
35-35
: Remove debugging output
console.log
in unit tests pollutes CI logs.- console.log(result);
src/appmixer/apigee/core/RemoveEntry/RemoveEntry.js (1)
4-4
: Unusedlib
import
lib
isn’t referenced; remove to keep the file lean.-const lib = require('../../lib.generated');
src/appmixer/apigee/core/AddEntry/AddEntry.js (1)
3-3
:lib
is imported but never used
lib
is required yet no symbol from it is referenced in this module. Drop the import to keep the file lean or use it if there is planned functionality.-const lib = require('../../lib.generated'); +// const lib = require('../../lib.generated'); // uncomment when helpers are neededsrc/appmixer/apigee/core/ListEnvironments/component.json (1)
39-43
: Consider defining an explicit output schema
outPorts
lacks a schema definition, which weakens type-safety and inspector UX. Adding at least a minimal array schema (e.g.{"type":"array","items":{"type":"string"}}
) makes the component integration clearer.src/appmixer/apigee/core/FindEntries/component.json (1)
1-3
: Add unit tests for this component.
No corresponding test file exists undertest/apigee
.Cover scenarios for empty maps, multiple entries, and error conditions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
package.json
(1 hunks)src/appmixer/apify/auth.js
(1 hunks)src/appmixer/apify/crawlers/FetchDatasets/FetchDatasets.js
(1 hunks)src/appmixer/apify/crawlers/FetchDatasets/component.json
(1 hunks)src/appmixer/apigee/README.md
(1 hunks)src/appmixer/apigee/auth.js
(1 hunks)src/appmixer/apigee/bundle.json
(1 hunks)src/appmixer/apigee/core/AddEntry/AddEntry.js
(1 hunks)src/appmixer/apigee/core/AddEntry/component.json
(1 hunks)src/appmixer/apigee/core/FindBlockingEntries/FindBlockingEntries.js
(1 hunks)src/appmixer/apigee/core/FindBlockingEntries/component.json
(1 hunks)src/appmixer/apigee/core/FindEntries/FindEntries.js
(1 hunks)src/appmixer/apigee/core/FindEntries/component.json
(1 hunks)src/appmixer/apigee/core/ListEnvironments/ListEnvironments.js
(1 hunks)src/appmixer/apigee/core/ListEnvironments/component.json
(1 hunks)src/appmixer/apigee/core/ListKeyValueMaps/ListKeyValueMaps.js
(1 hunks)src/appmixer/apigee/core/ListKeyValueMaps/component.json
(1 hunks)src/appmixer/apigee/core/ListOrganizations/ListOrganizations.js
(1 hunks)src/appmixer/apigee/core/ListOrganizations/component.json
(1 hunks)src/appmixer/apigee/core/RemoveEntry/RemoveEntry.js
(1 hunks)src/appmixer/apigee/core/RemoveEntry/component.json
(1 hunks)src/appmixer/apigee/lib.generated.js
(1 hunks)src/appmixer/apigee/quota.js
(1 hunks)src/appmixer/apigee/service.json
(1 hunks)test/apigee/listEnvironments.test.js
(1 hunks)test/apigee/listOrganizations.test.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`src/appmixer/**/*.js`: Not necessary to add try/catch blocks in their `receive` function. Appmixer engine automatically handles any exceptions that originate in these async functi...
src/appmixer/**/*.js
: Not necessary to add try/catch blocks in theirreceive
function. Appmixer engine automatically handles any exceptions that originate in these async functions.The directive
"use strict";
is desirable here
src/appmixer/apify/auth.js
src/appmixer/apigee/core/ListEnvironments/ListEnvironments.js
src/appmixer/apigee/quota.js
src/appmixer/apigee/core/ListOrganizations/ListOrganizations.js
src/appmixer/apigee/core/RemoveEntry/RemoveEntry.js
src/appmixer/apigee/core/ListKeyValueMaps/ListKeyValueMaps.js
src/appmixer/apigee/core/FindEntries/FindEntries.js
src/appmixer/apify/crawlers/FetchDatasets/FetchDatasets.js
src/appmixer/apigee/core/AddEntry/AddEntry.js
src/appmixer/apigee/core/FindBlockingEntries/FindBlockingEntries.js
src/appmixer/apigee/lib.generated.js
src/appmixer/apigee/auth.js
`**/*/bundle.json`: Make sure `version` also matches the last entry in `changelog`
**/*/bundle.json
: Make sureversion
also matches the last entry inchangelog
src/appmixer/apigee/bundle.json
🧬 Code Graph Analysis (6)
src/appmixer/apigee/core/ListEnvironments/ListEnvironments.js (2)
src/appmixer/apigee/core/ListKeyValueMaps/ListKeyValueMaps.js (2)
context
(6-6)context
(9-15)src/appmixer/apigee/core/ListOrganizations/ListOrganizations.js (1)
context
(7-13)
src/appmixer/apigee/core/RemoveEntry/RemoveEntry.js (3)
src/appmixer/apigee/core/AddEntry/AddEntry.js (2)
lib
(3-3)context
(17-23)src/appmixer/apigee/core/FindBlockingEntries/FindBlockingEntries.js (3)
lib
(4-4)context
(10-10)context
(17-23)src/appmixer/apigee/core/FindEntries/FindEntries.js (3)
lib
(4-4)context
(8-8)context
(11-17)
src/appmixer/apigee/core/ListKeyValueMaps/ListKeyValueMaps.js (1)
src/appmixer/apigee/core/ListEnvironments/ListEnvironments.js (2)
context
(6-6)context
(9-15)
test/apigee/listOrganizations.test.js (1)
src/appmixer/apigee/core/ListOrganizations/ListOrganizations.js (1)
context
(7-13)
src/appmixer/apigee/core/AddEntry/AddEntry.js (3)
src/appmixer/apigee/core/FindBlockingEntries/FindBlockingEntries.js (3)
lib
(4-4)context
(10-10)context
(17-23)src/appmixer/apigee/core/RemoveEntry/RemoveEntry.js (3)
lib
(4-4)context
(8-8)context
(11-17)src/appmixer/apigee/auth.js (2)
data
(56-62)data
(82-87)
src/appmixer/apigee/core/FindBlockingEntries/FindBlockingEntries.js (3)
src/appmixer/apigee/core/AddEntry/AddEntry.js (2)
lib
(3-3)context
(17-23)src/appmixer/apigee/core/FindEntries/FindEntries.js (3)
lib
(4-4)context
(8-8)context
(11-17)src/appmixer/apigee/core/RemoveEntry/RemoveEntry.js (3)
lib
(4-4)context
(8-8)context
(11-17)
🪛 markdownlint-cli2 (0.17.2)
src/appmixer/apigee/README.md
1-1: Bare URL used
null
(MD034, no-bare-urls)
1-1: Bare URL used
null
(MD034, no-bare-urls)
🪛 Biome (1.9.4)
src/appmixer/apigee/core/ListEnvironments/ListEnvironments.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/apigee/quota.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/apigee/core/ListOrganizations/ListOrganizations.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/apigee/core/RemoveEntry/RemoveEntry.js
[error] 1-2: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/apigee/core/ListKeyValueMaps/ListKeyValueMaps.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/apigee/core/FindEntries/FindEntries.js
[error] 1-2: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/apigee/core/AddEntry/AddEntry.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/apigee/core/FindBlockingEntries/FindBlockingEntries.js
[error] 1-2: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/apigee/auth.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🔇 Additional comments (7)
package.json (1)
14-14
:✅ Verification successful
Confirm
dotenv
is only used in test code; promote if used at runtime
dotenv
is declared underdevDependencies
, which is correct as long as it’s only required by the test suite.
If any production modules load it (e.g. viarequire('dotenv').config()
), it must move to"dependencies"
or those imports will break in production deployments where devDeps aren’t installed.Run a quick search to ensure no non-test file imports
dotenv
:
🏁 Script executed:
#!/bin/bash rg --glob '!test/**' --glob '!**/*.spec.js' --glob '!**/*.test.js' $'require[(]..dotenv.' || echo "✅ no runtime usage"Length of output: 137
dotenv usage verified – safe to keep as devDependency
No non-test files importdotenv
; it’s only used in your test suite, so leaving it underdevDependencies
is correct.src/appmixer/apigee/quota.js (1)
1-13
:'use strict'
is fine – ignore the lint false-positiveThe file is CommonJS, not an ES module. CommonJS isn’t strict by default, so keeping
'use strict';
is legitimate. No changes required.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
src/appmixer/apigee/service.json (1)
1-8
: Metadata JSON LGTMThe descriptor is syntactically valid and complete. Nice job embedding the icon to avoid extra assets.
src/appmixer/apigee/bundle.json (1)
1-8
: Version & changelog are correctly aligned – good catch
Theversion
field matches the last key inchangelog
, satisfying the repository guideline forbundle.json
files.src/appmixer/apigee/core/ListOrganizations/ListOrganizations.js (1)
11-11
: Token property differs from other componentsHere we use
accessToken
, elsewhere (FindEntries
,RemoveEntry
) useapiToken
. Standardise the field name across Apigee components to avoid integration bugs.src/appmixer/apigee/core/ListOrganizations/component.json (1)
7-10
: Component configuration aligns with authentication requirements.
Theauth
section correctly specifies both theservice
and required OAuthscope
.src/appmixer/apigee/auth.js (1)
1-1
:'use strict';
should stay – guideline overrides the linter noiseAppmixer’s own coding-guideline explicitly asks for
"use strict";
.
Ignore Biome’s “redundant” warning; this file is CommonJS and strict mode is desirable.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
const componentName = context.flowDescriptor[context.componentId].label || context.componentId; | ||
const fileName = `${context.config.outputFilePrefix || DEFAULT_PREFIX}-${componentName}.csv`; | ||
const savedFile = await context.saveFileStream(pathModule.normalize(fileName), buffer); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Potential path-traversal via componentName
componentName
is taken from user-configurable flowDescriptor
labels. If a label contains ../
or absolute paths, pathModule.normalize
could escape the intended directory.
Mitigate by stripping path separators:
-const fileName = `${context.config.outputFilePrefix || DEFAULT_PREFIX}-${componentName}.csv`;
+const safeName = componentName.replace(/[\\\/]/g, '_');
+const fileName = `${context.config.outputFilePrefix || DEFAULT_PREFIX}-${safeName}.csv`;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const componentName = context.flowDescriptor[context.componentId].label || context.componentId; | |
const fileName = `${context.config.outputFilePrefix || DEFAULT_PREFIX}-${componentName}.csv`; | |
const savedFile = await context.saveFileStream(pathModule.normalize(fileName), buffer); | |
const componentName = context.flowDescriptor[context.componentId].label || context.componentId; | |
const safeName = componentName.replace(/[\\\/]/g, '_'); | |
const fileName = `${context.config.outputFilePrefix || DEFAULT_PREFIX}-${safeName}.csv`; | |
const savedFile = await context.saveFileStream(pathModule.normalize(fileName), buffer); |
🤖 Prompt for AI Agents
In src/appmixer/apigee/lib.generated.js around lines 41 to 44, the componentName
is derived from user-configurable labels and may contain path traversal
characters like ../ which pathModule.normalize won't fully prevent. To fix this,
sanitize componentName by removing or replacing any path separators or traversal
sequences before using it to build the fileName, ensuring the final path cannot
escape the intended directory.
} else if (outputType === 'file') { | ||
|
||
// Into CSV file. | ||
const csvString = toCsv(records); | ||
|
||
let buffer = Buffer.from(csvString, 'utf8'); | ||
const componentName = context.flowDescriptor[context.componentId].label || context.componentId; | ||
const fileName = `${context.config.outputFilePrefix || DEFAULT_PREFIX}-${componentName}.csv`; | ||
const savedFile = await context.saveFileStream(pathModule.normalize(fileName), buffer); | ||
|
||
await context.log({ step: 'File was saved', fileName, fileId: savedFile.fileId }); | ||
await context.sendJson({ fileId: savedFile.fileId }, outputPortName); | ||
} else { | ||
throw new context.CancelError('Unsupported outputType ' + outputType); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty‐records path crashes for outputType === "file"
toCsv(records)
blindly accesses records[0]
; passing an empty array causes TypeError: Cannot convert undefined or null to object
.
Add the same empty-array guard used for "first"
:
} else if (outputType === 'file') {
- // Into CSV file.
- const csvString = toCsv(records);
+ if (records.length === 0) {
+ throw new context.CancelError('No records available for file output type');
+ }
+ // Into CSV file.
+ const csvString = toCsv(records);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
} else if (outputType === 'file') { | |
// Into CSV file. | |
const csvString = toCsv(records); | |
let buffer = Buffer.from(csvString, 'utf8'); | |
const componentName = context.flowDescriptor[context.componentId].label || context.componentId; | |
const fileName = `${context.config.outputFilePrefix || DEFAULT_PREFIX}-${componentName}.csv`; | |
const savedFile = await context.saveFileStream(pathModule.normalize(fileName), buffer); | |
await context.log({ step: 'File was saved', fileName, fileId: savedFile.fileId }); | |
await context.sendJson({ fileId: savedFile.fileId }, outputPortName); | |
} else { | |
throw new context.CancelError('Unsupported outputType ' + outputType); | |
} | |
} else if (outputType === 'file') { | |
if (records.length === 0) { | |
throw new context.CancelError('No records available for file output type'); | |
} | |
// Into CSV file. | |
const csvString = toCsv(records); | |
let buffer = Buffer.from(csvString, 'utf8'); | |
const componentName = context.flowDescriptor[context.componentId].label || context.componentId; | |
const fileName = `${context.config.outputFilePrefix || DEFAULT_PREFIX}-${componentName}.csv`; | |
const savedFile = await context.saveFileStream(pathModule.normalize(fileName), buffer); | |
await context.log({ step: 'File was saved', fileName, fileId: savedFile.fileId }); | |
await context.sendJson({ fileId: savedFile.fileId }, outputPortName); | |
} else { | |
throw new context.CancelError('Unsupported outputType ' + outputType); | |
} |
🤖 Prompt for AI Agents
In src/appmixer/apigee/lib.generated.js between lines 35 and 49, the code calls
toCsv(records) without checking if records is empty, causing a TypeError when
records is an empty array. Add a guard to check if records is empty before
calling toCsv, similar to the existing guard used for the "first" outputType. If
records is empty, handle that case appropriately to avoid calling toCsv with an
empty array.
const { data } = await context.httpRequest({ | ||
method: 'GET', | ||
url: `https://apigee.googleapis.com/v1/organizations/${org}/environments/${env}/keyvaluemaps`, | ||
headers: { | ||
Authorization: `Bearer ${context.auth.accessToken}` | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
API is paginated – component returns only the first page
/keyvaluemaps
supports pageSize
& pageToken
; the response may contain nextPageToken
. Consider iterating until token is empty or expose pagination controls to the user so data isn’t silently truncated.
🤖 Prompt for AI Agents
In src/appmixer/apigee/core/ListKeyValueMaps/ListKeyValueMaps.js around lines 9
to 15, the API call to fetch keyvaluemaps only retrieves the first page of
results, ignoring pagination. To fix this, implement a loop that repeatedly
calls the API using the nextPageToken from the response until no more tokens
remain, aggregating all pages of data before returning. Alternatively, modify
the component to expose pagination controls so users can request additional
pages explicitly.
it('should handle parameters.ADD with empty object', async () => { | ||
|
||
let result; | ||
context.sendJson = (data) => { | ||
result = data; | ||
}; | ||
context.messages = { | ||
in: { | ||
content: { | ||
org: 'new-edge-team', | ||
outputType: 'array' | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Test description is misleading & may hit live network
- Title says
parameters.ADD
which is unrelated. axios
is passed straight through; unless you stub Apigee endpoints, the test will perform real HTTP calls and become flaky. Swapaxios
for a nock/fetch-mock stub or inject a fakehttpRequest
.
🤖 Prompt for AI Agents
In test/apigee/listEnvironments.test.js around lines 27 to 38, the test title
incorrectly references parameters.ADD which is unrelated and misleading; update
the test description to accurately reflect what is being tested. Also, replace
the direct use of axios with a stub or mock HTTP client like nock or fetch-mock
to prevent real network calls during testing, or inject a fake httpRequest to
ensure the test is isolated and reliable.
const component = require('../../src/appmixer/apigee/core/ListEnvironments/ListEnvironments'); | ||
const { createMockContext } = require('../utils'); | ||
|
||
dotenv.config({ path: __dirname + '../../.env' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.env
path concatenation is missing a path separator
__dirname + '../../.env'
produces /tests/apigee../.env
. Use path.join
.
-dotenv.config({ path: __dirname + '../../.env' });
+const path = require('path');
+dotenv.config({ path: path.join(__dirname, '../../.env') });
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
dotenv.config({ path: __dirname + '../../.env' }); | |
const path = require('path'); | |
dotenv.config({ path: path.join(__dirname, '../../.env') }); |
🤖 Prompt for AI Agents
In test/apigee/listEnvironments.test.js at line 8, the dotenv config path
concatenation is incorrect because it lacks a path separator, causing an invalid
path. Replace the string concatenation with path.join(__dirname, '../../.env')
to correctly construct the path to the .env file.
"mapname": { | ||
"type": "string" | ||
}, | ||
"outputType": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent property naming: use mapName
instead of mapname
.
Align this key with other Apigee components (RemoveEntry
, FindEntries
) for schema consistency.
"properties": {
- "mapname": {
- "type": "string"
- },
+ "mapName": {
+ "type": "string"
+ },
"outputType": {
"type": "string"
}
}
Remember to update references in the inspector
and any downstream code.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"mapname": { | |
"type": "string" | |
}, | |
"outputType": { | |
"mapName": { | |
"type": "string" | |
}, | |
"outputType": { | |
"type": "string" | |
} |
🤖 Prompt for AI Agents
In src/appmixer/apigee/core/FindBlockingEntries/component.json around lines 26
to 29, rename the property key from "mapname" to "mapName" to maintain
consistent camelCase naming with other Apigee components. After updating the
key, ensure all references to this property in the inspector and any downstream
code are also updated to use "mapName" to avoid breaking changes.
"organization": { | ||
"type": "string" | ||
}, | ||
"environment": { | ||
"type": "string" | ||
}, | ||
"mapName": { | ||
"type": "string" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent input property names across components.
Other Apigee actions use org
and env
– here you use full words. Standardize to one convention for uniformity:
"properties": {
- "organization": {"type": "string"},
- "environment": {"type": "string"},
+ "org": {"type": "string"},
+ "env": {"type": "string"},
"mapName": {"type": "string"}
}
Don’t forget to adjust the inspector
labels and any implementation code accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"organization": { | |
"type": "string" | |
}, | |
"environment": { | |
"type": "string" | |
}, | |
"mapName": { | |
"type": "string" | |
} | |
"org": { | |
"type": "string" | |
}, | |
"env": { | |
"type": "string" | |
}, | |
"mapName": { | |
"type": "string" | |
} |
🤖 Prompt for AI Agents
In src/appmixer/apigee/core/FindEntries/component.json around lines 20 to 28,
the input property names "organization" and "environment" are inconsistent with
other Apigee actions that use "org" and "env". Rename these properties to "org"
and "env" respectively for consistency. Also update the "inspector" labels and
any related implementation code to reflect these new property names to maintain
uniformity across components.
authUrl: function(context) { | ||
const params = new URLSearchParams({ | ||
client_id: initData.clientId, | ||
redirect_uri: context.callbackUrl, | ||
response_type: 'code', | ||
scope: context.scope.join(' '), | ||
state: context.ticket, | ||
access_type: 'offline', | ||
approval_prompt: 'force' | ||
}).toString(); | ||
|
||
return `https://accounts.google.com/o/oauth2/auth?${params}`; | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Google OAuth query param approval_prompt
is deprecated – switch to prompt=consent
approval_prompt
was removed from Google OAuth 2.0 spec years ago. Use the modern prompt
param to avoid 400 errors on some tenants.
- approval_prompt: 'force'
+ prompt: 'consent'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
authUrl: function(context) { | |
const params = new URLSearchParams({ | |
client_id: initData.clientId, | |
redirect_uri: context.callbackUrl, | |
response_type: 'code', | |
scope: context.scope.join(' '), | |
state: context.ticket, | |
access_type: 'offline', | |
approval_prompt: 'force' | |
}).toString(); | |
return `https://accounts.google.com/o/oauth2/auth?${params}`; | |
}, | |
authUrl: function(context) { | |
const params = new URLSearchParams({ | |
client_id: initData.clientId, | |
redirect_uri: context.callbackUrl, | |
response_type: 'code', | |
scope: context.scope.join(' '), | |
state: context.ticket, | |
access_type: 'offline', | |
prompt: 'consent' | |
}).toString(); | |
return `https://accounts.google.com/o/oauth2/auth?${params}`; | |
}, |
🤖 Prompt for AI Agents
In src/appmixer/apigee/auth.js between lines 24 and 36, replace the deprecated
Google OAuth query parameter 'approval_prompt' with the current 'prompt'
parameter set to 'consent'. Update the URLSearchParams object by removing
'approval_prompt: force' and adding 'prompt: consent' to ensure compatibility
and prevent 400 errors.
refreshAccessToken: async function(context) { | ||
|
||
const data = { | ||
client_id: initData.clientId, | ||
client_secret: initData.clientSecret, | ||
refresh_token: context.refreshToken, | ||
grant_type: 'refresh_token' | ||
}; | ||
|
||
const response = await context.httpRequest({ | ||
method: 'POST', | ||
url: 'https://oauth2.googleapis.com/token', | ||
headers: { | ||
'Content-Type': 'application/x-www-form-urlencoded' | ||
}, | ||
data | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Same form-encoding issue in refreshAccessToken
- const data = {
- client_id: initData.clientId,
- client_secret: initData.clientSecret,
- refresh_token: context.refreshToken,
- grant_type: 'refresh_token'
- };
+ const data = new URLSearchParams({
+ client_id: initData.clientId,
+ client_secret: initData.clientSecret,
+ refresh_token: context.refreshToken,
+ grant_type: 'refresh_token'
+ }).toString();
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
refreshAccessToken: async function(context) { | |
const data = { | |
client_id: initData.clientId, | |
client_secret: initData.clientSecret, | |
refresh_token: context.refreshToken, | |
grant_type: 'refresh_token' | |
}; | |
const response = await context.httpRequest({ | |
method: 'POST', | |
url: 'https://oauth2.googleapis.com/token', | |
headers: { | |
'Content-Type': 'application/x-www-form-urlencoded' | |
}, | |
data | |
}); | |
refreshAccessToken: async function(context) { | |
- const data = { | |
- client_id: initData.clientId, | |
- client_secret: initData.clientSecret, | |
- refresh_token: context.refreshToken, | |
- grant_type: 'refresh_token' | |
- }; | |
+ const data = new URLSearchParams({ | |
+ client_id: initData.clientId, | |
+ client_secret: initData.clientSecret, | |
+ refresh_token: context.refreshToken, | |
+ grant_type: 'refresh_token' | |
+ }).toString(); | |
const response = await context.httpRequest({ | |
method: 'POST', | |
url: 'https://oauth2.googleapis.com/token', | |
headers: { | |
'Content-Type': 'application/x-www-form-urlencoded' | |
}, | |
data | |
}); |
🤖 Prompt for AI Agents
In src/appmixer/apigee/auth.js between lines 80 and 96, the refreshAccessToken
function sends the token refresh request with data as a JSON object, but the
Content-Type header is 'application/x-www-form-urlencoded'. To fix this, convert
the data object into a URL-encoded string format before sending it in the
request body, ensuring it matches the specified content type.
const data = { | ||
code: context.authorizationCode, | ||
client_id: initData.clientId, | ||
client_secret: initData.clientSecret, | ||
redirect_uri: context.callbackUrl, | ||
grant_type: 'authorization_code' | ||
}; | ||
|
||
const response = await context.httpRequest({ | ||
method: 'POST', | ||
url: 'https://oauth2.googleapis.com/token', | ||
headers: { | ||
'Content-Type': 'application/x-www-form-urlencoded' | ||
}, | ||
data | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Body is sent as object while header claims application/x-www-form-urlencoded
context.httpRequest
(axios-style) will JSON-encode plain objects, yielding a 400
from Google because the header and body mismatch. Encode with URLSearchParams
(or drop the header and let axios form-encode).
- const data = {
- code: context.authorizationCode,
- client_id: initData.clientId,
- client_secret: initData.clientSecret,
- redirect_uri: context.callbackUrl,
- grant_type: 'authorization_code'
- };
+ const data = new URLSearchParams({
+ code: context.authorizationCode,
+ client_id: initData.clientId,
+ client_secret: initData.clientSecret,
+ redirect_uri: context.callbackUrl,
+ grant_type: 'authorization_code'
+ }).toString();
Same remark for refreshAccessToken
below.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const data = { | |
code: context.authorizationCode, | |
client_id: initData.clientId, | |
client_secret: initData.clientSecret, | |
redirect_uri: context.callbackUrl, | |
grant_type: 'authorization_code' | |
}; | |
const response = await context.httpRequest({ | |
method: 'POST', | |
url: 'https://oauth2.googleapis.com/token', | |
headers: { | |
'Content-Type': 'application/x-www-form-urlencoded' | |
}, | |
data | |
}); | |
const data = new URLSearchParams({ | |
code: context.authorizationCode, | |
client_id: initData.clientId, | |
client_secret: initData.clientSecret, | |
redirect_uri: context.callbackUrl, | |
grant_type: 'authorization_code' | |
}).toString(); | |
const response = await context.httpRequest({ | |
method: 'POST', | |
url: 'https://oauth2.googleapis.com/token', | |
headers: { | |
'Content-Type': 'application/x-www-form-urlencoded' | |
}, | |
data | |
}); |
🤖 Prompt for AI Agents
In src/appmixer/apigee/auth.js around lines 56 to 72, the request body is sent
as a plain object while the Content-Type header is set to
'application/x-www-form-urlencoded', causing a mismatch and a 400 error from
Google. To fix this, convert the data object to a URLSearchParams instance
before sending it in the httpRequest call. Apply the same fix to the
refreshAccessToken function where a similar pattern occurs.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores