Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
113 changes: 103 additions & 10 deletions src/services/jsonSchemaService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as Parser from '../parser/jsonParser';
import { SchemaRequestService, WorkspaceContextService, PromiseConstructor, Thenable, MatchingSchema, TextDocument } from '../jsonLanguageTypes';

import * as nls from 'vscode-nls';
import { createRegex} from '../utils/glob';
import { createRegex } from '../utils/glob';

const localize = nls.loadMessageBundle();

Expand Down Expand Up @@ -433,20 +433,54 @@ export class JSONSchemaService implements IJSONSchemaService {
return current;
};

const merge = (target: JSONSchema, sourceRoot: JSONSchema, sourceURI: string, refSegment: string | undefined): void => {
const merge = (target: JSONSchema, section: any): void => {
for (const key in section) {
if (section.hasOwnProperty(key) && !target.hasOwnProperty(key)) {
(<any>target)[key] = section[key];
}
}
};

const mergeByJsonPointer = (target: JSONSchema, sourceRoot: JSONSchema, sourceURI: string, refSegment: string | undefined): void => {
const path = refSegment ? decodeURIComponent(refSegment) : undefined;
const section = findSection(sourceRoot, path);
if (section) {
for (const key in section) {
if (section.hasOwnProperty(key) && !target.hasOwnProperty(key)) {
(<any>target)[key] = section[key];
}
}
merge(target, section);
} else {
resolveErrors.push(localize('json.schema.invalidref', '$ref \'{0}\' in \'{1}\' can not be resolved.', path, sourceURI));
}
};
Comment on lines +443 to 459
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 have refactored merge to mergeByJsonPointer which calls findSection and the actual merge which just copies the properties over. I did it since when I resolve a subschema, I don't need to look for it since I have the object in hand, but merge is still required.


const isSubSchemaRef = (refSegment?: string): boolean => {
// Check if the first character is not '/' to determine whether it's a sub schema reference or a JSON Pointer
return !!refSegment && refSegment.charAt(0) !== '/';
};

const reconstructRefURI = (uri: string, fragment?: string, separator: string = '#'): string => {
return normalizeId(`${uri}${separator}${fragment}`);
};

// To find which $refs point to which $ids we'll keep two maps:
// One for '$id' we expect to encounter (if they exist)
// and one for '$id' we have encountered
const pendingSubSchemas: Record<string, JSONSchema[]> = {};
const resolvedSubSchemas: Record<string, JSONSchema> = {};

const tryMergeSubSchema = (uri: string, target: JSONSchema) : boolean => {
if (resolvedSubSchemas[uri]) { // subschema is resolved, merge it to the target
merge(target, resolvedSubSchemas[uri]);
return true; // return success
} else { // This subschema has not been resolved yet
if (!pendingSubSchemas[uri]) {
pendingSubSchemas[uri] = [];
}

// Remember the target to merge later once resolved
pendingSubSchemas[uri].push(target);
return false; // return failure - merge didn't occur
}
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the heart of the solution. It basically keeps two maps for subschema references. One for those subschemas that we may or may not encounter later and the latter for those subschemas we have resolved.

The tryMergeSubSchema merges already resolved subschemas into the targets that $ref them, or keeps a record of the target object against the full URI of the subschema for later use.

const resolveExternalLink = (node: JSONSchema, uri: string, refSegment: string | undefined, parentSchemaURL: string, parentSchemaDependencies: SchemaDependencies): Thenable<any> => {
if (contextService && !/^[A-Za-z][A-Za-z0-9+\-.+]*:\/\/.*/.test(uri)) {
uri = contextService.resolveRelativePath(uri, parentSchemaURL);
Expand All @@ -459,8 +493,23 @@ export class JSONSchemaService implements IJSONSchemaService {
const loc = refSegment ? uri + '#' + refSegment : uri;
resolveErrors.push(localize('json.schema.problemloadingref', 'Problems loading reference \'{0}\': {1}', loc, unresolvedSchema.errors[0]));
}
merge(node, unresolvedSchema.schema, uri, refSegment);
return resolveRefs(node, unresolvedSchema.schema, uri, referencedHandle.dependencies);

// A placeholder promise that might execute later a ref resolution for the newly resolved schema
let externalLinkPromise: Thenable<any> = Promise.resolve(true);
if(!isSubSchemaRef(refSegment)) {
// This is not a sub schema, merge the regular way
mergeByJsonPointer(node, unresolvedSchema.schema, uri, refSegment);
} else {
// This is a reference to a subschema
const fullId = reconstructRefURI(uri, refSegment);

if (!tryMergeSubSchema(fullId, node)) {
// We weren't able to merge currently so we'll try to resolve this schema first to obtain subschemas
// that could be missed
externalLinkPromise = resolveRefs(unresolvedSchema.schema, unresolvedSchema.schema, uri, referencedHandle.dependencies);
}
}
return externalLinkPromise.then(() => resolveRefs(node, unresolvedSchema.schema, uri, referencedHandle.dependencies));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case we have referenced a subschema in an external schema i.e schema1 -> $ref -> schema2#foo, we'll need to find schema2's #foo so we trigger schema2 resolution eagerly using that externalLinkPromise.

});
};

Expand Down Expand Up @@ -512,11 +561,20 @@ export class JSONSchemaService implements IJSONSchemaService {
const segments = ref.split('#', 2);
delete next.$ref;
if (segments[0].length > 0) {
// This is a reference to an external schema
openPromises.push(resolveExternalLink(next, segments[0], segments[1], parentSchemaURL, parentSchemaDependencies));
return;
} else {
// This is a reference inside the current schema
if (seenRefs.indexOf(ref) === -1) {
merge(next, parentSchema, parentSchemaURL, segments[1]); // can set next.$ref again, use seenRefs to avoid circle
if (isSubSchemaRef(segments[1])) { // A $ref to a sub-schema with an $id (i.e #hello)
// Get the full URI for the current schema to avoid matching schema1#hello and schema2#hello to the same
// reference by accident
const fullId = reconstructRefURI(parentSchemaURL, segments[1]);
tryMergeSubSchema(fullId, next);
} else { // A $ref to a JSON Pointer (i.e #/definitions/foo)
mergeByJsonPointer(next, parentSchema, parentSchemaURL, segments[1]); // can set next.$ref again, use seenRefs to avoid circle
}
seenRefs.push(ref);
}
}
Expand All @@ -527,17 +585,52 @@ export class JSONSchemaService implements IJSONSchemaService {
collectArrayEntries(next.anyOf, next.allOf, next.oneOf, <JSONSchema[]>next.items);
};

const handleId = (next: JSONSchema) => {
// TODO figure out should loops be preventse
const id = next.$id || next.id;
if (typeof id === 'string' && id.charAt(0) === '#') {
delete next.$id;
delete next.id;
// Use a blank separator, as the $id already has the '#'
const fullId = reconstructRefURI(parentSchemaURL, id, '');

if (!resolvedSubSchemas[fullId]) {
// This is the place we fill the blanks in
resolvedSubSchemas[fullId] = next;
} else if (resolvedSubSchemas[fullId] !== next) {
// Duplicate may occur in recursive $refs, but as long as they are the same object
// it's ok, otherwise report and error
resolveErrors.push(localize('json.schema.duplicateid', 'Duplicate id declaration: \'{0}\'', id));
}

// Resolve all pending requests and cleanup the queue list
if (pendingSubSchemas[fullId]) {
while (pendingSubSchemas[fullId].length) {
const target = pendingSubSchemas[fullId].shift();
merge(target!, next);
}

delete pendingSubSchemas[fullId];
}
}
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function resolves the actual subschema, whenever it encounters an $id or id in an object. It keeps a record of the full URI and the object and resolves any pending requests for that subschema.

while (toWalk.length) {
const next = toWalk.pop()!;
if (seen.indexOf(next) >= 0) {
continue;
}
seen.push(next);
handleId(next);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aeschli I was thinking about the two tests you have added and since I already had a map from the full URI to the full schema, it got me thinking. The root cause of the problem was that the handleRef function was called before handleId, thus all the schemas with $ref and $id together were missing that $ref. By simply changing the order between them I have managed to solve these two cases without compromising the rest (luckily).

Copy link
Collaborator

Choose a reason for hiding this comment

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

We still need to remember the full URI to schema mapping also with the resolved schema. Resolving happens on the unresolved schema nodes, so we don't have the information afterwards anymore when the schema is used again.
I added a test case for that: Resolving external $ref to already resolved schema

handleRef(next);
}
return this.promise.all(openPromises);
};

for(const unresolvedSubschemaId in pendingSubSchemas) {
resolveErrors.push(localize('json.schema.idnotfound', 'Subschema with id \'{0}\' was not found', unresolvedSubschemaId));
}

return resolveRefs(schema, schema, schemaURL, dependencies).then(_ => new ResolvedSchema(schema, resolveErrors));
}

Expand Down
Loading