Skip to content

chore: Update API to use 'route' instead of 'path' #35956

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.

Sign up for GitHub

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

Merged
ggazzo merged 2 commits into from
May 14, 2025
Merged

chore: Update API to use 'route' instead of 'path' #35956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e48563
fix: update API to use 'route' instead of 'path' for consistency in r…
ggazzo May 9, 2025
afb8c1c
Merge branch 'develop' into chore/request-type
ggazzo May 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/meteor/app/api/server/definition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,11 @@ export type PartialThis = {
readonly queryOperations?: string[];
readonly queryFields?: string[];
readonly logger: Logger;
readonly route: string;
};

type ActionThis<TMethod extends Method, TPathPattern extends PathPattern, TOptions> = {
route: string;
readonly requestIp: string;
urlParams: UrlParams<TPathPattern>;
readonly response: Response;
Expand Down
12 changes: 6 additions & 6 deletions apps/meteor/app/api/server/helpers/parseJsonQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export async function parseJsonQuery(api: PartialThis): Promise<{
*/
query: Record<string, unknown>;
}> {
const { userId, queryParams: params, logger, queryFields, queryOperations, response, path } = api;
const { userId, queryParams: params, logger, queryFields, queryOperations, response, route } = api;

let sort;
if (params.sort) {
Expand Down Expand Up @@ -52,7 +52,7 @@ export async function parseJsonQuery(api: PartialThis): Promise<{
let fields: Record<string, 0 | 1> | undefined;
if (params.fields && isUnsafeQueryParamsAllowed) {
try {
apiDeprecationLogger.parameter(api.path, 'fields', '8.0.0', response, messageGenerator);
apiDeprecationLogger.parameter(route, 'fields', '8.0.0', response, messageGenerator);
fields = JSON.parse(params.fields) as Record<string, 0 | 1>;
Object.entries(fields).forEach(([key, value]) => {
if (value !== 1 && value !== 0) {
Expand All @@ -72,7 +72,7 @@ export async function parseJsonQuery(api: PartialThis): Promise<{
// Verify the user's selected fields only contains ones which their role allows
if (typeof fields === 'object') {
let nonSelectableFields = Object.keys(API.v1.defaultFieldsToExclude);
if (path.includes('/v1/users.')) {
if (route.includes('/v1/users.')) {
nonSelectableFields = nonSelectableFields.concat(
Object.keys(
(await hasPermissionAsync(userId, 'view-full-other-user-info'))
Expand All @@ -91,7 +91,7 @@ export async function parseJsonQuery(api: PartialThis): Promise<{

// Limit the fields by default
fields = Object.assign({}, fields, API.v1.defaultFieldsToExclude);
if (path.includes('/v1/users.')) {
if (route.includes('/v1/users.')) {
if (await hasPermissionAsync(userId, 'view-full-other-user-info')) {
fields = Object.assign(fields, API.v1.limitedUserFieldsToExcludeIfIsPrivilegedUser);
} else {
Expand All @@ -101,7 +101,7 @@ export async function parseJsonQuery(api: PartialThis): Promise<{

let query: Record<string, any> = {};
if (params.query && isUnsafeQueryParamsAllowed) {
apiDeprecationLogger.parameter(api.path, 'query', '8.0.0', response, messageGenerator);
apiDeprecationLogger.parameter(route, 'query', '8.0.0', response, messageGenerator);
try {
query = ejson.parse(params.query);
query = clean(query, pathAllowConf.def);
Expand All @@ -117,7 +117,7 @@ export async function parseJsonQuery(api: PartialThis): Promise<{
if (typeof query === 'object') {
let nonQueryableFields = Object.keys(API.v1.defaultFieldsToExclude);

if (api.path.includes('/v1/users.')) {
if (route.includes('/v1/users.')) {
if (await hasPermissionAsync(userId, 'view-full-other-user-info')) {
nonQueryableFields = nonQueryableFields.concat(Object.keys(API.v1.limitedUserFieldsToExcludeIfIsPrivilegedUser));
} else {
Expand Down
26 changes: 8 additions & 18 deletions apps/meteor/app/api/server/router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,6 @@ declare module 'hono' {
}
}

declare global {
interface Request {
route: string;
}
}

export class Router<
TBasePath extends string,
TOperations extends {
Expand Down Expand Up @@ -190,7 +184,6 @@ export class Router<

this.innerRouter[method.toLowerCase() as Lowercase<Method>](`/${subpath}`.replace('//', '/'), ...middlewares, async (c) => {
const { req, res } = c;
req.raw.route = `${c.var.route ?? ''}${subpath}`;

const queryParams = this.parseQueryParams(req);

Expand Down Expand Up @@ -224,6 +217,8 @@ export class Router<
}
}

const request = req.raw.clone();

const {
body,
statusCode = 200,
Expand All @@ -234,11 +229,12 @@ export class Router<
urlParams: req.param(),
queryParams,
bodyParams,
request: req.raw.clone(),
request,
path: req.path,
response: res,
route: req.routePath,
} as any,
[req.raw.clone()],
[request],
);
if (process.env.NODE_ENV === 'test' || process.env.TEST_MODE) {
const responseValidatorFn = options?.response?.[statusCode];
Expand Down Expand Up @@ -376,15 +372,9 @@ export class Router<
router.use(
this.base,
honoAdapter(
hono
.use(`${this.base}/*`, (c, next) => {
c.set('route', `${c.var.route || ''}${this.base}`);
return next();
})
.route(this.base, this.innerRouter)
.options('*', (c) => {
return c.body('OK');
}),
hono.route(this.base, this.innerRouter).options('*', (c) => {
return c.body('OK');
}),
),
);
return router;
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/api/server/v1/oauthapps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ API.v1.addRoute(
}

if ('appId' in this.queryParams) {
apiDeprecationLogger.parameter(this.request.route, 'appId', '7.0.0', this.response);
apiDeprecationLogger.parameter(this.route, 'appId', '7.0.0', this.response);
}

return API.v1.success({
Expand Down
6 changes: 3 additions & 3 deletions apps/meteor/app/api/server/v1/roles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ API.v1.addRoute(
return API.v1.failure('error-invalid-role-properties');
}

apiDeprecationLogger.parameter(this.request.route, 'roleName', '7.0.0', this.response);
apiDeprecationLogger.parameter(this.route, 'roleName', '7.0.0', this.response);
}

const role = roleId ? await Roles.findOneById(roleId) : await Roles.findOneByIdOrName(roleName as string);
Expand Down Expand Up @@ -124,7 +124,7 @@ API.v1.addRoute(
}

apiDeprecationLogger.deprecatedParameterUsage(
this.request.route,
this.route,
'role',
'7.0.0',
this.response,
Expand Down Expand Up @@ -197,7 +197,7 @@ API.v1.addRoute(
return API.v1.failure('error-invalid-role-properties');
}

apiDeprecationLogger.parameter(this.request.route, 'roleName', '7.0.0', this.response);
apiDeprecationLogger.parameter(this.route, 'roleName', '7.0.0', this.response);
}

const user = await Users.findOneByUsername(username);
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/ee/server/api/engagementDashboard/channels.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ API.v1.addRoute(

if (hideRoomsWithNoActivity === undefined) {
apiDeprecationLogger.deprecatedParameterUsage(
this.request.route,
this.route,
'hideRoomsWithNoActivity',
'7.0.0',
this.response,
Expand Down
8 changes: 4 additions & 4 deletions apps/meteor/ee/server/apps/communication/rest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ export class AppsRestApi {

// Gets the Apps from the marketplace
if ('marketplace' in this.queryParams && this.queryParams.marketplace) {
apiDeprecationLogger.endpoint(this.request.route, '7.0.0', this.response, 'Use /apps/marketplace to get the apps list.');
apiDeprecationLogger.endpoint(this.route, '7.0.0', this.response, 'Use /apps/marketplace to get the apps list.');

try {
const apps = await fetchMarketplaceApps();
Expand All @@ -253,7 +253,7 @@ export class AppsRestApi {
}

if ('categories' in this.queryParams && this.queryParams.categories) {
apiDeprecationLogger.endpoint(this.request.route, '7.0.0', this.response, 'Use /apps/categories to get the categories list.');
apiDeprecationLogger.endpoint(this.route, '7.0.0', this.response, 'Use /apps/categories to get the categories list.');
try {
const categories = await fetchMarketplaceCategories();
return API.v1.success(categories);
Expand Down Expand Up @@ -282,7 +282,7 @@ export class AppsRestApi {
this.queryParams.buildExternalUrl &&
this.queryParams.appId
) {
apiDeprecationLogger.endpoint(this.request.route, '7.0.0', this.response, 'Use /apps/buildExternalUrl to get the modal URLs.');
apiDeprecationLogger.endpoint(this.route, '7.0.0', this.response, 'Use /apps/buildExternalUrl to get the modal URLs.');
const workspaceId = settings.get('Cloud_Workspace_Id');

if (!this.queryParams.purchaseType || !purchaseTypes.has(this.queryParams.purchaseType)) {
Expand All @@ -304,7 +304,7 @@ export class AppsRestApi {
}?workspaceId=${workspaceId}&token=${token.token}&seats=${seats}`,
});
}
apiDeprecationLogger.endpoint(this.request.route, '7.0.0', this.response, 'Use /apps/installed to get the installed apps list.');
apiDeprecationLogger.endpoint(this.route, '7.0.0', this.response, 'Use /apps/installed to get the installed apps list.');

const proxiedApps = await manager.get();
const apps = await Promise.all(proxiedApps.map((app) => formatAppInstanceForRest(app)));
Expand Down
Loading