Skip to content

Commit

Permalink
chore!: Align Coerce Methods (SAP#3431)
Browse files Browse the repository at this point in the history
  • Loading branch information
FrankEssenberger authored Jan 30, 2023
1 parent ff34e4a commit b1eb728
Show file tree
Hide file tree
Showing 17 changed files with 221 additions and 113 deletions.
5 changes: 5 additions & 0 deletions .changeset/cuddly-experts-chew.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sap-cloud-sdk/openapi-generator': major
---

[Compatibility Note] The internal option `packageVersion` of the OpenAPI generator is removed.
6 changes: 6 additions & 0 deletions .changeset/small-lions-boil.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sap-cloud-sdk/generator': major
---

[Compatibility Note] The type for paths in the `GeneratorOptions` is changed from `fs.PathLike` to `string`.
In case you passed a buffer object please resolve it to a string before passing it to the SAP Cloud SDK.
8 changes: 8 additions & 0 deletions .changeset/two-nails-protect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@sap-cloud-sdk/openapi-generator': major
'@sap-cloud-sdk/generator-common': major
'@sap-cloud-sdk/generator': major
---

[Compatibility Note] The two generators use the same CLI parsing code now, aligning the way paths are resolved.
In case you experience problems with the new logic, enable the `verbose` flag to investigate the new paths.
8 changes: 7 additions & 1 deletion V3-Upgrade-Guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,18 @@ Use the `include` option to add a `.npmrc` to the generated code if needed.
The internal options `sdkAfterVersionScript`, `s4HanaCloud` and `packageVersion` of the generator are removed.
These were hidden options never meant for external usage and there is no replacement.

The types for paths in the `GeneratorOptions` are changed from `fs.PathLike` to `string`.
In case you passed a buffer object please resolve it to a string before passing it to the SAP Cloud SDK.

#### Package `@sap-cloud-sdk/openapi-generator` <!-- omit from toc -->

The deprecated generator options `versionInPackageJson` and `licenseInPackageJson` are removed.
The deprecated generator options `versionInPackageJson`, `packageVersion` and `licenseInPackageJson` are removed.
In a generated `package.json` the version `1.0.0` and license `UNLICENSED` are used.
Use the `include` option to add a `package.json` with custom values.

The generator uses the same code to resolve paths as the OData generator now.
In case you experience problems with the new logic enable the `verbose` flag to investigate what are the new paths now.

### Package `@sap-cloud-sdk/odata-common` <!-- omit from toc -->

- When creating entities with the `fromJson()` method, the `_customFields` property is no longer considered. Add custom properties as root level properties in your object instead.
Expand Down
57 changes: 56 additions & 1 deletion packages/generator-common/src/options-parser.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { join, resolve } from 'path';
import { createLogger } from '@sap-cloud-sdk/util';
import { getOptionsWithoutDefaults, parseOptions } from './options-parser';
import mock from 'mock-fs';
import {
getOptionsWithoutDefaults,
parseOptions,
resolveGlob
} from './options-parser';
const logger = createLogger('generator-options');

describe('options parser', () => {
Expand Down Expand Up @@ -29,8 +35,17 @@ describe('options parser', () => {
default: 'default value',
coerce: val => `coerced: ${val}`
} as const;
const include = {
describe: 'include files with glob',
type: 'string' as const,
coerce: resolveGlob
};
const options = { deprecatedOption, otherOption, newOption } as const;

const absoluteJsonPaths = ['package.json', 'tsconfig.json'].map(s =>
resolve(s)
);

let warnSpy: jest.SpyInstance;

beforeEach(() => {
Expand Down Expand Up @@ -92,6 +107,46 @@ describe('options parser', () => {
});
});

it('includes using glob using cwd', () => {
expect(parseOptions({ include }, { include: '*.json' }).include).toEqual(
absoluteJsonPaths
);
});

it('includes using glob using root', () => {
const rootGlob = join(resolve(), '*.json');
expect(parseOptions({ include }, { include: rootGlob }).include).toEqual(
absoluteJsonPaths
);
});

it('does not fail on include option not set', () => {
expect(parseOptions({ include }, {}).include).toEqual([]);
});

it('includes using config path', () => {
const config = {
describe: 'config files',
type: 'string' as const
};

mock({
'/dummy/root': {
'file-1.json': '',
'sub-1': {
'file-2.json': ''
}
}
});
expect(
parseOptions(
{ include, config },
{ include: '**/*.json', config: '/dummy/root/config.json' }
).include
).toEqual(['file-1.json', 'sub-1/file-2.json'].map(s => resolve(s)));
mock.restore();
});

it('coerces value even if unset, using other options', () => {
expect(
parseOptions({ otherOption, coercedOption } as const, {
Expand Down
56 changes: 56 additions & 0 deletions packages/generator-common/src/options-parser.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { dirname, posix, resolve, sep } from 'path';
import { createLogger } from '@sap-cloud-sdk/util';
import { InferredOptionType, Options as YargsOption } from 'yargs';
const logger = createLogger('generator-options');
import { sync as globSync } from 'glob';

/**
* @internal
Expand Down Expand Up @@ -67,6 +69,60 @@ type OptionsWith<
: never;
}[keyof CliOptionsT];

/**
* Resolves a string using glob notation. If a config is given in generatorOptions, the glob working directory is considered relative to this config.
* @internal
* @param arg - Value for the string for which the glob is resolved.
* @param options - Generator options.
*/
export function resolveGlob<GeneratorOptionsT>(
arg: string | undefined,
options: GeneratorOptionsT & { config?: string }
): string[] {
if (!arg) {
return [];
}

const globConfig = options.config
? { cwd: resolve(dirname(options.config)) }
: { cwd: resolve() };

// Glob expressions only support unix style path separator (/). The below adjustment is made so it works on Windows. https://github.com/isaacs/node-glob#windows
return globSync(arg.split(sep).join(posix.sep), globConfig).map(s =>
resolve(s)
);
}

/**
* Resolves arguments that represent paths to an absolute path as a `string`. Only works for required options.
* @internal
* @param arg - Path argument as passed by the user.
* @param options - Options as passed by the user.
* @returns Absolute path as a `string`.
*/
export function resolveRequiredPath<GeneratorOptionsT>(
arg: string,
options: GeneratorOptionsT & { config?: string }
): string {
return options.config
? resolve(dirname(options.config), arg.toString())
: resolve(arg.toString());
}

/**
* Same as `resolveRequiredPath`, but for non-required options.
* @internal
* @param arg - Path argument as passed by the user, or `undefined` if nothing was passed.
* @param options - Options as passed by the user.
* @returns Absolute path as a `string` or `undefined`.
*/
export function resolvePath<GeneratorOptionsT>(
arg: string | undefined,
options: GeneratorOptionsT & { config?: string }
): string | undefined {
return arg ? resolveRequiredPath(arg, options) : undefined;
}

/**
* @internal
* Remove defaults from CLI options. This is necessary to handle default setting on our own.
Expand Down
1 change: 0 additions & 1 deletion packages/generator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
"@types/fs-extra": "^11.0.1",
"fast-xml-parser": "^4.0.15",
"fs-extra": "^11.1.0",
"glob": "^8.1.0",
"ts-morph": "^17.0.1",
"typescript": "~4.9.4",
"voca": "^1.4.0",
Expand Down
21 changes: 12 additions & 9 deletions packages/generator/src/generator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import { SourceFile } from 'ts-morph';
import mock from 'mock-fs';
import prettier from 'prettier';
import { createLogger } from '@sap-cloud-sdk/util';
import { createOptions } from '../test/test-util/create-generator-options';
import {
createOptions,
createParsedOptions
} from '../test/test-util/create-generator-options';
import {
checkStaticProperties,
getOperationFunctionDeclarations,
Expand Down Expand Up @@ -46,11 +49,11 @@ describe('generator', () => {
optionsPerService: 'someDir/test-service-options.json',
overwrite: true,
prettierConfig: '/prettier/config',
generateSdkMetadata: true,
metadata: true,
include: join(pathTestResources, '*.md')
});
// TODO the first call will go away once ts-morph is removed
project = await generateProject(options);
project = await generateProject(createParsedOptions(options));
await generate(options);
});

Expand All @@ -65,7 +68,7 @@ describe('generator', () => {
});
try {
// TODO the first call will go away once ts-morph is removed
project = await generateProject(options);
project = await generateProject(createParsedOptions(options));
await generate(options);
throw new Error('Should not go here.');
} catch (e) {
Expand Down Expand Up @@ -344,7 +347,7 @@ describe('generator', () => {
skipValidation: true,
overwrite: true
});
await generateProject(options);
await generateProject(createParsedOptions(options));
await generate(options);

const actual = readFile('anotherConfig', 'utf8');
Expand Down Expand Up @@ -417,12 +420,12 @@ describe('generator', () => {
outputDir: 'logger',
overwrite: true,
prettierConfig: '/prettier/config',
generateSdkMetadata: true,
metadata: true,
skipValidation: true,
include: join(pathTestResources, '*.md')
});

await generateProject(options);
await generateProject(createParsedOptions(options));
await generate(options);
expect(logger.level).toBe('info');
expect(consoleSpy).not.toBeCalled();
Expand All @@ -444,12 +447,12 @@ describe('generator', () => {
overwrite: true,
skipValidation: true,
prettierConfig: '/prettier/config',
generateSdkMetadata: true,
metadata: true,
include: join(pathTestResources, '*.md'),
verbose: true
});

await generateProject(options);
await generateProject(createParsedOptions(options));
await generate(options);
expect(logger.level).toBe('verbose');
const log = await readFile('test.log', { encoding: 'utf-8' });
Expand Down
13 changes: 4 additions & 9 deletions packages/generator/src/generator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { existsSync, promises as fsPromises } from 'fs';
import { dirname, join, posix, resolve, sep } from 'path';
import { dirname, join, resolve } from 'path';
import {
copyFiles,
createFile,
Expand All @@ -21,7 +21,6 @@ import {
splitInChunks
} from '@sap-cloud-sdk/util';
import { emptyDirSync } from 'fs-extra';
import { GlobSync } from 'glob';
import {
IndentationText,
ModuleKind,
Expand Down Expand Up @@ -247,13 +246,9 @@ async function generateIncludes(
service: VdmServiceMetadata,
options: ParsedGeneratorOptions
): Promise<void> {
if (options.include) {
const includeDir = resolve(options.inputDir.toString(), options.include)
.split(sep)
.join(posix.sep);
if (options.include && options.include.length > 0) {
const serviceDir = join(options.outputDir, service.directoryName);
const files = new GlobSync(includeDir).found;
await copyFiles(files, serviceDir, options.overwrite);
await copyFiles(options.include, serviceDir, options.overwrite);
}
}

Expand Down Expand Up @@ -441,7 +436,7 @@ export async function generateSourcesForService(
);
}

if (options.generateSdkMetadata) {
if (options.metadata) {
const { clientFileName } = getSdkMetadataFileNames(
service.originalFileName
);
Expand Down
6 changes: 4 additions & 2 deletions packages/generator/src/options-per-service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { parse } from 'path';
import { existsSync, readFileSync } from 'fs';
import { unixEOL, createLogger } from '@sap-cloud-sdk/util';
import { GeneratorOptions } from './options';
import { ParsedGeneratorOptions } from './options';
import { VdmServiceMetadata } from './vdm-types';
import { servicePathFromSwagger } from './swagger-parser/swagger-util';
import { ServiceMetadata } from './edmx-parser/edmx-file-reader';
Expand Down Expand Up @@ -41,7 +41,9 @@ export interface OptionsPerService {
/**
* @internal
*/
export function readOptionsPerService(options: GeneratorOptions): VdmMapping {
export function readOptionsPerService(
options: ParsedGeneratorOptions
): VdmMapping {
const configPath = options.optionsPerService;
return configPath && existsSync(configPath)
? (JSON.parse(readFileSync(configPath, 'utf8')) as VdmMapping)
Expand Down
Loading

0 comments on commit b1eb728

Please sign in to comment.