Skip to content

Commit

Permalink
fix(rosetta): rosetta reader expects default tablet file to be uncomp…
Browse files Browse the repository at this point in the history
…ressed (#3723)

Due to a bug in `rosetta-reader.ts`, we always expect to find the implicit tablet at `.jsii.tabl.json`. This means that any compressed implicit tablets will be unable to be used as a cache, and causes the following failure:

```bash
The following snippet was not found in any of the loaded tablets: ...
```

This PR also slightly changes the default behavior of `compressTablet`; previously it was `false` by default. Now it respects the original compression status of the implicit tablet, i.e. if it was previously compressed, then `extract` will update it as compressed.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
kaizencc committed Aug 26, 2022
1 parent 93aec85 commit 9768e2a
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 15 deletions.
18 changes: 13 additions & 5 deletions packages/jsii-rosetta/lib/commands/extract.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import * as path from 'path';

import { loadAssemblies, allTypeScriptSnippets, loadAllDefaultTablets } from '../jsii/assemblies';
import {
loadAssemblies,
allTypeScriptSnippets,
loadAllDefaultTablets,
compressedTabletExists,
} from '../jsii/assemblies';
import * as logging from '../logging';
import { RosettaTranslator, RosettaTranslatorOptions } from '../rosetta-translator';
import { TypeScriptSnippet, SnippetParameters } from '../snippet';
Expand Down Expand Up @@ -75,9 +80,9 @@ export interface ExtractOptions {
readonly allowDirtyTranslations?: boolean;

/**
* Compress the implicit tablet files
* Compress the implicit tablet files.
*
* @default false
* @default - preserves the original compression status of each individual implicit tablet file.
*/
readonly compressTablet?: boolean;

Expand Down Expand Up @@ -169,16 +174,19 @@ export async function extractSnippets(
if (options.writeToImplicitTablets ?? true) {
await Promise.all(
Object.entries(snippetsPerAssembly).map(async ([location, snips]) => {
// Compress the implicit tablet if explicitly asked to, otherwise compress only if the original tablet was compressed.
const compressedTablet = options.compressTablet ?? compressedTabletExists(location);

const asmTabletFile = path.join(
location,
options.compressTablet ? DEFAULT_TABLET_NAME_COMPRESSED : DEFAULT_TABLET_NAME,
compressedTablet ? DEFAULT_TABLET_NAME_COMPRESSED : DEFAULT_TABLET_NAME,
);
logging.debug(`Writing ${snips.length} translations to ${asmTabletFile}`);
const translations = snips.map(({ key }) => translator.tablet.tryGetSnippet(key)).filter(isDefined);

const asmTablet = new LanguageTablet();
asmTablet.addSnippets(...translations);
await asmTablet.save(asmTabletFile, options.compressTablet);
await asmTablet.save(asmTabletFile, compressedTablet);
}),
);
}
Expand Down
23 changes: 17 additions & 6 deletions packages/jsii-rosetta/lib/jsii/assemblies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,26 @@ export function loadAssemblies(
export async function loadAllDefaultTablets(asms: readonly LoadedAssembly[]): Promise<Record<string, LanguageTablet>> {
return mkDict(
await Promise.all(
asms.map(async (a) => [a.directory, await LanguageTablet.fromOptionalFile(guessTabletLocation(a))] as const),
asms.map(
async (a) => [a.directory, await LanguageTablet.fromOptionalFile(guessTabletLocation(a.directory))] as const,
),
),
);
}

function guessTabletLocation(a: LoadedAssembly): string {
const defaultTablet = path.join(a.directory, DEFAULT_TABLET_NAME);
const compDefaultTablet = path.join(a.directory, DEFAULT_TABLET_NAME_COMPRESSED);
return fs.existsSync(defaultTablet) ? defaultTablet : compDefaultTablet;
}
/**
* Returns the location of the tablet file, either .jsii.tabl.json or .jsii.tabl.json.gz.
* Assumes that a tablet exists in the directory and if not, the ensuing behavior is
* handled by the caller of this function.
*/
export function guessTabletLocation(directory: string) {
return compressedTabletExists(directory)
? path.join(directory, DEFAULT_TABLET_NAME_COMPRESSED)
: path.join(directory, DEFAULT_TABLET_NAME);
}

export function compressedTabletExists(directory: string) {
return fs.existsSync(path.join(directory, DEFAULT_TABLET_NAME_COMPRESSED));
}

export type AssemblySnippetSource =
Expand Down
7 changes: 3 additions & 4 deletions packages/jsii-rosetta/lib/rosetta-reader.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as spec from '@jsii/spec';
import * as path from 'path';

import { allTypeScriptSnippets } from './jsii/assemblies';
import { allTypeScriptSnippets, guessTabletLocation } from './jsii/assemblies';
import { TargetLanguage } from './languages';
import * as logging from './logging';
import { transformMarkdown } from './markdown/markdown';
Expand All @@ -16,7 +15,7 @@ import {
typeScriptSnippetFromSource,
} from './snippet';
import { snippetKey } from './tablets/key';
import { DEFAULT_TABLET_NAME, LanguageTablet, Translation } from './tablets/tablets';
import { LanguageTablet, Translation } from './tablets/tablets';
import { Translator } from './translate';
import { commentToken, pathExists, printDiagnostics } from './util';

Expand Down Expand Up @@ -148,7 +147,7 @@ export class RosettaTabletReader {
* pacmak sends our way later on is not going to be enough to do that.
*/
public async addAssembly(assembly: spec.Assembly, assemblyDir: string) {
const defaultTablet = path.join(assemblyDir, DEFAULT_TABLET_NAME);
const defaultTablet = guessTabletLocation(assemblyDir);
if (await pathExists(defaultTablet)) {
try {
await this.loadTabletFromFile(defaultTablet);
Expand Down
72 changes: 72 additions & 0 deletions packages/jsii-rosetta/test/commands/extract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,78 @@ test('extract can compress implicit tablet file', async () => {
expect(tablet.snippetKeys.length).toEqual(1);
});

describe('extract behavior regarding compressing implicit tablets', () => {
function hasCompressedDefaultTablet(directory: string) {
return (
fs.existsSync(path.join(directory, DEFAULT_TABLET_NAME_COMPRESSED)) &&
!fs.existsSync(path.join(directory, DEFAULT_TABLET_NAME))
);
}

function hasUncompressedDefaultTablet(directory: string) {
return (
!fs.existsSync(path.join(directory, DEFAULT_TABLET_NAME_COMPRESSED)) &&
fs.existsSync(path.join(directory, DEFAULT_TABLET_NAME))
);
}

function hasBothDefaultTablets(directory: string) {
return (
fs.existsSync(path.join(directory, DEFAULT_TABLET_NAME_COMPRESSED)) &&
fs.existsSync(path.join(directory, DEFAULT_TABLET_NAME))
);
}

test('preserve uncompressed tablet by default', async () => {
// Run extract with compressTablet = false to create .jsii.tabl.json file
await extract.extractSnippets([assembly.moduleDirectory], {
...defaultExtractOptions,
compressTablet: false,
});

expect(hasUncompressedDefaultTablet(assembly.moduleDirectory)).toBeTruthy();

await extract.extractSnippets([assembly.moduleDirectory], {
...defaultExtractOptions,
});

expect(hasUncompressedDefaultTablet(assembly.moduleDirectory)).toBeTruthy();
});

test('preserve compressed tablet by default', async () => {
// Run extract with compressTablet = true to create .jsii.tabl.json.gz file
await extract.extractSnippets([assembly.moduleDirectory], {
...defaultExtractOptions,
compressTablet: true,
});

expect(hasCompressedDefaultTablet(assembly.moduleDirectory)).toBeTruthy();

await extract.extractSnippets([assembly.moduleDirectory], {
...defaultExtractOptions,
});

expect(hasCompressedDefaultTablet(assembly.moduleDirectory)).toBeTruthy();
});

test('respects the explicit call to compress the tablet', async () => {
// Run extract with compressTablet = false to create .jsii.tabl.json file
await extract.extractSnippets([assembly.moduleDirectory], {
...defaultExtractOptions,
compressTablet: false,
});

expect(hasUncompressedDefaultTablet(assembly.moduleDirectory)).toBeTruthy();

await extract.extractSnippets([assembly.moduleDirectory], {
...defaultExtractOptions,
compressTablet: true,
});

expect(hasBothDefaultTablets(assembly.moduleDirectory)).toBeTruthy();
});
});

test('extract works from compressed test assembly', async () => {
const compressedAssembly = TestJsiiModule.fromSource(
{
Expand Down
37 changes: 37 additions & 0 deletions packages/jsii-rosetta/test/commands/transliterate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,43 @@ test('will read translations from cache even if they are dirty', async () => {
}
});

test('will read translations from cache when tablet is compressed', async () => {
const infusedAssembly = TestJsiiModule.fromSource(
{
'index.ts': `
/**
* ClassA
*
* @example x
*/
export class ClassA {
public someMethod() {
}
}
`,
},
{
name: 'my_assembly',
jsii: DUMMY_JSII_CONFIG,
},
);
try {
// Run an extract and tell it to compress the tablet
await extractSnippets([infusedAssembly.moduleDirectory], {
compressTablet: true,
});

await transliterateAssembly([infusedAssembly.moduleDirectory], [TargetLanguage.PYTHON]);

const translated: Assembly = JSON.parse(
await fs.promises.readFile(path.join(infusedAssembly.moduleDirectory, '.jsii.python'), 'utf-8'),
);
expect(translated.types?.['my_assembly.ClassA'].docs?.example).toEqual('x');
} finally {
infusedAssembly.cleanup();
}
});

test('will output to specified directory', async () =>
withTemporaryDirectory(async (tmpDir) => {
// GIVEN
Expand Down

0 comments on commit 9768e2a

Please sign in to comment.