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

refactor: Specify common types TDE-1030 #849

Closed
wants to merge 40 commits into from
Closed
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
25f890c
refactor: Specify common branded types
l0b0 Feb 1, 2024
243ceeb
docs: Explain branded types
l0b0 Feb 1, 2024
a119ba7
chore: Remove unused special case for JSON input
l0b0 Feb 2, 2024
907f18a
refactor: Use built-in `URL` type
l0b0 Feb 2, 2024
8f8c146
Merge remote-tracking branch 'origin/master' into refactor/types
l0b0 Feb 6, 2024
8d716c3
fix: Revert wrong use of base64url encoding
l0b0 Feb 7, 2024
3fe4a21
refactor: Re-use `UrlParser`
l0b0 Feb 7, 2024
9ca2b9e
fix: Handle local path in `$ACTION_PATH`
l0b0 Feb 7, 2024
aeeb40a
refactor: Use `UrlParser` instead of `tryParseUrl`
l0b0 Feb 7, 2024
48f5b2d
refactor: Use `fsa.join` instead of `normaliseHref`
l0b0 Feb 7, 2024
cf7aad3
fix: Use `URL` for STAC URL concatenation
l0b0 Feb 7, 2024
ab8603d
build(deps): Bump the chunkd group with 3 updates
dependabot[bot] Feb 1, 2024
73e08c1
fix: Work with latest chunkd update
l0b0 Feb 7, 2024
e0f9241
refactor: fixup dependencies
blacha Feb 7, 2024
a5aa796
build: remove esno as its not used
blacha Feb 7, 2024
f341b8f
refactor: fixup linting
blacha Feb 7, 2024
f4f916d
refactor: apply linter
blacha Feb 7, 2024
67a8bc0
refactor: correct import locations
blacha Feb 7, 2024
1eeb1cc
refactor: createTiff is no longer needed
blacha Feb 7, 2024
804b355
refactor: fixup tests for filter
blacha Feb 7, 2024
86cfa0d
refactor: correct sample.json loading
blacha Feb 7, 2024
40581de
refactor: missed a few tests
blacha Feb 7, 2024
2cd22d5
fix: correct logic of relative test
blacha Feb 7, 2024
c6dc92b
Merge remote-tracking branch 'origin/master' into refactor/types
l0b0 Feb 8, 2024
fa98b63
Update src/commands/copy/copy.ts
l0b0 Feb 8, 2024
1f36e73
refactor: Join imports
l0b0 Feb 8, 2024
3b5eb17
fix: Use absolute path for file URL
l0b0 Feb 12, 2024
dcd9868
chore: Uncomment test
l0b0 Feb 12, 2024
f09b7d3
chore: Remove redundant lookup of `URL.href`
l0b0 Feb 12, 2024
5306cc8
refactor: Use existing `fsa.toUrl`
l0b0 Feb 12, 2024
273235e
refactor: Simplify by using synchronous URL parser
l0b0 Feb 12, 2024
cc9ddb1
Merge remote-tracking branch 'origin/master' into refactor/types
l0b0 Feb 12, 2024
87c913f
fix: Pass plain strings to workers
l0b0 Feb 13, 2024
894218c
Update src/commands/basemaps-mapsheet/create-mapsheet.ts
l0b0 Feb 13, 2024
1a26e94
refactor: Let `URL` constructor deal with relative `path`s
l0b0 Feb 13, 2024
767edc0
fix: Syntax and type
l0b0 Feb 13, 2024
fc92748
Merge remote-tracking branch 'origin/master' into refactor/types
l0b0 Feb 19, 2024
ffd1d02
refactor: Simplify URL generation
l0b0 Feb 19, 2024
8191600
refactor: Create URL with library method
l0b0 Feb 19, 2024
79737fc
Merge remote-tracking branch 'origin/master' into refactor/types
l0b0 Feb 23, 2024
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
792 changes: 52 additions & 740 deletions package-lock.json

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@
"organization": true
},
"devDependencies": {
"@chunkd/source-memory": "^10.1.0",
"@chunkd/source-memory": "^11.0.2",
"@linzjs/style": "^5.1.0",
"@types/node": "^20.8.9",
"@types/prettier": "^3.0.0",
"esno": "^0.17.0",
"stac-ts": "^1.0.0"
},
"scripts": {
Expand All @@ -40,9 +39,10 @@
"@aws-sdk/lib-storage": "^3.440.0",
"@basemaps/config": "^6.46.0",
"@basemaps/geo": "^6.44.0",
"@chunkd/fs": "^10.0.9",
"@chunkd/source-aws-v3": "^10.1.3",
"@cogeotiff/core": "^8.1.1",
"@chunkd/fs": "^11.2.0",
"@chunkd/fs-aws": "^11.2.1",
"@chunkd/source-aws": "^11.0.2",
"@cogeotiff/core": "^9.0.3",
"@linzjs/geojson": "^6.43.0",
"@octokit/core": "^5.0.0",
"@octokit/plugin-rest-endpoint-methods": "^10.1.1",
Expand Down
2 changes: 1 addition & 1 deletion src/commands/basemaps-github/create-pr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ async function parseTargetInfo(
}

if (epsg == null || name == null) throw new Error(`Invalid target ${target} to parse the epsg and imagery name.`);
const collectionPath = fsa.join(target, 'collection.json');
const collectionPath = new URL('collection.json', target);
const collection = await fsa.readJson<StacCollection>(collectionPath);
if (collection == null) throw new Error(`Failed to get target collection json from ${collectionPath}.`);
const title = collection.title;
Expand Down
7 changes: 3 additions & 4 deletions src/commands/basemaps-github/make.cog.github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
TileSetType,
} from '@basemaps/config/build/config/tile.set.js';
import { TileSetConfigSchema } from '@basemaps/config/build/json/parse.tile.set.js';
import { fsa } from '@chunkd/fs';

import { logger } from '../../log.js';
import { DEFAULT_PRETTIER_FORMAT } from '../../utils/config.js';
Expand Down Expand Up @@ -83,13 +82,13 @@ export class MakeCogGithub {
layers: [layer],
};
const content = await prettyPrint(JSON.stringify(tileSet, null, 2), ConfigPrettierFormat);
const tileSetPath = fsa.joinAll('config', 'tileset', region, `${layer.name}.json`);
const tileSetPath = `config/tileset/${region}/${layer.name}.json`;
const file = { path: tileSetPath, content };
// Github create pull request
await createPR(gh, branch, title, botEmail, [file]);
} else {
// Prepare new aerial tileset config
const tileSetPath = fsa.joinAll('config', 'tileset', `${filename}.json`);
const tileSetPath = `config/tileset/${filename}.json`;
const tileSetContent = await gh.getContent(tileSetPath);
const tileSet = JSON.parse(tileSetContent) as ConfigTileSetRaster;
const newTileSet = await this.prepareRasterTileSetConfig(layer, tileSet, category);
Expand Down Expand Up @@ -176,7 +175,7 @@ export class MakeCogGithub {

// Prepare new aerial tileset config
logger.info({ imagery: this.imagery }, 'GitHub: Get the master TileSet config file');
const tileSetPath = fsa.joinAll('config', 'tileset', `${filename}.json`);
const tileSetPath = `config/tileset/${filename}.json`;
const tileSetContent = await gh.getContent(tileSetPath);
const tileSet = JSON.parse(tileSetContent) as ConfigTileSetVector;
const newTileSet = await this.prepareVectorTileSetConfig(layer, tileSet);
Expand Down
27 changes: 14 additions & 13 deletions src/commands/basemaps-mapsheet/create-mapsheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { gunzip } from 'zlib';

import { CliInfo } from '../../cli.info.js';
import { logger } from '../../log.js';
import { UrlParser } from '../../utils/parsers.js';
import { registerCli, verbose } from '../common.js';

const gunzipProm = promisify(gunzip);
Expand All @@ -22,9 +23,9 @@ export function isGzip(b: Buffer): boolean {
*
* If the file ends with .gz or is a GZIP like {@link isGzip} file it will automatically be decompressed.
*/
async function readConfig(config: string): Promise<ConfigBundled> {
async function readConfig(config: URL): Promise<ConfigBundled> {
const obj = await fsa.read(config);
if (config.endsWith('.gz') || isGzip(obj)) {
if (config.href.endsWith('.gz') || isGzip(obj)) {
const data = await gunzipProm(obj);
return JSON.parse(data.toString());
}
Expand All @@ -39,17 +40,17 @@ interface Output {
export const CommandCreateMapSheetArgs = {
verbose,
path: option({
type: string,
type: UrlParser,
long: 'path',
description: 'Path of flatgeobuf, this can be both a local path or s3 location',
}),
bmConfig: option({
type: string,
type: UrlParser,
long: 'bm-config',
description: 'Path of basemaps config json, this can be both a local path or s3 location',
}),
output: option({
type: string,
type: UrlParser,
long: 'output',
description: 'Output of the map sheet file',
}),
Expand All @@ -72,30 +73,30 @@ export const basemapsCreateMapSheet = command({
args: CommandCreateMapSheetArgs,
async handler(args) {
registerCli(this, args);
const path = args.path;
const url = args.path;
const config = args.bmConfig;
const outputPath = args.output;
const outputUrl = args.output;

const include = args.include ? new RegExp(args.include.toLowerCase(), 'i') : undefined;
const exclude = args.exclude ? new RegExp(args.exclude.toLowerCase(), 'i') : undefined;

logger.info({ path }, 'MapSheet:LoadFgb');
const buf = await fsa.read(path);
logger.info({ path: url }, 'MapSheet:LoadFgb');
const buf = await fsa.read(url);
logger.info({ config }, 'MapSheet:LoadConfig');
const configJson = await readConfig(config);
const mem = ConfigProviderMemory.fromJson(configJson);

const rest = fgb.deserialize(buf) as FeatureCollection;
const featuresWritePromise = fsa.write('features.json', JSON.stringify(rest));
const featuresWritePromise = fsa.write(new URL(`file://${process.cwd()}/features.json`), JSON.stringify(rest));
l0b0 marked this conversation as resolved.
Show resolved Hide resolved

const aerial = await mem.TileSet.get('ts_aerial');
if (aerial == null) throw new Error('Invalid config file.');

logger.info({ path, config }, 'MapSheet:CreateMapSheet');
logger.info({ path: url, config }, 'MapSheet:CreateMapSheet');
const outputs = await createMapSheet(aerial, mem, rest, include, exclude);

logger.info({ outputPath }, 'MapSheet:WriteOutput');
const outputWritePromise = fsa.write(outputPath, JSON.stringify(outputs, null, 2));
logger.info({ outputPath: outputUrl }, 'MapSheet:WriteOutput');
const outputWritePromise = fsa.write(outputUrl, JSON.stringify(outputs, null, 2));

await Promise.all([featuresWritePromise, outputWritePromise]);
},
Expand Down
43 changes: 1 addition & 42 deletions src/commands/common.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { fsa } from '@chunkd/fs';
import { CogTiff } from '@cogeotiff/core';
import { boolean, flag, option, optional, string } from 'cmd-ts';
import pLimit from 'p-limit';
import { fileURLToPath, pathToFileURL } from 'url';
import { fileURLToPath } from 'url';

import { CliInfo } from '../cli.info.js';
import { registerFileSystem } from '../fs.register.js';
Expand Down Expand Up @@ -80,44 +77,6 @@ export function parseSize(size: string): number {
return Math.round(fileSize);
}

/** Limit fetches to 25 concurrently **/
const TiffQueue = pLimit(25);

/**
* There is a minor difference between @chunkd/core and @cogeotiff/core
* because @chunkd/core is a major version behind, when it upgrades this can be removed
*
* Because the major version upgrade for chunkd is a lot of work skip it for now (2023-11)
*
* @param loc location to load the tiff from
* @returns Initialized tiff
*/
export function createTiff(loc: string): Promise<CogTiff> {
const source = fsa.source(loc);

const tiff = new CogTiff({
url: tryParseUrl(loc),
fetch: (offset, length): Promise<ArrayBuffer> => {
/** Limit fetches concurrency see {@link TiffQueue} **/
return TiffQueue(() => source.fetchBytes(offset, length));
},
});
return tiff.init();
}

/**
* Attempt to parse a location as a string as a URL,
*
* Relative paths will be converted into file urls.
*/
function tryParseUrl(loc: string): URL {
try {
return new URL(loc);
} catch (e) {
return pathToFileURL(loc);
}
}

/**
* When chunkd moves to URLs this can be removed
*
Expand Down
63 changes: 31 additions & 32 deletions src/commands/copy/__test__/copy.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import assert from 'node:assert';
import { beforeEach, describe, it } from 'node:test';

import { fsa } from '@chunkd/fs';
import { FsMemory } from '@chunkd/source-memory';
import { fsa, FsMemory } from '@chunkd/fs';

import { worker } from '../copy-worker.js';

Expand All @@ -16,22 +15,22 @@ describe('copyFiles', () => {

it('should copy to the target location', async () => {
await Promise.all([
fsa.write('memory://source/topographic.json', Buffer.from(JSON.stringify({ test: true })), {
fsa.write(new URL('memory://source/topographic.json'), Buffer.from(JSON.stringify({ test: true })), {
contentType: 'application/json',
}),
fsa.write('memory://source/foo/bar/topographic.png', Buffer.from('test'), { contentType: 'image/png' }),
fsa.write(new URL('memory://source/foo/bar/topographic.png'), Buffer.from('test'), { contentType: 'image/png' }),
]);

await worker.routes.copy({
id: '1',
manifest: [
{
source: 'memory://source/topographic.json',
target: 'memory://target/topographic.json',
source: new URL('memory://source/topographic.json'),
target: new URL('memory://target/topographic.json'),
},
{
source: 'memory://source/foo/bar/topographic.png',
target: 'memory://target/topographic.png',
source: new URL('memory://source/foo/bar/topographic.png'),
target: new URL('memory://target/topographic.png'),
},
],
start: 0,
Expand All @@ -42,16 +41,16 @@ describe('copyFiles', () => {
});

const [jsonSource, jsonTarget] = await Promise.all([
fsa.head('memory://source/topographic.json'),
fsa.head('memory://target/topographic.json'),
fsa.head(new URL('memory://source/topographic.json')),
fsa.head(new URL('memory://target/topographic.json')),
]);

assert.equal(jsonTarget?.contentType, 'application/json');
assert.equal(jsonSource?.contentType, 'application/json');

const [pngSource, pngTarget] = await Promise.all([
fsa.head('memory://source/foo/bar/topographic.png'),
fsa.head('memory://target/topographic.png'),
fsa.head(new URL('memory://source/foo/bar/topographic.png')),
fsa.head(new URL('memory://target/topographic.png')),
]);

assert.equal(pngTarget?.contentType, 'image/png');
Expand All @@ -66,23 +65,23 @@ describe('copyFiles', () => {

it('should default to COG/json', async () => {
await Promise.all([
fsa.write('memory://source/topographic.json', Buffer.from(JSON.stringify({ test: true })), {
fsa.write(new URL('memory://source/topographic.json'), Buffer.from(JSON.stringify({ test: true })), {
contentType: 'application/octet-stream',
}),
fsa.write('memory://source/foo/bar/topographic.tiff', Buffer.from('test'), {
fsa.write(new URL('memory://source/foo/bar/topographic.tiff'), Buffer.from('test'), {
contentType: 'binary/octet-stream',
}),
]);
await worker.routes.copy({
id: '1',
manifest: [
{
source: 'memory://source/topographic.json',
target: 'memory://target/topographic.json',
source: new URL('memory://source/topographic.json'),
target: new URL('memory://target/topographic.json'),
},
{
source: 'memory://source/foo/bar/topographic.tiff',
target: 'memory://target/topographic.tiff',
source: new URL('memory://source/foo/bar/topographic.tiff'),
target: new URL('memory://target/topographic.tiff'),
},
],
start: 0,
Expand All @@ -92,16 +91,16 @@ describe('copyFiles', () => {
fixContentType: true,
});
const [jsonSource, jsonTarget] = await Promise.all([
fsa.head('memory://source/topographic.json'),
fsa.head('memory://target/topographic.json'),
fsa.head(new URL('memory://source/topographic.json')),
fsa.head(new URL('memory://target/topographic.json')),
]);

assert.equal(jsonSource?.contentType, 'application/octet-stream');
assert.equal(jsonTarget?.contentType, 'application/json');

const [tiffSource, tiffTarget] = await Promise.all([
fsa.head('memory://source/foo/bar/topographic.tiff'),
fsa.head('memory://target/topographic.tiff'),
fsa.head(new URL('memory://source/foo/bar/topographic.tiff')),
fsa.head(new URL('memory://target/topographic.tiff')),
]);

assert.equal(tiffSource?.contentType, 'binary/octet-stream');
Expand All @@ -110,23 +109,23 @@ describe('copyFiles', () => {

it('should not default COG/json when fixContentType=false', async () => {
await Promise.all([
fsa.write('memory://source/topographic.json', Buffer.from(JSON.stringify({ test: true })), {
fsa.write(new URL('memory://source/topographic.json'), Buffer.from(JSON.stringify({ test: true })), {
contentType: 'application/octet-stream',
}),
fsa.write('memory://source/foo/bar/topographic.tiff', Buffer.from('test'), {
fsa.write(new URL('memory://source/foo/bar/topographic.tiff'), Buffer.from('test'), {
contentType: 'binary/octet-stream',
}),
]);
await worker.routes.copy({
id: '1',
manifest: [
{
source: 'memory://source/topographic.json',
target: 'memory://target/topographic.json',
source: new URL('memory://source/topographic.json'),
target: new URL('memory://target/topographic.json'),
},
{
source: 'memory://source/foo/bar/topographic.tiff',
target: 'memory://target/topographic.tiff',
source: new URL('memory://source/foo/bar/topographic.tiff'),
target: new URL('memory://target/topographic.tiff'),
},
],
start: 0,
Expand All @@ -136,16 +135,16 @@ describe('copyFiles', () => {
fixContentType: false,
});
const [jsonSource, jsonTarget] = await Promise.all([
fsa.head('memory://source/topographic.json'),
fsa.head('memory://target/topographic.json'),
fsa.head(new URL('memory://source/topographic.json')),
fsa.head(new URL('memory://target/topographic.json')),
]);

assert.equal(jsonSource?.contentType, 'application/octet-stream');
assert.equal(jsonTarget?.contentType, 'application/octet-stream');

const [tiffSource, tiffTarget] = await Promise.all([
fsa.head('memory://source/foo/bar/topographic.tiff'),
fsa.head('memory://target/topographic.tiff'),
fsa.head(new URL('memory://source/foo/bar/topographic.tiff')),
fsa.head(new URL('memory://target/topographic.tiff')),
]);

assert.equal(tiffSource?.contentType, 'binary/octet-stream');
Expand Down
2 changes: 1 addition & 1 deletion src/commands/copy/copy-rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export interface CopyContractArgs {
/** Copy ID for tracing */
id: string;
/** List of files that need to be copied */
manifest: { source: string; target: string }[];
manifest: { source: URL; target: URL }[];
Copy link
Member

Choose a reason for hiding this comment

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

can worker threads actually pass URLS? I thought everything was meant to be a POJO for passing around?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume you mean either JSON or JS object, not "Plain old Java object". I have no idea how WorkerRpc works; I guess since you ask we don't have tests for this message passing?

Copy link
Member

Choose a reason for hiding this comment

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

worker_threads are a mechanisim of using small scripts to add more threads to do processing, they are limited to posting messages to/from the thread using a rpc type mechanisim, WorkerRpc wraps the posting so it is somewhat typesafe.

I am not sure we can pass a URL between threads as its not really a easily searliazable thing, just like you cant pass a date but you can pass a ISO string.

/** Offset into the manifest to start at */
start: number;
/** Number of records to copy */
Expand Down
Loading
Loading