Open
Conversation
Contributor
Reviewer's GuideRefactors mock OAuth2 and payment servers into reusable seedwork packages and relocates mock servers into dedicated apps, while renaming core backend/api/docs/ui packages from @sthrift/* to @app/* and standardizing payment/infra packages under @cellix/* with corresponding tooling and documentation updates. Sequence diagram for starting the mock OAuth2 server via app entrypointsequenceDiagram
participant Dev as Developer
participant MockOAuth2App as mock_oauth2_app_index_ts
participant Env as setupEnvironment
participant Config as OAuth2Config_builder
participant Seed as startMockOAuth2Server
participant Express as ExpressServer
Dev->>MockOAuth2App: node dist/src/index.js
MockOAuth2App->>Env: setupEnvironment()
Env-->>MockOAuth2App: environment loaded from .env and .env.local
MockOAuth2App->>Config: resolvePathsAndCerts(projectRoot, port, env)
Config-->>MockOAuth2App: OAuth2Config instance
MockOAuth2App->>Seed: startMockOAuth2Server(config)
Seed->>Express: configureRoutesAndSecurity(config)
Express-->>Seed: serverInstance
Seed-->>MockOAuth2App: Promise resolved
MockOAuth2App-->>Dev: mock OAuth2 server listening at config.baseUrl
Sequence diagram for starting the mock payment server via app entrypointsequenceDiagram
participant Dev as Developer
participant MockPaymentApp as mock_payment_app_index_ts
participant Env as dotenv_setup
participant Config as PaymentConfig_builder
participant Seed as startMockPaymentServer
participant Express as ExpressServer
Dev->>MockPaymentApp: node dist/src/index.js
MockPaymentApp->>Env: dotenv.config()
Env-->>MockPaymentApp: environment loaded
MockPaymentApp->>Config: resolveProjectRootAndCerts(port, env)
Config-->>MockPaymentApp: PaymentConfig instance
MockPaymentApp->>Seed: startMockPaymentServer(config)
Seed->>Express: configurePaymentEndpoints(config)
Express-->>Seed: serverInstance
Seed-->>MockPaymentApp: Promise resolved
MockPaymentApp-->>Dev: mock payment server listening at paymentBaseUrl
Class diagram for OAuth2 mock server seedwork typesclassDiagram
class OAuth2Config {
+number port
+string baseUrl
+string host
+Set~string~ allowedRedirectUris
+string allowedRedirectUri
+Map~string,string~ redirectUriToAudience
+string certKeyPath
+boolean hasCerts
+string certPath
+NodeJS_ProcessEnv env
+getUserProfile(isAdminPortal boolean) UserProfile
}
class UserProfile {
+string email
+string given_name
+string family_name
}
class MockOAuth2SeedworkModule {
+startMockOAuth2Server(config OAuth2Config) Promise~void~
}
class MockOAuth2AppEntry {
+setupEnvironment() void
+main() void
}
OAuth2Config --> UserProfile : returns
MockOAuth2SeedworkModule --> OAuth2Config : uses
MockOAuth2AppEntry --> OAuth2Config : builds
MockOAuth2AppEntry --> MockOAuth2SeedworkModule : calls
Class diagram for payment mock server seedwork typesclassDiagram
class PaymentConfig {
+number port
+string protocol
+string host
+string paymentHost
+string frontendBaseUrl
+string paymentBaseUrl
+string certKeyPath
+string certPath
+string iframeJsPath
}
class MockPaymentSeedworkModule {
+startMockPaymentServer(config PaymentConfig) Promise~void~
}
class MockPaymentAppEntry {
+setupEnvironment() void
+main() void
}
class PaymentServiceTypes {
<<from @cellix/payment-service>>
CustomerProfile
PaymentTokenInfo
PaymentTransactionResponse
CustomerPaymentResponse
CustomerPaymentInstrumentResponse
CustomerPaymentInstrumentsResponse
TransactionReceipt
PlanCreation
PlanCreationResponse
PlansListResponse
PlanResponse
Subscription
SubscriptionResponse
SuspendSubscriptionResponse
SubscriptionsListResponse
PaymentInstrumentInfo
}
MockPaymentSeedworkModule --> PaymentConfig : uses
MockPaymentSeedworkModule --> PaymentServiceTypes : returns
MockPaymentAppEntry --> PaymentConfig : builds
MockPaymentAppEntry --> MockPaymentSeedworkModule : calls
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
@cellix/mock-payment-server-seedwork/src/index.ts,app.use(express.json())is registered twice; drop the duplicate call to avoid unnecessary middleware re-execution and keep the setup clearer. startMockOAuth2Serverin@cellix/mock-oauth2-server-seedworkcallsprocess.exit(1)on error while the app-level entrypoint also does its owncatch/process.exit, which makes the seedwork less reusable; consider letting the caller decide how to handle startup failures and remove theprocess.exitfrom the seedwork.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `@cellix/mock-payment-server-seedwork/src/index.ts`, `app.use(express.json())` is registered twice; drop the duplicate call to avoid unnecessary middleware re-execution and keep the setup clearer.
- `startMockOAuth2Server` in `@cellix/mock-oauth2-server-seedwork` calls `process.exit(1)` on error while the app-level entrypoint also does its own `catch`/`process.exit`, which makes the seedwork less reusable; consider letting the caller decide how to handle startup failures and remove the `process.exit` from the seedwork.
## Individual Comments
### Comment 1
<location path="apps/mock-oauth2-server/src/index.ts" line_range="84" />
<code_context>
+ const family_name = isAdminPortal
+ ? process.env['ADMIN_FAMILY_NAME'] || process.env['FAMILY_NAME'] || ''
+ : process.env['FAMILY_NAME'] || '';
+ return { email, given_name, family_name };
+ },
+};startMockOAuth2Server(config).catch((err: unknown) => {
</code_context>
<issue_to_address>
**nitpick:** Separate the config object literal and `startMockOAuth2Server` call to avoid relying on ASI.
Right now the closing brace and `startMockOAuth2Server(config)...` call are on the same line as `};startMockOAuth2Server(...)`. This unusual style can hurt readability and trigger linting issues. Moving the call to a new line or adding an explicit semicolon before it will make the intent clearer and avoid ASI-related surprises.
</issue_to_address>
### Comment 2
<location path="packages/cellix/mock-oauth2-server-seedwork/README.md" line_range="40" />
<code_context>
+## Exports
+
+- `startMockOAuth2Server` - Main function to start the OAuth2 server
+- `setupEnvironment` - Utility to setup environment variables from `.env` and `.env.local`
+- Type exports: `OAuth2Config`, `UserProfile`, etc.
+
</code_context>
<issue_to_address>
**nitpick (typo):** Use "set up" (verb) instead of "setup" (noun) in this sentence.
Here it’s used as a verb phrase (“to set up”), so the description should read: `Utility to set up environment variables from .env and .env.local.` The `setupEnvironment` function name can stay as-is; this only applies to the description text.
Suggested implementation:
```
// Set up environment variables
```
There is likely also an export description bullet earlier in the README that reads:
- `setupEnvironment` - Utility to setup environment variables from `.env` and `.env.local`
That line should be updated to:
- `setupEnvironment` - Utility to set up environment variables from `.env` and `.env.local`
to use the verb phrase “set up” in the sentence while keeping the function name unchanged.
</issue_to_address>
### Comment 3
<location path="packages/cellix/mock-payment-server-seedwork/src/index.ts" line_range="39" />
<code_context>
+ iframeJsPath?: string;
+}
+
+export function startMockPaymentServer(config: PaymentConfig): Promise<void> {
+ const app: Express = express();
+ app.disable('x-powered-by');
</code_context>
<issue_to_address>
**issue (complexity):** Consider splitting route registration, mock response construction, and server startup into dedicated helpers to keep this large mock server file easier to understand and maintain without changing behavior.
You can keep the same behavior but reduce complexity by extracting some of the repeated patterns into small helpers. A few targeted refactors would make this much easier to navigate without changing any responses.
### 1. Factor out route groups from `startMockPaymentServer`
Right now `startMockPaymentServer` both configures the app and registers all routes. You can keep the function as the only public API but move route registration into focused helpers:
```ts
// payment-routes.ts
export function registerPaymentRoutes(app: Express, config: PaymentConfig) {
const CYBERSOURCE_MERCHANT_ID = 'simnova_sharethrift';
app.post('/payments/v1/authorizations', (req, res) => {
res.json({
status: 'AUTHORIZED',
id: 'mock-transaction-id',
amount: req.body.amount || '100.00',
currency: req.body.currency || 'USD',
merchantId: CYBERSOURCE_MERCHANT_ID,
decision: 'ACCEPT',
message: 'Mock authorization successful',
});
});
// /pts/v2/payments, /pts/v2/refunds, etc.
}
```
```ts
// customer-routes.ts
export function registerCustomerRoutes(app: Express) {
app.post('/pts/v2/customers', (req, res) => {
const { customerProfile, paymentTokenInfo } = req.body;
const mockResponse = buildCreateCustomerResponse(customerProfile, paymentTokenInfo);
return res.status(201).json(mockResponse);
});
app.get('/pts/v2/customers/:customerId', (req, res) => {
const mockResponse = buildCustomerPaymentResponse(req.params.customerId);
return res.status(200).json(mockResponse);
});
// payment-instrument routes...
}
```
And in `startMockPaymentServer`:
```ts
export function startMockPaymentServer(config: PaymentConfig): Promise<void> {
const app = express();
// common middleware, CORS, static, etc.
registerFlexRoutes(app, config);
registerPaymentRoutes(app, config);
registerCustomerRoutes(app);
registerPlanRoutes(app);
registerSubscriptionRoutes(app);
return startServerWithRetry(app, config);
}
```
This keeps the public API unchanged but makes each group of endpoints self-contained.
### 2. Extract mock object factories to remove duplication
Several endpoints build nearly identical nested structures. For example, the single payment instrument response in `/tms/v2/customers/:customerId/payment-instruments/:paymentInstrumentId` and the one in the list endpoint share the same shape. You can centralize that to reduce drift:
```ts
// mock-factories.ts
export function buildPaymentInstrument(
customerId: string,
overrides: Partial<CustomerPaymentInstrumentResponse> = {},
): CustomerPaymentInstrumentResponse {
const base: CustomerPaymentInstrumentResponse = {
_links: {
self: {
href: `https://api.mockcybersource.com/customers/${customerId}/payment-instruments/PI_1111`,
},
customer: {
href: `https://api.mockcybersource.com/customers/${customerId}`,
},
},
id: 'PI_1111',
object: 'paymentInstrument',
default: true,
state: 'ACTIVE',
card: { expirationMonth: '12', expirationYear: '2026', type: '001' },
buyerInformation: { currency: 'USD' },
billTo: {
firstName: 'John',
lastName: 'Doe',
address1: '123 Mock Street',
address2: '',
locality: 'Faketown',
administrativeArea: 'CA',
postalCode: '99999',
country: 'US',
email: 'john.doe@example.com',
phoneNumber: '+1-555-1234',
},
processingInformation: { billPaymentProgramEnabled: false },
instrumentIdentifier: { id: 'MOCK_IDENTIFIER_1111' },
metadata: { creator: 'mock_system' },
_embedded: {
instrumentIdentifier: {
_links: {
self: {
href: `https://api.mockcybersource.com/instrument-identifiers/MOCK_IDENTIFIER_1111`,
},
paymentInstruments: {
href: `https://api.mockcybersource.com/customers/${customerId}/payment-instruments`,
},
},
id: 'MOCK_IDENTIFIER_1111',
object: 'instrumentIdentifier',
state: 'ACTIVE',
card: { number: '411111XXXXXX1111' },
processingInformation: {
authorizationOptions: {
initiator: {
merchantInitiatedTransaction: { previousTransactionId: 'TXN12345' },
},
},
},
metadata: { creator: 'mock_system' },
},
},
};
return { ...base, ...overrides };
}
```
Then your handlers become much simpler:
```ts
app.get(
'/tms/v2/customers/:customerId/payment-instruments/:paymentInstrumentId',
(req, res) => {
const { customerId, paymentInstrumentId } = req.params;
const mockResponse = buildPaymentInstrument(customerId, { id: paymentInstrumentId });
return res.status(200).json(mockResponse);
},
);
app.get(
'/tms/v2/customers/:customerId/payment-instruments',
(req, res) => {
const { customerId } = req.params;
const offset = Number((req.query['offset'] as string) ?? 0);
const limit = Number((req.query['limit'] as string) ?? 10);
const all = [buildPaymentInstrument(customerId)];
const response: CustomerPaymentInstrumentsResponse = {
_links: {
self: { href: `https://api.mockcybersource.com/customers/${customerId}/payment-instruments` },
first: { href: `https://api.mockcybersource.com/customers/${customerId}/payment-instruments?offset=0&limit=${limit}` },
last: { href: `https://api.mockcybersource.com/customers/${customerId}/payment-instruments?offset=0&limit=${limit}` },
},
offset,
limit,
count: all.length,
total: all.length,
_embedded: { paymentInstruments: all.slice(offset, offset + limit) },
};
return res.status(200).json(response);
},
);
```
You can apply the same pattern for:
- `buildCustomerPaymentResponse(customerId)`
- `buildPlanResponse(planId?)`
- `buildSubscriptionResponse(subscriptionId?)`
- `buildPaymentTransactionResponse(...)` (for create customer / add instrument)
This keeps the types as-is but moves the nested literals out of the route callbacks.
### 3. Deduplicate HTTP/HTTPS server startup with a small helper
The `startServer` inner function has two nearly identical branches. You can keep the retry behavior but centralize the common logic:
```ts
// server-start.ts
function listenWithRetry(
create: () => import('http').Server,
port: number,
host: string,
log: (port: number) => void,
maxAttempts = 5,
attempt = 0,
onError?: (err: NodeJS.ErrnoException) => void,
): void {
const server = create().listen(port, host, () => {
log(port);
});
server.on('error', (error: NodeJS.ErrnoException) => {
if (error.code === 'EADDRINUSE' && attempt < maxAttempts) {
const nextPort = port + 1;
console.warn(`Port ${port} in use. Retrying mock-payment-server on ${nextPort}...`);
server.close(() => {
listenWithRetry(create, nextPort, host, log, maxAttempts, attempt + 1, onError);
});
return;
}
onError?.(error);
});
}
```
Then inside `startMockPaymentServer`:
```ts
return new Promise((resolve, reject) => {
const log = (port: number) => {
console.log(` Mock Payment Server listening on ${config.protocol}://${config.paymentHost.replace(/:\d+$/, ':' + port)}`);
console.log(` CORS origin: ${config.frontendBaseUrl}`);
console.log(` Microform origin: ${config.paymentBaseUrl}`);
resolve();
};
if (hasCerts) {
const httpsOptions = {
key: fs.readFileSync(config.certKeyPath as string),
cert: fs.readFileSync(config.certPath as string),
};
listenWithRetry(
() => https.createServer(httpsOptions, app),
config.port,
config.host,
log,
5,
0,
reject,
);
} else {
listenWithRetry(
() => app,
config.port,
config.host,
(port) => {
console.log(` Mock Payment Server listening on ${config.protocol}://localhost:${port} (no certs found)`);
console.log(` CORS origin: ${config.frontendBaseUrl}`);
console.log(` Microform origin: ${config.paymentBaseUrl}`);
resolve();
},
5,
0,
reject,
);
}
});
```
This keeps the retry semantics and logging but removes duplication and makes the startup flow easier to follow.
---
All of these changes preserve the existing behavior and responses, but they reduce the visual and structural complexity by isolating concerns: route registration per domain, mock structure construction, and server startup.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
… will chnge with serenity
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Extract mock OAuth2 and payment server logic into reusable seedwork packages and re-home local mock servers as apps while aligning package naming and references with the new @app and @Cellix conventions.
New Features:
Enhancements:
Documentation: