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.

Sign up for GitHub

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

Derived class caching #7429

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Derived class caching #7429

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3b6bd67
Derived class caching
christophermlawson Nov 26, 2024
1ca352c
change log and api extract
christophermlawson Nov 26, 2024
5c2b545
Merge branch 'master' into derived-class-caching
christophermlawson Nov 26, 2024
b453744
Merge branch 'master' into derived-class-caching
christophermlawson Dec 3, 2024
167bf2e
Merge branch 'master' into derived-class-caching
christophermlawson Dec 6, 2024
acbd85e
Merge branch 'master' into derived-class-caching
christophermlawson Dec 6, 2024
7b320d3
Merge branch 'derived-class-caching' of https://github.com/iTwin/itwi…
christophermlawson Dec 6, 2024
cca0819
PR updates
christophermlawson Dec 6, 2024
90ea87e
Merge branch 'master' into derived-class-caching
christophermlawson Dec 20, 2024
c9dab88
rush extract
christophermlawson Dec 20, 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
8 changes: 4 additions & 4 deletions common/api/ecschema-editing.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1928,25 +1928,25 @@ export const SchemaCompareDiagnostics: {
diagnosticType: DiagnosticType;
};
FormatUnitMissing: {
new (ecDefinition: SchemaItem, messageArgs: [Unit | InvertedUnit], category?: DiagnosticCategory): {
new (ecDefinition: SchemaItem, messageArgs: [InvertedUnit | Unit], category?: DiagnosticCategory): {
readonly code: string;
readonly messageText: string;
readonly schema: Schema;
readonly diagnosticType: DiagnosticType;
ecDefinition: Format;
messageArgs?: [Unit | InvertedUnit] | undefined;
messageArgs?: [InvertedUnit | Unit] | undefined;
category: DiagnosticCategory;
};
diagnosticType: DiagnosticType;
};
UnitLabelOverrideDelta: {
new (ecDefinition: SchemaItem, messageArgs: [Unit | InvertedUnit, string | undefined, string | undefined], category?: DiagnosticCategory): {
new (ecDefinition: SchemaItem, messageArgs: [InvertedUnit | Unit, string | undefined, string | undefined], category?: DiagnosticCategory): {
readonly code: string;
readonly messageText: string;
readonly schema: Schema;
readonly diagnosticType: DiagnosticType;
ecDefinition: Format;
messageArgs?: [Unit | InvertedUnit, string | undefined, string | undefined] | undefined;
messageArgs?: [InvertedUnit | Unit, string | undefined, string | undefined] | undefined;
category: DiagnosticCategory;
};
diagnosticType: DiagnosticType;
Expand Down
6 changes: 4 additions & 2 deletions common/api/ecschema-metadata.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,7 @@ export abstract class ECClass extends SchemaItem implements CustomAttributeConta
// (undocumented)
protected addCustomAttribute(customAttribute: CustomAttribute): void;
protected addProperty<T extends Property>(prop: T): T;
// (undocumented)
get baseClass(): LazyLoadedECClass | undefined;
set baseClass(baseClass: LazyLoadedECClass | undefined);
// (undocumented)
protected _baseClass?: LazyLoadedECClass;
// (undocumented)
Expand Down Expand Up @@ -293,6 +291,8 @@ export abstract class ECClass extends SchemaItem implements CustomAttributeConta
// @alpha
protected deletePropertySync(name: string): void;
// (undocumented)
protected _derivedClasses?: Map<string, LazyLoadedECClass>;
// (undocumented)
fromJSON(classProps: ClassProps): Promise<void>;
// (undocumented)
fromJSONSync(classProps: ClassProps): void;
Expand All @@ -303,6 +303,7 @@ export abstract class ECClass extends SchemaItem implements CustomAttributeConta
getBaseClassSync(): ECClass | undefined;
getCustomAttributes(): Promise<CustomAttributeSet>;
getCustomAttributesSync(): CustomAttributeSet;
getDerivedClasses(): Promise<ECClass[] | undefined>;
getInheritedProperty(name: string): Promise<Property | undefined>;
getInheritedPropertySync(name: string): Property | undefined;
getProperties(resetCache?: boolean): Promise<Property[]>;
Expand Down Expand Up @@ -333,6 +334,7 @@ export abstract class ECClass extends SchemaItem implements CustomAttributeConta
get properties(): IterableIterator<Property> | undefined;
// (undocumented)
protected _properties?: Map<string, Property>;
setBaseClass(baseClass: LazyLoadedECClass | undefined): Promise<void>;
// @alpha
protected setModifier(modifier: ECClassModifier): void;
toJSON(standalone?: boolean, includeSchemaVersion?: boolean): ClassProps;
Expand Down
10 changes: 10 additions & 0 deletions common/changes/@itwin/ecschema-editing/derived-class-caching_2024-11-26-13-11.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/ecschema-editing",
"comment": "Replaced ECClass.baseClass setter with setBaseClass method due to underlying changes in the ecschema-metadata package.",
"type": "none"
}
],
"packageName": "@itwin/ecschema-editing"
}
10 changes: 10 additions & 0 deletions common/changes/@itwin/ecschema-metadata/derived-class-caching_2024-11-26-13-11.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/ecschema-metadata",
"comment": "ECClass.baseClass setter has been replaced with setBaseClass method to enable derived class caching support.",
"type": "none"
}
],
"packageName": "@itwin/ecschema-metadata"
}
6 changes: 3 additions & 3 deletions core/ecschema-editing/src/Editing/ECClasses.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class ECClasses extends SchemaItems{
if (baseClassKey !== undefined) {
const baseClassSchema = !baseClassKey.schemaKey.matches(newClass.schema.schemaKey) ? await this.getSchema(baseClassKey.schemaKey) : newClass.schema as MutableSchema;
const baseClassItem = await this.lookupSchemaItem<ECClass>(baseClassSchema, baseClassKey);
newClass.baseClass = new DelayedPromiseWithProps<SchemaItemKey, T>(baseClassKey, async () => baseClassItem as T);
await newClass.setBaseClass(new DelayedPromiseWithProps<SchemaItemKey, T>(baseClassKey, async () => baseClassItem as T));
}

return newClass;
Expand Down Expand Up @@ -275,7 +275,7 @@ export class ECClasses extends SchemaItems{
try {
const classItem = await this.getSchemaItem<ECClass>(itemKey);
if (!baseClassKey) {
classItem.baseClass = undefined;
await classItem.setBaseClass(undefined);
return;
}

Expand All @@ -284,7 +284,7 @@ export class ECClasses extends SchemaItems{
if (classItem.baseClass !== undefined && !await baseClassItem.is(await classItem.baseClass))
throw new SchemaEditingError(ECEditingStatus.InvalidBaseClass, new ClassId(this.schemaItemType, baseClassKey), undefined, undefined, `Base class ${baseClassKey.fullName} must derive from ${(await classItem.baseClass).fullName}.`);

classItem.baseClass = new DelayedPromiseWithProps<SchemaItemKey, ECClass>(baseClassKey, async () => baseClassItem);
await classItem.setBaseClass(new DelayedPromiseWithProps<SchemaItemKey, ECClass>(baseClassKey, async () => baseClassItem));
} catch(e: any) {
throw new SchemaEditingError(ECEditingStatus.SetBaseClass, new ClassId(this.schemaItemType, itemKey), e);
}
Expand Down
2 changes: 1 addition & 1 deletion core/ecschema-editing/src/Editing/RelationshipClasses.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export class RelationshipClasses extends ECClasses {
try {
await this.validate(relClass!);
} catch(e: any) {
relClass!.baseClass = baseClass;
await relClass!.setBaseClass(baseClass);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add some check here (or actually in line 111) to ensure that relClass is not undefined? Since it's a lookup by key, I assume it could be happen (eg when loading from a referenced schema) or did I missed some previous validation here?

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 added a catch block on line 112.

throw new SchemaEditingError(ECEditingStatus.SetBaseClass, new ClassId(SchemaItemType.RelationshipClass, itemKey), e);
}
}
Expand Down
30 changes: 29 additions & 1 deletion core/ecschema-editing/src/test/Editing/Entities.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,13 @@ describe("Entities tests", () => {
const result = await testEditor.entities.create(testKey, "testEntity", ECClassModifier.None, "testLabel", testEntityBaseRes);

const testEntity = await testEditor.schemaContext.getSchemaItem<EntityClass>(result);
expect(await testEntity?.baseClass).to.eql(await testEditor.schemaContext.getSchemaItem(testEntityBaseRes));
const baseEntity = await testEditor.schemaContext.getSchemaItem<EntityClass>(testEntityBaseRes);
expect(await testEntity?.baseClass).to.eql(baseEntity);
expect(testEntity?.label).to.eql("testLabel");
const derivedClasses = await baseEntity?.getDerivedClasses();
expect(derivedClasses).to.not.be.undefined;
expect(derivedClasses![0]).to.eql(testEntity);
expect(derivedClasses?.length).to.eql(1);
});

it("should create a new entity class with a base class from different schema", async () => {
Expand All @@ -85,8 +90,13 @@ describe("Entities tests", () => {
const result = await testEditor.entities.create(testKey, "testEntity", ECClassModifier.None, "testLabel", baseClassKey);

const testEntity = await testEditor.schemaContext.getSchemaItem<EntityClass>(result);
const baseEntity = await testEditor.schemaContext.getSchemaItem<EntityClass>(baseClassKey);
expect(await testEntity?.baseClass).to.eql(await testEditor.schemaContext.getSchemaItem(baseClassKey));
expect(testEntity?.label).to.eql("testLabel");
const derivedClasses = await baseEntity?.getDerivedClasses();
expect(derivedClasses).to.not.be.undefined;
expect(derivedClasses![0]).to.eql(testEntity);
expect(derivedClasses?.length).to.eql(1);
});

it("try creating a new entity class with base class from unknown schema, returns error", async () => {
Expand Down Expand Up @@ -157,7 +167,12 @@ describe("Entities tests", () => {
await testEditor.entities.setBaseClass(result, testEntityBaseRes);

const testEntity = await testEditor.schemaContext.getSchemaItem<EntityClass>(result);
const baseEntity = await testEditor.schemaContext.getSchemaItem<EntityClass>(testEntityBaseRes);
expect(await testEntity?.baseClass).to.eql(await testEditor.schemaContext.getSchemaItem(testEntityBaseRes));
const derivedClasses = await baseEntity?.getDerivedClasses();
expect(derivedClasses).to.not.be.undefined;
expect(derivedClasses![0]).to.eql(testEntity);
expect(derivedClasses?.length).to.eql(1);
});

it("should add base class to existing entity class where base class is from a different schema", async () => {
Expand All @@ -182,7 +197,12 @@ describe("Entities tests", () => {
await testEditor.entities.setBaseClass(result, baseClassKey);

const testEntity = await testEditor.schemaContext.getSchemaItem<EntityClass>(result);
const baseEntity = await testEditor.schemaContext.getSchemaItem<EntityClass>(baseClassKey);
expect(await testEntity?.baseClass).to.eql(await testEditor.schemaContext.getSchemaItem(baseClassKey));
const derivedClasses = await baseEntity?.getDerivedClasses();
expect(derivedClasses).to.not.be.undefined;
expect(derivedClasses![0]).to.eql(testEntity);
expect(derivedClasses?.length).to.eql(1);
});

it("should change base class of existing entity class with different base class.", async () => {
Expand Down Expand Up @@ -215,8 +235,13 @@ describe("Entities tests", () => {
expect(await testEntity?.baseClass).to.eql(await testEditor.schemaContext.getSchemaItem(firstBaseClassKey));

const secondBaseClassKey = new SchemaItemKey("testEntityBase2", refSchema.schemaKey);
const secondBaseClass = await testEditor.schemaContext.getSchemaItem<EntityClass>(secondBaseClassKey);
await testEditor.entities.setBaseClass(testEntityResult, secondBaseClassKey);
expect(await testEntity?.baseClass).to.eql(await testEditor.schemaContext.getSchemaItem(secondBaseClassKey));
const derivedClasses = await secondBaseClass?.getDerivedClasses();
expect(derivedClasses).to.not.be.undefined;
expect(derivedClasses![0]).to.eql(testEntity);
expect(derivedClasses?.length).to.eql(1);
});

it("should remove base class from existing entity class.", async () => {
Expand All @@ -228,6 +253,9 @@ describe("Entities tests", () => {

await testEditor.entities.setBaseClass(result, undefined);
expect(await testEntity?.baseClass).to.eql(undefined);
const baseEntity = await testEditor.schemaContext.getSchemaItem<EntityClass>(testEntityBaseRes);
const derivedClasses = await baseEntity?.getDerivedClasses();
expect(derivedClasses).to.be.undefined;
});

it("try adding base class with unknown schema to existing entity class, returns error", async () => {
Expand Down
8 changes: 4 additions & 4 deletions core/ecschema-editing/src/test/Validation/ECRules/ClassRules.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe("ClassRule tests", () => {
it("BaseClassIsSealed, rule violated.", async () => {
const baseClass = new EntityClass(schema, "TestBase", ECClassModifier.Sealed);
const entityClass = new EntityClass(schema, "TestClass");
entityClass.baseClass = new DelayedPromiseWithProps(baseClass.key, async () => baseClass);
await entityClass.setBaseClass(new DelayedPromiseWithProps(baseClass.key, async () => baseClass));

const result = Rules.baseClassIsSealed(entityClass);

Expand All @@ -42,7 +42,7 @@ describe("ClassRule tests", () => {
it("BaseClassIsSealed, base is not sealed, rule passes.", async () => {
const baseClass = new EntityClass(schema, "TestBase");
const entityClass = new EntityClass(schema, "TestClass");
entityClass.baseClass = new DelayedPromiseWithProps(baseClass.key, async () => baseClass);
await entityClass.setBaseClass(new DelayedPromiseWithProps(baseClass.key, async () => baseClass));

const result = Rules.baseClassIsSealed(entityClass);
for await (const _diagnostic of result) {
Expand All @@ -62,7 +62,7 @@ describe("ClassRule tests", () => {
it("BaseClassIsOfDifferentType, rule violated.", async () => {
const baseClass = new RelationshipClass(schema, "TestBase");
const entityClass = new EntityClass(schema, "TestClass");
entityClass.baseClass = new DelayedPromiseWithProps(baseClass.key, async () => baseClass);
await entityClass.setBaseClass(new DelayedPromiseWithProps(baseClass.key, async () => baseClass));
const baseType = schemaItemTypeToString(baseClass.schemaItemType);

const result = Rules.baseClassIsOfDifferentType(entityClass);
Expand All @@ -82,7 +82,7 @@ describe("ClassRule tests", () => {
it("BaseClassIsOfDifferentType, same type, rule passes.", async () => {
const baseClass = new EntityClass(schema, "TestBase");
const entityClass = new EntityClass(schema, "TestClass");
entityClass.baseClass = new DelayedPromiseWithProps(baseClass.key, async () => baseClass);
await entityClass.setBaseClass(new DelayedPromiseWithProps(baseClass.key, async () => baseClass));

const result = Rules.baseClassIsOfDifferentType(entityClass);
for await (const _diagnostic of result) {
Expand Down
4 changes: 2 additions & 2 deletions core/ecschema-editing/src/test/Validation/ECRules/MixinRules.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe("Mixin Rule Tests", () => {
const mixin = new TestMixin(schema, "TestMixin", constraintClass);
const entityClass = new EntityClass(schema, "TestClass");
(entityClass as MutableEntityClass).addMixin(mixin);
entityClass.baseClass = new DelayedPromiseWithProps(constraintClass.key, async () => constraintClass);
await entityClass.setBaseClass(new DelayedPromiseWithProps(constraintClass.key, async () => constraintClass));

const result = Rules.mixinAppliedToClassMustDeriveFromConstraint(entityClass);
for await (const _diagnostic of result) {
Expand Down Expand Up @@ -73,7 +73,7 @@ describe("Mixin Rule Tests", () => {
const mixin = new TestMixin(schema, "TestMixin", constraintClass);
const entityClass = new EntityClass(schema, "TestClass");
(entityClass as MutableEntityClass).addMixin(mixin);
entityClass.baseClass = new DelayedPromiseWithProps(baseClass.key, async () => baseClass);
await entityClass.setBaseClass(new DelayedPromiseWithProps(baseClass.key, async () => baseClass));

const result = Rules.mixinAppliedToClassMustDeriveFromConstraint(entityClass);

Expand Down
8 changes: 4 additions & 4 deletions core/ecschema-editing/src/test/Validation/ECRules/PropertyRules.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe("PropertyRule tests", () => {
testBaseClass = await mutable.createEntityClass("TestBaseClass");
testKindOfQuantity = await mutable.createKindOfQuantity("TestKoQ");
testBaseKindOfQuantity = await mutable.createKindOfQuantity("TestBaseKoQ");
testClass.baseClass = new DelayedPromiseWithProps(testBaseClass.key, async () => testBaseClass);
await testClass.setBaseClass(new DelayedPromiseWithProps(testBaseClass.key, async () => testBaseClass));
});

describe("IncompatibleValueTypePropertyOverride Tests", () => {
Expand Down Expand Up @@ -59,7 +59,7 @@ describe("PropertyRule tests", () => {
const rootBaseClass = new EntityClass(schema, "RootBaseClass");
await (rootBaseClass as ECClass as MutableClass).createPrimitiveProperty("TestProperty", PrimitiveType.String);
await (testClass as ECClass as MutableClass).createPrimitiveProperty("TestProperty", PrimitiveType.Integer);
testBaseClass.baseClass = new DelayedPromiseWithProps(rootBaseClass.key, async () => rootBaseClass);
await testBaseClass.setBaseClass(new DelayedPromiseWithProps(rootBaseClass.key, async () => rootBaseClass));

const properties = [...testClass.properties!];
const results = Rules.incompatibleValueTypePropertyOverride(properties[0] as PrimitiveProperty);
Expand Down Expand Up @@ -155,7 +155,7 @@ describe("PropertyRule tests", () => {
const rootBaseClass = new EntityClass(schema, "RootBaseClass");
await (rootBaseClass as ECClass as MutableClass).createPrimitiveArrayProperty("TestProperty", PrimitiveType.String);
await (testClass as ECClass as MutableClass).createPrimitiveProperty("TestProperty", PrimitiveType.String);
testBaseClass.baseClass = new DelayedPromiseWithProps(rootBaseClass.key, async () => rootBaseClass);
await testBaseClass.setBaseClass(new DelayedPromiseWithProps(rootBaseClass.key, async () => rootBaseClass));

const properties = [...testClass.properties!];
const results = Rules.incompatibleTypePropertyOverride(properties[0] as PrimitiveProperty);
Expand Down Expand Up @@ -257,7 +257,7 @@ describe("PropertyRule tests", () => {
const rootBaseClass = new EntityClass(schema, "RootBaseClass");
await (rootBaseClass as ECClass as MutableClass).createPrimitiveProperty("TestProperty", PrimitiveType.String);
await (testClass as ECClass as MutableClass).createPrimitiveProperty("TestProperty", PrimitiveType.Integer);
testBaseClass.baseClass = new DelayedPromiseWithProps(rootBaseClass.key, async () => rootBaseClass);
await testBaseClass.setBaseClass(new DelayedPromiseWithProps(rootBaseClass.key, async () => rootBaseClass));

const basePropJson = {
name: "TestProperty",
Expand Down
4 changes: 2 additions & 2 deletions core/ecschema-editing/src/test/Validation/SchemaValidater.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe("SchemaValidater tests", () => {
it("validateSchema, rule violation reported correctly", async () => {
const baseClass = new EntityClass(schema, "TestBase", ECClassModifier.Sealed);
const entityClass = new EntityClass(schema, "TestClass");
entityClass.baseClass = new DelayedPromiseWithProps(baseClass.key, async () => baseClass);
await entityClass.setBaseClass(new DelayedPromiseWithProps(baseClass.key, async () => baseClass));
(schema as MutableSchema).addItem(entityClass);

const result = await SchemaValidater.validateSchema(schema);
Expand All @@ -32,7 +32,7 @@ describe("SchemaValidater tests", () => {
const ruleSet = new TestRuleSet();
const baseClass = new EntityClass(schema, "TestBase", ECClassModifier.Sealed);
const entityClass = new EntityClass(schema, "TestClass");
entityClass.baseClass = new DelayedPromiseWithProps(baseClass.key, async () => baseClass);
await entityClass.setBaseClass(new DelayedPromiseWithProps(baseClass.key, async () => baseClass));
(schema as MutableSchema).addItem(entityClass);

const result = await SchemaValidater.validateSchema(schema, ruleSet);
Expand Down
Loading
Loading