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.

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

pass pullMerge to getIModelProps which causes extents to be loaded from the database instead of the cached value #7187

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion common/api/core-backend.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

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

are we really just going to use a string literal?
what other possible option is there for this in the future?

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 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.

Copy link
Member

Choose a reason for hiding this comment

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

get isBriefcase(): boolean;
isBriefcaseDb(): this is BriefcaseDb;
// @internal
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-backend",
"comment": "pulling a changeset with project extents changes now updates the extents of the opened imodel",
"type": "none"
}
],
"packageName": "@itwin/core-backend"
}
8 changes: 4 additions & 4 deletions core/backend/src/IModelDb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,8 @@ export abstract class IModelDb extends IModel {
}

/** @internal */
protected initializeIModelDb() {
const props = this[_nativeDb].getIModelProps();
protected initializeIModelDb(when?: "pullMerge") {
const props = this[_nativeDb].getIModelProps(when);
super.initialize(props.rootSubject.name, props);
if (this._initialized)
return;
Expand Down Expand Up @@ -3008,7 +3008,7 @@ export class BriefcaseDb extends IModelDb {
await BriefcaseManager.pullAndApplyChangesets(this, arg ?? {});
if (!this.skipSyncSchemasOnPullAndPush)
await SchemaSync.pull(this);
this.initializeIModelDb();
this.initializeIModelDb("pullMerge");
});

IpcHost.notifyTxns(this, "notifyPulledChanges", this.changeset as ChangesetIndexAndId);
Expand Down Expand Up @@ -3094,7 +3094,7 @@ export class BriefcaseDb extends IModelDb {
// pushing changes requires a writeable briefcase
await this.executeWritable(async () => {
await BriefcaseManager.pullMergePush(this, arg);
this.initializeIModelDb();
this.initializeIModelDb("pullMerge");
});

const changeset = this.changeset as ChangesetIndexAndId;
Expand Down
36 changes: 35 additions & 1 deletion core/backend/src/test/standalone/IModelWrite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { AccessToken, DbResult, GuidString, Id64, Id64String } from "@itwin/core-bentley";
import {
ChangesetIdWithIndex, Code, ColorDef,
GeometricElement2dProps, GeometryStreamProps, IModel, LockState, QueryRowFormat, RequestNewBriefcaseProps, SchemaState, SubCategoryAppearance,
GeometricElement2dProps, GeometryStreamProps, IModel, IModelVersion, LockState, QueryRowFormat, RequestNewBriefcaseProps, SchemaState, SubCategoryAppearance,
} from "@itwin/core-common";
import { Arc3d, IModelJson, Point2d, Point3d } from "@itwin/core-geometry";
import * as chai from "chai";
Expand Down Expand Up @@ -839,6 +839,40 @@ describe("IModelWriteTest", () => {
rwIModel2.close();
});

it("pulling a changeset with extents changes should update the extents of the opened imodel", async () => {
const accessToken = await HubWrappers.getAccessToken(TestUserType.Regular);
const version0 = IModelTestUtils.resolveAssetFile("mirukuru.ibim");
const iModelId = await HubMock.createNewIModel({ iTwinId, iModelName: "projectExtentsTest", version0 });
const iModel = await HubWrappers.downloadAndOpenBriefcase({ iTwinId, iModelId });
const changesetIdBeforeExtentsChange = iModel.changeset.id;
const extents = iModel.projectExtents;
const newExtents = extents.clone();
newExtents.low.x += 100;
newExtents.low.y += 100;
newExtents.high.x += 100;
newExtents.high.y += 100;
iModel.updateProjectExtents(newExtents);
iModel.saveChanges("update project extents");
await iModel.pushChanges({ description: "update project extents" });
await HubWrappers.closeAndDeleteBriefcaseDb(accessToken, iModel);
const iModelBeforeExtentsChange = await HubWrappers.downloadAndOpenBriefcase({ accessToken, iTwinId, iModelId, asOf: IModelVersion.asOfChangeSet(changesetIdBeforeExtentsChange).toJSON() });
const extentsBeforePull = iModelBeforeExtentsChange.projectExtents;
// Read the extents fileProperty.
const extentsStrBeforePull = iModelBeforeExtentsChange.queryFilePropertyString({name: "Extents", namespace: "dgn_Db"});
const ecefLocationBeforeExtentsChange = iModelBeforeExtentsChange.ecefLocation;
await iModelBeforeExtentsChange.pullChanges(); // Pulls the extents change.
const extentsAfterPull = iModelBeforeExtentsChange.projectExtents;
const extentsStrAfterPull = iModelBeforeExtentsChange.queryFilePropertyString({name: "Extents", namespace: "dgn_Db"});
const ecefLocationAfterExtentsChange = iModelBeforeExtentsChange.ecefLocation;

expect(ecefLocationBeforeExtentsChange).to.not.be.undefined;
expect(ecefLocationAfterExtentsChange).to.not.be.undefined;
expect(ecefLocationBeforeExtentsChange?.isAlmostEqual(ecefLocationAfterExtentsChange!)).to.be.false;
expect(extentsStrAfterPull).to.not.equal(extentsStrBeforePull);
expect(extentsAfterPull.isAlmostEqual(extentsBeforePull)).to.be.false;
await HubWrappers.closeAndDeleteBriefcaseDb(accessToken, iModelBeforeExtentsChange);
});

it("parent lock should suffice when inserting into deeply nested sub-model", async () => {
const version0 = IModelTestUtils.resolveAssetFile("test.bim");
const iModelId = await HubMock.createNewIModel({ iTwinId, iModelName: "subModelCoveredByParentLockTest", version0 });
Expand Down
Loading