Skip to content

Commit

Permalink
Cleanup some misc fixmes (#156)
Browse files Browse the repository at this point in the history 
I removed the fixme about structured testing and added it to work item:
#155.

Removed transformerresumption API and fixmes associated with it.
  • Loading branch information
@nick4598
nick4598 authored Feb 28, 2024
1 parent 2d26ba5 commit 1f594de
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 1,762 deletions.
47 changes: 0 additions & 47 deletions packages/transformer/src/IModelCloneContext.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,51 +280,4 @@ export class IModelCloneContext extends IModelElementCloneContext {
});
return targetElementAspectProps;
}

private static aspectRemapTableName = "AspectIdRemaps";

public override saveStateToDb(db: SQLiteDb): void {
super.saveStateToDb(db);
if (
DbResult.BE_SQLITE_DONE !==
db.executeSQL(
`CREATE TABLE ${IModelCloneContext.aspectRemapTableName} (Source INTEGER, Target INTEGER)`
)
)
throw Error(
"Failed to create the aspect remap table in the state database"
);
db.saveChanges();
db.withPreparedSqliteStatement(
`INSERT INTO ${IModelCloneContext.aspectRemapTableName} (Source, Target) VALUES (?, ?)`,
(stmt) => {
for (const [source, target] of this._aspectRemapTable) {
stmt.reset();
stmt.bindId(1, source);
stmt.bindId(2, target);
if (DbResult.BE_SQLITE_DONE !== stmt.step())
throw Error(
"Failed to insert aspect remapping into the state database"
);
}
}
);
}

public override loadStateFromDb(db: SQLiteDb): void {
super.loadStateFromDb(db);
// FIXME: test this
db.withSqliteStatement(
`SELECT Source, Target FROM ${IModelCloneContext.aspectRemapTableName}`,
(stmt) => {
let status = DbResult.BE_SQLITE_ERROR;
while ((status = stmt.step()) === DbResult.BE_SQLITE_ROW) {
const source = stmt.getValue(0).getId();
const target = stmt.getValue(1).getId();
this._aspectRemapTable.set(source, target);
}
assert(status === DbResult.BE_SQLITE_DONE);
}
);
}
}
122 changes: 5 additions & 117 deletions packages/transformer/src/IModelExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ export type ExporterInitOptions = ExportChangesOptions;
*/
export type ExportChangesOptions = Omit<InitOptions, "startChangeset"> & {
skipPropagateChangesToRootElements?: boolean;
} & /**
* an array of ChangesetFileProps which are used to read the changesets and populate the ChangedInstanceIds using [[ChangedInstanceIds.initialize]] in [[IModelExporter.exportChanges]]
* @note mutually exclusive with @see changesetRanges, @see startChangeset and @see changedInstanceIds, so define one of the four, never more
*/
(| { csFileProps: ChangesetFileProps[] }
} /**
* an array of ChangesetFileProps which are used to read the changesets and populate the ChangedInstanceIds using [[ChangedInstanceIds.initialize]] in [[IModelExporter.exportChanges]]
* @note mutually exclusive with @see changesetRanges, @see startChangeset and @see changedInstanceIds, so define one of the four, never more
*/ & (
| { csFileProps: ChangesetFileProps[] }
/**
* Class instance that contains modified elements between 2 versions of an iModel.
* If this parameter is not provided, then [[ChangedInstanceIds.initialize]] in [[IModelExporter.exportChanges]]
Expand Down Expand Up @@ -998,118 +998,6 @@ export class IModelExporter {
return this.handler.onProgress();
}
}

/**
* You may override this to store arbitrary json state in a exporter state dump, useful for some resumptions
* @see [[IModelTransformer.saveStateToFile]]
*/
protected getAdditionalStateJson(): any {
return {};
}

/**
* You may override this to load arbitrary json state in a transformer state dump, useful for some resumptions
* @see [[IModelTransformer.loadStateFromFile]]
*/
protected loadAdditionalStateJson(_additionalState: any): void {}

/**
* Reload our state from a JSON object
* Intended for [[IModelTransformer.resumeTransformation]]
* @internal
* You can load custom json from the exporter save state for custom exporters by overriding [[IModelExporter.loadAdditionalStateJson]]
*/
public loadStateFromJson(state: IModelExporterState): void {
if (state.exporterClass !== this.constructor.name)
throw Error(
"resuming from a differently named exporter class, it is not necessarily valid to resume with a different exporter class"
);
this.wantGeometry = state.wantGeometry;
this.wantTemplateModels = state.wantTemplateModels;
this.wantSystemSchemas = state.wantSystemSchemas;
this.visitElements = state.visitElements;
this.visitRelationships = state.visitRelationships;
this._excludedCodeSpecNames = new Set(state.excludedCodeSpecNames);
(this._excludedElementIds = CompressedId64Set.decompressSet(
state.excludedElementIds
)),
(this._excludedElementCategoryIds = CompressedId64Set.decompressSet(
state.excludedElementCategoryIds
)),
(this._excludedElementClasses = new Set(
state.excludedElementClassNames.map((c) => this.sourceDb.getJsClass(c))
));
this._exportElementAspectsStrategy.loadExcludedElementAspectClasses(
state.excludedElementAspectClassFullNames
);
this._excludedRelationshipClasses = new Set(
state.excludedRelationshipClassNames.map((c) =>
this.sourceDb.getJsClass(c)
)
);
this.loadAdditionalStateJson(state.additionalState);
}

/**
* Serialize state to a JSON object
* Intended for [[IModelTransformer.resumeTransformation]]
* @internal
* You can add custom json to the exporter save state for custom exporters by overriding [[IModelExporter.getAdditionalStateJson]]
*/
public saveStateToJson(): IModelExporterState {
return {
exporterClass: this.constructor.name,
wantGeometry: this.wantGeometry,
wantTemplateModels: this.wantTemplateModels,
wantSystemSchemas: this.wantSystemSchemas,
visitElements: this.visitElements,
visitRelationships: this.visitRelationships,
excludedCodeSpecNames: [...this._excludedCodeSpecNames],
excludedElementIds: CompressedId64Set.compressSet(
this._excludedElementIds
),
excludedElementCategoryIds: CompressedId64Set.compressSet(
this._excludedElementCategoryIds
),
excludedElementClassNames: Array.from(
this._excludedElementClasses,
(cls) => cls.classFullName
),
excludedElementAspectClassFullNames: [
...this._exportElementAspectsStrategy
.excludedElementAspectClassFullNames,
],
excludedRelationshipClassNames: Array.from(
this._excludedRelationshipClasses,
(cls) => cls.classFullName
),
additionalState: this.getAdditionalStateJson(),
};
}
}

/**
* The JSON format of a serialized IModelExporter instance
* Used for starting an exporter in the middle of an export operation,
* such as resuming a crashed transformation
*
* @note Must be kept synchronized with IModelExporter
* @internal
*/
export interface IModelExporterState {
exporterClass: string;
wantGeometry: boolean;
wantTemplateModels: boolean;
wantSystemSchemas: boolean;
visitElements: boolean;
visitRelationships: boolean;
excludedCodeSpecNames: string[];
excludedElementIds: CompressedId64Set;
excludedElementCategoryIds: CompressedId64Set;
excludedElementClassNames: string[];
excludedElementAspectClassFullNames: string[];
excludedRelationshipClassNames: string[];
additionalState?: any;
}

/**
Expand Down
95 changes: 11 additions & 84 deletions packages/transformer/src/IModelImporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ import {
SourceAndTarget,
SubCategory,
} from "@itwin/core-backend";
import type { IModelTransformOptions } from "./IModelTransformer";
import type {
IModelTransformOptions,
RelationshipPropsForDelete,
} from "./IModelTransformer";
import * as assert from "assert";
import { deleteElementTreeCascade } from "./ElementCascadingDeleter";

Expand Down Expand Up @@ -636,23 +639,25 @@ export class IModelImporter {
}

/** Delete the specified Relationship from the target iModel. */
protected onDeleteRelationship(relationshipProps: RelationshipProps): void {
protected onDeleteRelationship(
relationshipProps: RelationshipPropsForDelete
): void {
// Only passing in what deleteInstance actually uses, full relationshipProps is not necessary.
this.targetDb.relationships.deleteInstance({
id: relationshipProps.id,
classFullName: relationshipProps.classFullName,
} as RelationshipProps);
Logger.logInfo(
loggerCategory,
`Deleted relationship ${this.formatRelationshipForLogger(
relationshipProps
)}`
`Deleted relationship ${relationshipProps.classFullName} id=${relationshipProps.id}`
);
this.trackProgress();
}

/** Delete the specified Relationship from the target iModel. */
public deleteRelationship(relationshipProps: RelationshipProps): void {
public deleteRelationship(
relationshipProps: RelationshipPropsForDelete
): void {
this.onDeleteRelationship(relationshipProps);
}

Expand Down Expand Up @@ -763,67 +768,6 @@ export class IModelImporter {
}
}

/**
* You may override this to store arbitrary json state in a exporter state dump, useful for some resumptions
* @see [[IModelTransformer.saveStateToFile]]
*/
protected getAdditionalStateJson(): any {
return {};
}

/**
* You may override this to load arbitrary json state in a transformer state dump, useful for some resumptions
* @see [[IModelTransformer.loadStateFromFile]]
*/
protected loadAdditionalStateJson(_additionalState: any): void {}

/**
* Reload our state from a JSON object
* Intended for [[IModelTransformer.resumeTransformation]]
* @internal
* You can load custom json from the importer save state for custom importers by overriding [[IModelImporter.loadAdditionalStateJson]]
*/
public loadStateFromJson(state: IModelImporterState): void {
if (state.importerClass !== this.constructor.name)
throw Error(
"resuming from a differently named importer class, it is not necessarily valid to resume with a different importer class"
);
// ignore readonly since this runs right after construction in [[IModelTransformer.resumeTransformation]]
(this.options as IModelTransformOptions) = state.options;
if (this.targetDb.iModelId !== state.targetDbId)
throw Error(
"can only load importer state when the same target is reused"
);
// TODO: fix upstream, looks like a bad case for the linter rule when casting away readonly for this generic
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
(this.doNotUpdateElementIds as Set<Id64String>) =
CompressedId64Set.decompressSet(state.doNotUpdateElementIds);
this._duplicateCodeValueMap = new Map(
Object.entries(state.duplicateCodeValueMap)
);
this.loadAdditionalStateJson(state.additionalState);
}

/**
* Serialize state to a JSON object
* Intended for [[IModelTransformer.resumeTransformation]]
* @internal
* You can add custom json to the importer save state for custom importers by overriding [[IModelImporter.getAdditionalStateJson]]
*/
public saveStateToJson(): IModelImporterState {
return {
importerClass: this.constructor.name,
options: this.options,
targetDbId:
this.targetDb.iModelId || this.targetDb.nativeDb.getFilePath(),
doNotUpdateElementIds: CompressedId64Set.compressSet(
this.doNotUpdateElementIds
),
duplicateCodeValueMap: Object.fromEntries(this._duplicateCodeValueMap),
additionalState: this.getAdditionalStateJson(),
};
}

private resolveDuplicateCodeValues(): void {
for (const [elementId, codeValue] of this._duplicateCodeValueMap) {
const element = this.targetDb.elements.getElement(elementId);
Expand All @@ -844,23 +788,6 @@ export class IModelImporter {
}
}

/**
* The JSON format of a serialized IModelimporter instance
* Used for starting an importer in the middle of an imxport operation,
* such as resuming a crashed transformation
*
* @note Must be kept synchronized with IModelImxporter
* @internal
*/
export interface IModelImporterState {
importerClass: string;
options: IModelImportOptions;
targetDbId: string;
doNotUpdateElementIds: CompressedId64Set;
duplicateCodeValueMap: Record<Id64String, string>;
additionalState?: any;
}

/** Returns true if a change within an Entity is detected.
* @param entity The current persistent Entity.
* @param entityProps The new EntityProps to compare against
Expand Down
Loading

0 comments on commit 1f594de

Please sign in to comment.