pass pullMerge to getIModelProps which causes extents to be loaded from the database instead of the cached value #7187
Conversation
core/backend/src/IModelDb.ts
Outdated
| @@ -305,6 +305,13 @@ export abstract class IModelDb extends IModel { | |||
| GeoCoordConfig.loadForImodel(this.workspace.settings); // load gcs data specified by iModel's settings dictionaries, must be done before calling initializeIModelDb | |||
|
|
|||
| this.initializeIModelDb(); | |||
| this.onChangesetApplied.addListener(() => { | |||
| const extents = this.queryFilePropertyString({ name: "Extents", namespace: "dgn_Db" }); | |||
| // TODO: What if extents is undefined? And was previously defined? Is this a possibility? | |||
There was a problem hiding this comment.
Any thoughts on this? My guess is that it is unlikely for an imodel to go from having project extents to no project extents so it is not worth handling, but not sure.
There was a problem hiding this comment.
I dont think its possible to from defined extents to undefined, the other way around yes
core/backend/src/IModelDb.ts
Outdated
| @@ -305,6 +305,13 @@ export abstract class IModelDb extends IModel { | |||
| GeoCoordConfig.loadForImodel(this.workspace.settings); // load gcs data specified by iModel's settings dictionaries, must be done before calling initializeIModelDb | |||
|
|
|||
| this.initializeIModelDb(); | |||
There was a problem hiding this comment.
initializeIModelDb() is called after pullMerge() after applying all change set. It internally calls following. If I understand correctly, you want to notify native side of any change to project extent or ecf location. If this is the case, you should not listen to apply change set event, rather just do that in initializeIModelDb(). You may want to add parameter to initializeIModelDb(when: "pullMerge" | undefined)
protected initialize(name: string, props: IModelProps) {
this.name = name;
this.rootSubject = props.rootSubject;
this.projectExtents = Range3d.fromJSON(props.projectExtents);
this.globalOrigin = Point3d.fromJSON(props.globalOrigin);
const ecefLocation = props.ecefLocation ? new EcefLocation(props.ecefLocation) : undefined;
this.ecefLocation = ecefLocation?.isValid ? ecefLocation : undefined;
this.geographicCoordinateSystem = props.geographicCoordinateSystem ? new GeographicCRS(props.geographicCoordinateSystem) : undefined;
}There was a problem hiding this comment.
Switched to passing pullMerge into intializeIModelDb
nick4598
changed the title
| @@ -3116,7 +3116,7 @@ export abstract class IModelDb extends IModel { | |||
| // @alpha | |||
| importSchemaStrings(serializedXmlSchemas: string[]): Promise<void>; | |||
| // @internal (undocumented) | |||
| protected initializeIModelDb(): void; | |||
| protected initializeIModelDb(when?: "pullMerge"): void; | |||
There was a problem hiding this comment.
are we really just going to use a string literal?
what other possible option is there for this in the future?
There was a problem hiding this comment.
I also personally wanted to do a boolean "afterPullMerge" or something along those lines that defaults to false because it seems unlikely we'd need to special case anything else. But, affan mentioned that he was working on a Pr to support table conflict handlers which are called before applying changests, after applying changesets, and eventually after calling pullMerge. That was where his suggestion came from. Im not tied to either approach.
|
@nick4598 Any updates on this? |
This PR is ready for another round of review (especially the native one) |
|
@khanaffan can you review? |
imodel-native: iTwin/imodel-native#866
Fixes https://github.com/iTwin/itwinjs-backlog/issues/1240