Skip to content

Commit

Permalink
#179 Consistently use URI object in APIs (#59)
Browse files Browse the repository at this point in the history
* General Housekeeping

- Remove simple-import-sort eslint plugin as it interferes with the built-in import sort
- Add script to upgrade emfcloud dependencies

* #179 Consistently use URI object in APIs

- Update to latest modelserver-client version
- Use urijs as typed URI library
  - Use URI objects in APIs and internally for proper URI handling
  - Adapt interfaces, tests, custom examples

Part of eclipse-emfcloud/emfcloud#179

Contributed on behalf of STMicroelectronics
  • Loading branch information
ndoschek authored Nov 15, 2022
1 parent 4e5d48f commit e9ebcb4
Show file tree
Hide file tree
Showing 19 changed files with 1,095 additions and 944 deletions.
10 changes: 4 additions & 6 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@
{
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.fixAll.eslint": true
"source.fixAll.eslint": true,
"source.organizeImports": true
},
"eslint.validate": [
"javascript",
"typescript"
],
"eslint.validate": ["javascript", "typescript"],
"prettier.prettierPath": "node_modules/prettier",
"search.exclude": {
"**/node_modules": true,
Expand Down Expand Up @@ -43,4 +41,4 @@
"[yaml]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
}
}
}
2 changes: 1 addition & 1 deletion configs/base.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"jsx": true
}
},
"plugins": ["@typescript-eslint", "header", "import", "simple-import-sort", "no-null", "prettier"],
"plugins": ["@typescript-eslint", "header", "import", "no-null", "prettier"],
"extends": [
"eslint:recommended",
"plugin:@typescript-eslint/eslint-recommended",
Expand Down
10 changes: 1 addition & 9 deletions configs/errors.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,6 @@
"import/export": "off", // we have multiple exports due to namespaces, enums and classes that share the same name
"import/no-deprecated": "error",
// eslint-plugin-no-null
"no-null/no-null": "error",
// simple-import/sort
"sort-imports": "off",
"import/order": "off",
"simple-import-sort/imports": "error",
"simple-import-sort/exports": "error",
"import/first": "error",
"import/newline-after-import": "error",
"import/no-duplicates": "error"
"no-null/no-null": "error"
}
}
9 changes: 5 additions & 4 deletions examples/custom-command-example/src/example-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,19 @@ class IncrementDurationCommandProvider implements CommandProvider {
}

getCommands(_modelUri: URI, customCommand: ModelServerCommand): Transaction {
const [modelURI, elementID] = customCommand.owner.$ref.split('#');
const commandModelUri = new URI(customCommand.owner.$ref).fragment('');
const elementID = new URI(customCommand.owner.$ref).fragment();

return async executor => {
const element = await this.modelServerClient.getElementById(modelURI, elementID, isTask);
const element = await this.modelServerClient.getElementById(commandModelUri, elementID, isTask);

if (element) {
this.logger.debug('Got an element: %s.', element);
const oldDuration = element.duration ?? 0;
const newDuration = oldDuration + 1;
this.logger.debug(`Setting duration to ${newDuration}.`);

const setOp = replace(modelURI, element, 'duration', newDuration);
const setOp = replace(commandModelUri, element, 'duration', newDuration);
const executionResult = (await executor.applyPatch(setOp)).patch;

if (!executionResult || !executionResult.length) {
Expand All @@ -83,7 +84,7 @@ class IncrementDurationCommandProvider implements CommandProvider {
: undefined;
if (newValue) {
const newNewDuration = oldDuration + newValue;
const doubleOp = replace(modelURI, element, 'duration', newNewDuration);
const doubleOp = replace(commandModelUri, element, 'duration', newNewDuration);
this.logger.debug(`Updating duration to ${newNewDuration}.`);
return executor.applyPatch(doubleOp).then(() => true);
}
Expand Down
4 changes: 2 additions & 2 deletions examples/trigger-example/src/example-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class IncrementDurationTriggerProvider implements TriggerProvider {
// we have all the information we need in the `modelDelta`. But in the general case,
// a trigger provider will need to find other related objects in the model and generate
// new changes from those, so that's how we do it here
const model = await this.modelServerClient.get(modelURI.toString());
const model = await this.modelServerClient.get(modelURI);
const result: Operation[] = [];

// Take only the last change to any given duration in case of multiple (such as
Expand All @@ -66,7 +66,7 @@ class IncrementDurationTriggerProvider implements TriggerProvider {
// Don't update if already a multiple of ten
if (element && op.value % 10 !== 0) {
this.logger.debug('Rounding up duration %d of object at %s', op.value, op.path);
result.push(replace(modelURI.toString(), element, 'duration', roundUp(op.value, 10)));
result.push(replace(modelURI, element, 'duration', roundUp(op.value, 10)));
}
});

Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,16 @@
"start": "yarn --cwd examples/example-server start",
"publish:prepare": "lerna version minor --exact --ignore-scripts --yes --no-push",
"publish:latest": "lerna publish from-git --no-git-reset --no-verify-access --no-push",
"publish:next": "SHA=$(git rev-parse --short HEAD) && lerna publish preminor --exact --canary --preid next.${SHA} --dist-tag next --no-git-reset --no-git-tag-version --no-push --ignore-scripts --yes --no-verify-access"
"publish:next": "SHA=$(git rev-parse --short HEAD) && lerna publish preminor --exact --canary --preid next.${SHA} --dist-tag next --no-git-reset --no-git-tag-version --no-push --ignore-scripts --yes --no-verify-access",
"upgrade:next": "yarn upgrade -p \"@eclipse-emfcloud.*\" --next "
},
"devDependencies": {
"@sinonjs/referee": "^9.1.1",
"@types/chai": "^4.3.0",
"@types/chai-like": "^1.1.1",
"@types/mocha": "^9.1.0",
"@types/sinon": "^10.0.11",
"@types/urijs": "^1.19.19",
"@typescript-eslint/eslint-plugin": "^5.38.1",
"@typescript-eslint/parser": "^5.38.1",
"chai": "^4.3.6",
Expand All @@ -34,7 +36,6 @@
"eslint-plugin-import": "^2.22.1",
"eslint-plugin-no-null": "^1.0.2",
"eslint-plugin-prettier": "^4.0.0",
"eslint-plugin-simple-import-sort": "^7.0.0",
"ignore-styles": "^5.0.1",
"lerna": "^4.0.0",
"mocha": "^9.2.1",
Expand Down
76 changes: 38 additions & 38 deletions packages/modelserver-node/src/client/model-server-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export class InternalModelServerClient implements InternalModelServerClientApi {
port: this.upstreamConnectionConfig.serverPort,
path: this.upstreamConnectionConfig.baseURL
});
return this.delegate.initialize(this._baseURL.toString(), DEFAULT_FORMAT);
return this.delegate.initialize(this._baseURL, DEFAULT_FORMAT);
}

async openTransaction(modelUri: URI): Promise<TransactionContext> {
Expand All @@ -182,7 +182,7 @@ export class InternalModelServerClient implements InternalModelServerClientApi {

const clientID = uuid();

return axios.post(this.makeURL('transaction', { modeluri: modelUri.toString() }), { data: clientID }).then(response => {
return axios.post(this.makeURL('transaction', modelUri), { data: clientID }).then(response => {
const { uri: transactionURI } = (response.data as CreateTransactionResponseBody).data;
const result = new DefaultTransactionContext(
transactionURI,
Expand All @@ -202,10 +202,10 @@ export class InternalModelServerClient implements InternalModelServerClientApi {
}
}

protected makeURL(path: string, queryParameters?: any): string {
protected makeURL(path: string, modeluri: URI): string {
const uri = this._baseURL.clone();
uri.path(URI.joinPaths(uri.path(), path).toString());
uri.addQuery(queryParameters);
path.split('/').forEach(seg => uri.segment(seg));
uri.addQuery('modeluri', modeluri);
return uri.toString();
}

Expand All @@ -222,9 +222,9 @@ export class InternalModelServerClient implements InternalModelServerClientApi {
}
return this.delegate.getAll(format);
}
get(modeluri: string, format?: Format): Promise<AnyObject>;
get<M>(modeluri: string, typeGuard: TypeGuard<M>, format?: Format): Promise<M>;
get<M>(modeluri: string, typeGuard?: Format | TypeGuard<M>, format?: Format): Promise<AnyObject | M> {
get(modeluri: URI, format?: Format): Promise<AnyObject>;
get<M>(modeluri: URI, typeGuard: TypeGuard<M>, format?: Format): Promise<M>;
get<M>(modeluri: URI, typeGuard?: Format | TypeGuard<M>, format?: Format): Promise<AnyObject | M> {
if (!typeGuard) {
return this.delegate.get(modeluri, format);
}
Expand All @@ -233,12 +233,12 @@ export class InternalModelServerClient implements InternalModelServerClientApi {
}
return this.delegate.get(modeluri, typeGuard);
}
getModelUris(): Promise<string[]> {
getModelUris(): Promise<URI[]> {
return this.delegate.getModelUris();
}
getElementById(modeluri: string, elementid: string, format?: Format): Promise<AnyObject>;
getElementById<M>(modeluri: string, elementid: string, typeGuard: TypeGuard<M>): Promise<M>;
getElementById<M>(modeluri: string, elementid: string, typeGuard?: Format | TypeGuard<M>, format?: Format): Promise<AnyObject | M> {
getElementById(modeluri: URI, elementid: string, format?: Format): Promise<AnyObject>;
getElementById<M>(modeluri: URI, elementid: string, typeGuard: TypeGuard<M>): Promise<M>;
getElementById<M>(modeluri: URI, elementid: string, typeGuard?: Format | TypeGuard<M>, format?: Format): Promise<AnyObject | M> {
if (!typeGuard) {
return this.delegate.getElementById(modeluri, elementid, format);
}
Expand All @@ -247,9 +247,9 @@ export class InternalModelServerClient implements InternalModelServerClientApi {
}
return this.delegate.getElementById(modeluri, elementid, typeGuard, format);
}
getElementByName(modeluri: string, elementname: string, format?: Format): Promise<AnyObject>;
getElementByName<M>(modeluri: string, elementname: string, typeGuard: TypeGuard<M>): Promise<M>;
getElementByName<M>(modeluri: string, elementname: string, typeGuard?: Format | TypeGuard<M>, format?: Format): Promise<AnyObject | M> {
getElementByName(modeluri: URI, elementname: string, format?: Format): Promise<AnyObject>;
getElementByName<M>(modeluri: URI, elementname: string, typeGuard: TypeGuard<M>): Promise<M>;
getElementByName<M>(modeluri: URI, elementname: string, typeGuard?: Format | TypeGuard<M>, format?: Format): Promise<AnyObject | M> {
if (!typeGuard) {
return this.delegate.getElementByName(modeluri, elementname, format);
}
Expand All @@ -258,15 +258,15 @@ export class InternalModelServerClient implements InternalModelServerClientApi {
}
return this.delegate.getElementByName(modeluri, elementname, typeGuard, format);
}
delete(modeluri: string): Promise<boolean> {
delete(modeluri: URI): Promise<boolean> {
return this.delegate.delete(modeluri);
}
close(modeluri: string): Promise<boolean> {
close(modeluri: URI): Promise<boolean> {
return this.delegate.close(modeluri);
}
create(modeluri: string, model: string | AnyObject, format?: Format): Promise<AnyObject>;
create<M>(modeluri: string, model: string | AnyObject, typeGuard: TypeGuard<M>, format?: Format): Promise<M>;
create<M>(modeluri: string, model: string, typeGuard?: Format | TypeGuard<M>, format?: Format): Promise<AnyObject | M> {
create(modeluri: URI, model: string | AnyObject, format?: Format): Promise<AnyObject>;
create<M>(modeluri: URI, model: string | AnyObject, typeGuard: TypeGuard<M>, format?: Format): Promise<M>;
create<M>(modeluri: URI, model: string, typeGuard?: Format | TypeGuard<M>, format?: Format): Promise<AnyObject | M> {
if (!typeGuard) {
return this.delegate.create(modeluri, model, format);
}
Expand All @@ -275,9 +275,9 @@ export class InternalModelServerClient implements InternalModelServerClientApi {
}
return this.delegate.create(modeluri, model, typeGuard, format);
}
update(modeluri: string, model: AnyObject | string, format?: Format): Promise<AnyObject>;
update<M>(modeluri: string, model: AnyObject | string, typeGuard: TypeGuard<M>, format?: Format): Promise<M>;
update<M>(modeluri: string, model: AnyObject | string, typeGuard?: Format | TypeGuard<M>, format?: Format): Promise<AnyObject | M> {
update(modeluri: URI, model: AnyObject | string, format?: Format): Promise<AnyObject>;
update<M>(modeluri: URI, model: AnyObject | string, typeGuard: TypeGuard<M>, format?: Format): Promise<M>;
update<M>(modeluri: URI, model: AnyObject | string, typeGuard?: Format | TypeGuard<M>, format?: Format): Promise<AnyObject | M> {
if (!typeGuard) {
return this.delegate.update(modeluri, model, format);
}
Expand All @@ -286,19 +286,19 @@ export class InternalModelServerClient implements InternalModelServerClientApi {
}
return this.delegate.update(modeluri, model, typeGuard, format);
}
save(modeluri: string): Promise<boolean> {
save(modeluri: URI): Promise<boolean> {
return this.delegate.save(modeluri);
}
saveAll(): Promise<boolean> {
return this.delegate.saveAll();
}
validate(modeluri: string): Promise<Diagnostic> {
validate(modeluri: URI): Promise<Diagnostic> {
return this.delegate.validate(modeluri);
}
getValidationConstraints(modeluri: string): Promise<string> {
getValidationConstraints(modeluri: URI): Promise<string> {
return this.delegate.getValidationConstraints(modeluri);
}
getTypeSchema(modeluri: string): Promise<string> {
getTypeSchema(modeluri: URI): Promise<string> {
return this.delegate.getTypeSchema(modeluri);
}
getUiSchema(schemaname: string): Promise<string> {
Expand All @@ -310,9 +310,9 @@ export class InternalModelServerClient implements InternalModelServerClientApi {
ping(): Promise<boolean> {
return this.delegate.ping();
}
edit(modeluri: string, patch: Operation | Operation[], format?: Format): Promise<ModelUpdateResult>;
edit(modeluri: string, command: ModelServerCommand, format?: Format): Promise<ModelUpdateResult>;
edit(modeluri: string, patchOrCommand: Operation | Operation[] | ModelServerCommand): Promise<ModelUpdateResult> {
edit(modeluri: URI, patch: Operation | Operation[], format?: Format): Promise<ModelUpdateResult>;
edit(modeluri: URI, command: ModelServerCommand, format?: Format): Promise<ModelUpdateResult>;
edit(modeluri: URI, patchOrCommand: Operation | Operation[] | ModelServerCommand): Promise<ModelUpdateResult> {
if (patchOrCommand instanceof ModelServerCommand) {
return this.delegate.edit(modeluri, patchOrCommand);
}
Expand All @@ -321,20 +321,20 @@ export class InternalModelServerClient implements InternalModelServerClientApi {
}
return this.delegate.edit(modeluri, patchOrCommand);
}
undo(modeluri: string): Promise<ModelUpdateResult> {
undo(modeluri: URI): Promise<ModelUpdateResult> {
return this.delegate.undo(modeluri);
}
redo(modeluri: string): Promise<ModelUpdateResult> {
redo(modeluri: URI): Promise<ModelUpdateResult> {
return this.delegate.redo(modeluri);
}
subscribe(modeluri: string, listener: SubscriptionListener, options?: SubscriptionOptions): SubscriptionListener {
subscribe(modeluri: URI, listener: SubscriptionListener, options?: SubscriptionOptions): SubscriptionListener {
return this.delegate.subscribe(modeluri, listener, options);
}
send(modelUri: string, message: ModelServerMessage<unknown>): void {
return this.delegate.send(modelUri, message);
send(modeluri: URI, message: ModelServerMessage<unknown>): void {
return this.delegate.send(modeluri, message);
}
unsubscribe(modelUri: string): void {
return this.delegate.unsubscribe(modelUri);
unsubscribe(modeluri: URI): void {
return this.delegate.unsubscribe(modeluri);
}
}

Expand Down Expand Up @@ -681,7 +681,7 @@ class DefaultTransactionContext implements TransactionContext {
protected message(type: MessageBody['type'], data: unknown = {}): MessageBody {
return {
type,
modelUri: this.modelURI.toString(),
modeluri: this.modelURI,
data
};
}
Expand Down
4 changes: 2 additions & 2 deletions packages/modelserver-node/src/routes/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ export class ModelsRoutes implements RouteProvider {
const isCreate = req.method.toUpperCase() === 'POST';
this.logger.debug(`Delegating ${isCreate ? 'creation' : 'update'} of ${modeluri.toString()}.`);
const delegated = isCreate //
? this.modelServerClient.create(modeluri.toString(), model, format)
: this.modelServerClient.update(modeluri.toString(), model, format);
? this.modelServerClient.create(modeluri, model, format)
: this.modelServerClient.update(modeluri, model, format);

delegated.then(this.performModelValidation(modeluri)).then(relay(res)).catch(handleError(res));
};
Expand Down
4 changes: 3 additions & 1 deletion packages/modelserver-node/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import * as expressWS from 'express-ws';
import { WebsocketRequestHandler } from 'express-ws';
import * as http from 'http';
import { inject, injectable, multiInject, named, optional, postConstruct } from 'inversify';
import * as URI from 'urijs';
import * as WebSocket from 'ws';

import { InternalModelServerClientApi } from './client/model-server-client';
Expand Down Expand Up @@ -138,7 +139,8 @@ export class ModelServer {
// Use provided after-middlewares that are applicable globally
this.middlewareProviders.flatMap(p => p.getAfterMiddlewares?.(app) ?? []).forEach(mw => app.use(mw));

const upstream = axios.create({ baseURL: `http://localhost:${upstreamPort}/` });
const baseURL = new URI({ protocol: 'http', hostname: 'localhost', port: upstreamPort });
const upstream = axios.create({ baseURL: baseURL.toString() });

app.all('*', this.forward(upstream));
app.ws('*', this.forwardWS(upstream));
Expand Down
4 changes: 2 additions & 2 deletions packages/modelserver-node/src/services/edit-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ export class EditService {

if (isModelServerCommand(providedEdit)) {
// Command case
result = this.modelServerClient.edit(modelURI.toString(), providedEdit);
result = this.modelServerClient.edit(modelURI, providedEdit);
} else {
// JSON Patch case
result = this.modelServerClient.edit(modelURI.toString(), providedEdit);
result = this.modelServerClient.edit(modelURI, providedEdit);
}

return result.then(this.performPatchValidation(modelURI));
Expand Down
4 changes: 2 additions & 2 deletions packages/modelserver-node/src/services/validation-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class ValidationManager {
async validate(modelURI: URI): Promise<Diagnostic> {
let model: ModelServerObjectV2;
try {
model = await this.modelServerClient.get(modelURI.toString()).then(asModelServerObject);
model = await this.modelServerClient.get(modelURI).then(asModelServerObject);
if (!model) {
throw new Error(`Could not retrieve model '${modelURI}' to validate.`);
}
Expand All @@ -61,7 +61,7 @@ export class ValidationManager {
}

this.logger.debug(`Performing core validation of ${modelURI}`);
const defaultDiagnostic = await this.modelServerClient.validate(modelURI.toString());
const defaultDiagnostic = await this.modelServerClient.validate(modelURI);

this.logger.debug(`Performing custom validation of ${modelURI}`);
const providedDiagnostic = await this.validationProviderRegistry.validate(model, modelURI);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,14 @@ class TestCommandProvider implements CommandProvider {
console.error('custom command owner was unexpectedly undefined');
return async () => false;
}
const [modelURI] = customCommand.owner.$ref.split('#');

const commandModelUri = new URI(customCommand.owner.$ref).fragment('');

return async executor => {
const obj = { $id: 'any-id', $type: 'any-type', label: 'anyLabel' };

if (obj) {
const setCommand = replace(modelURI, obj, 'label', 'newLabel');
const setCommand = replace(commandModelUri, obj, 'label', 'newLabel');
const executionResult = (await executor.applyPatch(setCommand)).patch;
if (!executionResult || !executionResult.length) {
return false; // Failed to complete the chain, so roll back
Expand Down
Loading

0 comments on commit e9ebcb4

Please sign in to comment.