Skip to content
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

fix: create new annotationFile #2432

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 16 additions & 1 deletion packages/adp-tooling/src/base/abap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,22 @@ export async function getManifest(appId: string, adpConfig: AdpPreviewConfig, lo
}
try {
const response = await provider.get(manifestUrl);
return JSON.parse(response.data);
const manifest = JSON.parse(response.data);

// retrieve localmetadata from base project
const dataSourceName = Object.keys(manifest?.['sap.app']?.dataSources)?.[0];
const metadataPath = manifest?.['sap.app']?.dataSources?.[dataSourceName];
let metadataUrl = '';
if (metadataPath?.settings?.localUri) {//v2
metadataUrl = `${appInfo.url}/${metadataPath.settings.localUri}`;
} else if (metadataPath?.uri) { //v4
metadataUrl = `${appInfo.url}/${metadataPath.uri}$metadata`;
Copy link
Contributor Author

@Jimmy-Joseph19 Jimmy-Joseph19 Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to get metadata for all v2 apps i tested.
For one V4 app tested the request was failing. But the same request passes in CPE when baseUrl is localhost.

}

const metadata = await provider.get(metadataUrl);

console.log(metadata.data);
return manifest;
} catch (error) {
if (isAxiosError(error)) {
logger.error('Manifest fetching failed');
Expand Down
37 changes: 22 additions & 15 deletions packages/adp-tooling/src/base/change-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Dirent } from 'fs';
import path from 'path';
import { join, dirname, posix, resolve } from 'path';
import type { Editor } from 'mem-fs-editor';
import { existsSync, readFileSync, readdirSync } from 'fs';

Expand All @@ -13,6 +13,7 @@ import {
type ManifestChangeProperties,
type PropertyValueType
} from '../types';
import { renderFile } from 'ejs';

export type ChangeMetadata = Pick<DescriptorVariant, 'id' | 'layer' | 'namespace'>;

Expand Down Expand Up @@ -40,26 +41,32 @@ export function writeAnnotationChange(
): void {
try {
const changeFileName = `id_${timestamp}_addAnnotationsToOData.change`;
const changesFolderPath = path.join(projectPath, DirName.Webapp, DirName.Changes);
const changeFilePath = path.join(changesFolderPath, DirName.Manifest, changeFileName);
const annotationsFolderPath = path.join(changesFolderPath, DirName.Annotations);
const changesFolderPath = join(projectPath, DirName.Webapp, DirName.Changes);
const changeFilePath = join(changesFolderPath, DirName.Manifest, changeFileName);
const annotationsFolderPath = join(changesFolderPath, DirName.Annotations);

writeChangeToFile(changeFilePath, change, fs);

if (!annotation.filePath) {
const annotationsTemplate = path.join(
const annotationsTemplate = join(
__dirname,
'..',
'..',
'templates',
'changes',
TemplateFileName.Annotation
);
fs.copy(annotationsTemplate, path.join(annotationsFolderPath, annotation.fileName ?? ''));
const { namespaces, serviceUrl: path } = annotation;
renderFile(annotationsTemplate, { namespaces: namespaces, path: path }, {}, async (err, str) => {
if (err) {
throw new Error('Error rendering template: ' + err.message);
}
fs.write(join(annotationsFolderPath, annotation.fileName ?? ''), str);
});
} else {
const selectedDir = path.dirname(annotation.filePath);
const selectedDir = dirname(annotation.filePath);
if (selectedDir !== annotationsFolderPath) {
fs.copy(annotation.filePath, path.join(annotationsFolderPath, annotation.fileName ?? ''));
fs.copy(annotation.filePath, join(annotationsFolderPath, annotation.fileName ?? ''));
}
}
} catch (e) {
Expand All @@ -86,13 +93,13 @@ export function writeChangeToFolder(
dir = ''
): void {
try {
let targetFolderPath = path.join(projectPath, DirName.Webapp, DirName.Changes);
let targetFolderPath = join(projectPath, DirName.Webapp, DirName.Changes);

if (dir) {
targetFolderPath = path.join(targetFolderPath, dir);
targetFolderPath = join(targetFolderPath, dir);
}

const filePath = path.join(targetFolderPath, fileName);
const filePath = join(targetFolderPath, fileName);
writeChangeToFile(filePath, change, fs);
} catch (e) {
throw new Error(`Could not write change to folder. Reason: ${e.message}`);
Expand Down Expand Up @@ -187,7 +194,7 @@ export function getChangesByType(

const changeFiles: ManifestChangeProperties[] = fileNames
.map((fileName) => {
const filePath = path.resolve(targetDir, fileName);
const filePath = resolve(targetDir, fileName);
const fileContent = readFileSync(filePath, 'utf-8');
const change: ManifestChangeProperties = JSON.parse(fileContent);
return change;
Expand All @@ -212,7 +219,7 @@ export function findChangeWithInboundId(projectPath: string, inboundId: string):
let changeObj: InboundChange | undefined;
let filePath = '';

const pathToInboundChangeFiles = path.join(projectPath, DirName.Webapp, DirName.Changes, DirName.Manifest);
const pathToInboundChangeFiles = join(projectPath, DirName.Webapp, DirName.Changes, DirName.Manifest);

if (!existsSync(pathToInboundChangeFiles)) {
return {
Expand All @@ -227,7 +234,7 @@ export function findChangeWithInboundId(projectPath: string, inboundId: string):
);

for (const file of files) {
const pathToFile = path.join(pathToInboundChangeFiles, file.name);
const pathToFile = join(pathToInboundChangeFiles, file.name);
const change: InboundChange = JSON.parse(readFileSync(pathToFile, 'utf-8'));

if (change.content?.inboundId === inboundId) {
Expand Down Expand Up @@ -263,7 +270,7 @@ export function getChange(
): ManifestChangeProperties {
return {
fileName: `id_${timestamp}`,
namespace: path.posix.join(namespace, DirName.Changes),
namespace: posix.join(namespace, DirName.Changes),
layer,
fileType: 'change',
creation: new Date(timestamp).toISOString(),
Expand Down
6 changes: 5 additions & 1 deletion packages/adp-tooling/src/preview/adp-preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import type { AdpPreviewConfig, CommonChangeProperties, DescriptorVariant, OperationType } from '../types';
import type { Editor } from 'mem-fs-editor';
import { addXmlFragment, isAddXMLChange, moduleNameContentMap, tryFixChange } from './change-handler';
import { join } from 'path';

declare global {
// false positive, const can't be used here https://github.com/eslint/eslint/issues/15896
Expand All @@ -22,7 +23,8 @@
export const enum ApiRoutes {
FRAGMENT = '/adp/api/fragment',
CONTROLLER = '/adp/api/controller',
CODE_EXT = '/adp/api/code_ext/:controllerName'
CODE_EXT = '/adp/api/code_ext/:controllerName',
ANNOTATION_FILE = '/adp/api/annotation'
}

/**
Expand Down Expand Up @@ -198,6 +200,8 @@
router.post(ApiRoutes.CONTROLLER, this.routesHandler.handleWriteControllerExt as RequestHandler);

router.get(ApiRoutes.CODE_EXT, this.routesHandler.handleGetControllerExtensionData as RequestHandler);

router.post(ApiRoutes.ANNOTATION_FILE, this.routesHandler.handleCreateAnnoationFile as RequestHandler);

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a file system access
, but is not rate-limited.
This route handler performs
a file system access
, but is not rate-limited.
This route handler performs
a file system access
, but is not rate-limited.
This route handler performs
a file system access
, but is not rate-limited.

Copilot Autofix AI about 1 month ago

To fix the problem, we will introduce a rate-limiting middleware using the express-rate-limit package. This middleware will limit the number of requests to the route handler handleCreateAnnoationFile to a reasonable number within a specified time window. This will help prevent potential DoS attacks by ensuring that the server can handle requests without being overwhelmed.

  1. Install the express-rate-limit package if it is not already installed.
  2. Import the express-rate-limit package in the file.
  3. Create a rate limiter with a specified window and maximum number of requests.
  4. Apply the rate limiter to the specific route handler handleCreateAnnoationFile.
Suggested changeset 2
packages/adp-tooling/src/preview/adp-preview.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/adp-tooling/src/preview/adp-preview.ts b/packages/adp-tooling/src/preview/adp-preview.ts
--- a/packages/adp-tooling/src/preview/adp-preview.ts
+++ b/packages/adp-tooling/src/preview/adp-preview.ts
@@ -4,2 +4,3 @@
 import type { NextFunction, Request, Response, Router, RequestHandler } from 'express';
+import rateLimit from 'express-rate-limit';
 
@@ -196,2 +197,7 @@
     addApis(router: Router): void {
+        const limiter = rateLimit({
+            windowMs: 15 * 60 * 1000, // 15 minutes
+            max: 100, // limit each IP to 100 requests per windowMs
+        });
+
         router.get(ApiRoutes.FRAGMENT, this.routesHandler.handleReadAllFragments as RequestHandler);
@@ -203,3 +209,3 @@
 
-        router.post(ApiRoutes.ANNOTATION_FILE, this.routesHandler.handleCreateAnnoationFile as RequestHandler);
+        router.post(ApiRoutes.ANNOTATION_FILE, limiter, this.routesHandler.handleCreateAnnoationFile as RequestHandler);
     }
EOF
@@ -4,2 +4,3 @@
import type { NextFunction, Request, Response, Router, RequestHandler } from 'express';
import rateLimit from 'express-rate-limit';

@@ -196,2 +197,7 @@
addApis(router: Router): void {
const limiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // limit each IP to 100 requests per windowMs
});

router.get(ApiRoutes.FRAGMENT, this.routesHandler.handleReadAllFragments as RequestHandler);
@@ -203,3 +209,3 @@

router.post(ApiRoutes.ANNOTATION_FILE, this.routesHandler.handleCreateAnnoationFile as RequestHandler);
router.post(ApiRoutes.ANNOTATION_FILE, limiter, this.routesHandler.handleCreateAnnoationFile as RequestHandler);
}
packages/adp-tooling/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/adp-tooling/package.json b/packages/adp-tooling/package.json
--- a/packages/adp-tooling/package.json
+++ b/packages/adp-tooling/package.json
@@ -53,3 +53,4 @@
         "sanitize-filename": "1.6.3",
-        "uuid": "10.0.0"
+        "uuid": "10.0.0",
+        "express-rate-limit": "^7.4.1"
     },
EOF
@@ -53,3 +53,4 @@
"sanitize-filename": "1.6.3",
"uuid": "10.0.0"
"uuid": "10.0.0",
"express-rate-limit": "^7.4.1"
},
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 7.4.1 None
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/adp-tooling/src/preview/change-handler.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Editor } from 'mem-fs-editor';
import type { AddXMLChange, CommonChangeProperties, CodeExtChange } from '../types';
import { type AddXMLChange, type CommonChangeProperties, type CodeExtChange, DescriptorVariant } from '../types';
import { join } from 'path';
import { DirName } from '@sap-ux/project-access';
import type { Logger } from '@sap-ux/logger';
Expand Down
77 changes: 76 additions & 1 deletion packages/adp-tooling/src/preview/routes-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,21 @@ import type { NextFunction, Request, Response } from 'express';

import { TemplateFileName, HttpStatusCodes } from '../types';
import { DirName } from '@sap-ux/project-access';
import type { CodeExtChange } from '../types';
import { ChangeType, CodeExtChange } from '../types';
import { generateChange } from '../writer/editors';

interface WriteControllerBody {
controllerName: string;
projectId: string;
}

export interface WriteAnnotationFile {
datasource: string;
namespaces: { namespace: string; alias: string }[];
path: string;
odataVersion: '2.0' | '4.0';
}

/**
* @description Handles API Routes
*/
Expand Down Expand Up @@ -244,4 +252,71 @@ export default class RoutesHandler {
next(e);
}
};

public handleCreateAnnoationFile = async (req: Request, res: Response, next: NextFunction) => {
try {
const data = req.body as WriteAnnotationFile;

const annotationFileName = sanitize('annotation0');
// const projectId = data.projectId;
const projectPath = this.util.getProject().getRootPath();
const sourcePath = this.util.getProject().getSourcePath();

if (!annotationFileName) {
res.status(HttpStatusCodes.BAD_REQUEST).send('Annotation File name was not provided!');
this.logger.debug('Bad request. Annotation File name was not provided!');
return;
}

const fullPath = path.join(sourcePath, DirName.Annotations);
const filePath = path.join(fullPath, `${annotationFileName}.xml`);

if (!fs.existsSync(fullPath)) {
fs.mkdirSync(fullPath, { recursive: true });
}

if (fs.existsSync(filePath)) {
res.status(HttpStatusCodes.CONFLICT).send(
`Annotation file with name "${annotationFileName}" already exists`
);
this.logger.debug(`Annotation file with name name "${annotationFileName}" already exists`);
return;
}

const annatationFilePath = path.join(__dirname, '../../templates/changes', TemplateFileName.Annotation);

// renderFile(annatationFilePath, { ...data }, {}, async (err, str) => {
// if (err) {
// throw new Error('Error rendering template: ' + err.message);
// }
const { datasource, namespaces, path: serviceUrl, odataVersion } = data;
const fsEditor = await generateChange<ChangeType.ADD_ANNOTATIONS_TO_ODATA>(
projectPath,
ChangeType.ADD_ANNOTATIONS_TO_ODATA,
{
annotation: {
fileName: 'annotation0.xml',
datasource,
namespaces,
serviceUrl,
odataVersion
},
variant: JSON.parse(
fs.readFileSync(path.join(sourcePath, 'manifest.appdescr_variant')).toString('utf-8')
)
}
);
fsEditor.commit((err) => this.logger.error(err));
// });

const message = 'Annotation file created!';
res.status(HttpStatusCodes.CREATED).send(message);
this.logger.debug(`Annotation file with name "${annotationFileName}" was created`);
} catch (e) {
const sanitizedMsg = sanitize(e.message);
this.logger.error(sanitizedMsg);
res.status(HttpStatusCodes.INTERNAL_SERVER_ERROR).send(sanitizedMsg);
next(e);
}
};
}
26 changes: 25 additions & 1 deletion packages/adp-tooling/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,27 @@ export interface AddXMLChange extends CommonChangeProperties {
jsOnly: boolean;
}

export interface AddAnnotationToOdata extends CommonChangeProperties {
changeType: ChangeType.ADD_ANNOTATIONS_TO_ODATA;
creation: string;
packageName: string;
content: {
datasourceId: string;
annotations: string[];
annotationsInsertPosition: 'END';
datasource: {
[key: string]: {
uri: string;
type: 'ODataAnntation';
};
};
};
selector: {
id: string;
idIsLocal: boolean;
};
}

export interface CodeExtChange extends CommonChangeProperties {
changeType: 'codeExt';
content: {
Expand Down Expand Up @@ -302,9 +323,12 @@ export interface AnnotationsData {
/** Optional name of the annotation file. */
fileName?: string;
/** Data source associated with the annotation. */
dataSource: string;
datasource: string;
/** Optional path to the annotation file. */
filePath?: string;
namespaces?: { namespace: string; alias: string }[];
serviceUrl?: string;
odataVersion?: '2.0' | '4.0';
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ export class AnnotationsWriter implements IWriter<AnnotationsData> {
private constructContent(data: AnnotationsData): object {
const {
variant: { layer },
annotation: { dataSource, fileName }
annotation: { datasource, fileName }
} = data;
const annotationFileNameWithoutExtension = fileName?.toLocaleLowerCase().replace('.xml', '');
const annotationNameSpace =
layer === FlexLayer.CUSTOMER_BASE
? `customer.annotation.${annotationFileNameWithoutExtension}`
: `annotation.${annotationFileNameWithoutExtension}`;
return {
dataSourceId: `${dataSource}`,
dataSourceId: `${datasource}`,
annotations: [annotationNameSpace],
annotationsInsertPosition: 'END',
dataSource: {
Expand Down Expand Up @@ -71,6 +71,6 @@ export class AnnotationsWriter implements IWriter<AnnotationsData> {
const content = this.constructContent(data);
const timestamp = Date.now();
const change = getChange(variant, timestamp, content, ChangeType.ADD_ANNOTATIONS_TO_ODATA);
writeAnnotationChange(this.projectPath, timestamp, data.annotation, change, this.fs);
writeAnnotationChange(this.projectPath, timestamp, annotation, change, this.fs);
}
}
3 changes: 3 additions & 0 deletions packages/adp-tooling/templates/changes/annotation.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
</edmx:Reference>
<edmx:Reference Uri="https://sap.github.io/odata-vocabularies/vocabularies/Communication.xml">
<edmx:Include Namespace="com.sap.vocabularies.Communication.v1" Alias="Communication"/>
</edmx:Reference>
<edmx:Reference Uri="<%- path %>/$metadata"><% namespaces.forEach(function(namespace){ %>
<edmx:Include Namespace="<%- namespace.namespace %>"<% if (namespace.alias) { %> Alias="<%- namespace.alias %>"<% } %>/><% }); %>
</edmx:Reference>
<edmx:DataServices>
<Schema xmlns="http://docs.oasis-open.org/odata/ns/edm" Namespace="local">
Expand Down
2 changes: 1 addition & 1 deletion packages/create/src/cli/add/annotations-to-odata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ async function addAnnotationsToOdata(basePath: string, simulate: boolean, yamlPa
{
variant,
annotation: {
dataSource: answers.id,
datasource: answers.id,
filePath: answers.filePath
}
}
Expand Down
11 changes: 11 additions & 0 deletions packages/preview-middleware-client/src/adp/api-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const enum ApiEndpoints {
FRAGMENT = '/adp/api/fragment',
CONTROLLER = '/adp/api/controller',
CODE_EXT = '/adp/api/code_ext',
ANNOTATION_FILE = '/adp/api/annotation',
MANIFEST_APP_DESCRIPTOR = '/manifest.appdescr_variant'
}

Expand Down Expand Up @@ -139,6 +140,16 @@ export async function writeController<T>(data: T): Promise<T> {
return request<T>(ApiEndpoints.CONTROLLER, RequestMethod.POST, data);
}

/**
* Writes a new annotation file to the project's workspace
*
* @param data Data to be send to the server
* @returns Generic Promise<T>
*/
export async function writeAnnotationFle<T>(data: T): Promise<T> {
return request<T>(ApiEndpoints.ANNOTATION_FILE, RequestMethod.POST, data);
}

/**
* Checks for existing controller in the project's workspace
*
Expand Down
Loading
Loading