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

Jump to bottom

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

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ef92ce2
updateProjectExtents on changesetapplied event and add test
nick4598 Sep 23, 2024
33f7abd
Merge branch 'master' into nick/refreshextentsafterpullchanges
nick4598 Sep 23, 2024
0d5db65
rush change
nick4598 Sep 23, 2024
2aaa285
pass "pullMergef" to initializeImodelDb
nick4598 Sep 24, 2024
eeab6bb
lint error
nick4598 Sep 24, 2024
51ed0c0
run rush extract-api
nick4598 Sep 25, 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
10 changes: 10 additions & 0 deletions common/changes/@itwin/core-backend/nick-refreshextentsafterpullchanges_2024-09-23-18-57.json
View file
Open in desktop
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"
}
7 changes: 7 additions & 0 deletions core/backend/src/IModelDb.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

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

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;
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to passing pullMerge into intializeIModelDb

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?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

I dont think its possible to from defined extents to undefined, the other way around yes

// Note: updateProjectExtents will return early if the extents are the same as the current extents.
if (extents)
this[_nativeDb].updateProjectExtents(extents, true);
});
IModelDb._openDbs.set(this._fileKey, this);

if (undefined === IModelDb._shutdownListener) { // the first time we create an IModelDb, add a listener to close any orphan files at shutdown.
Expand Down
38 changes: 36 additions & 2 deletions core/backend/src/test/standalone/IModelWrite.test.ts
View file
Open in desktop
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 All @@ -24,7 +24,7 @@ import {
BriefcaseDb,
BriefcaseManager,
ChannelControl,
CodeService, DefinitionModel, DictionaryModel, DocumentListModel, Drawing, DrawingGraphic, OpenBriefcaseArgs, SpatialCategory, Subject,
CodeService, DefinitionModel, DictionaryModel, DocumentListModel, Drawing, DrawingGraphic, OpenBriefcaseArgs, SnapshotDb, SpatialCategory, Subject,
} from "../../core-backend";
import { IModelTestUtils, TestUserType } from "../IModelTestUtils";
import { ServerBasedLocks } from "../../internal/ServerBasedLocks";
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