itwinjs-core
itwinjs-core copied to clipboard
Changes to Channels
- Remove hasChannels API.
- Remove sharedChannel from allowedChannels list by default.
- Update tests, API and documentation regarding creation of Channels generally against Partitions instead of Subjects.
I suggest you get rid of the concept of "hasChannels" entirely and presume there's always channels. All the "legacy channel" stuff can be eliminated and just say that's all part of the (non-editable by default) shared channel. No need to propagate that at all since nothing from the old connectors should be editable by iTwin.js.
I suggest you get rid of the concept of "hasChannels" entirely and presume there's always channels.
note: Will require some changes to snapshots created by tests that currently rely on early return for hasChannels being false.
/azp run iTwin.js
Azure Pipelines successfully started running 1 pipeline(s).
/azp run iTwin.js Docs - Yaml
Azure Pipelines successfully started running 1 pipeline(s).
/azp run iTwin.js Integration - GitHub
Azure Pipelines successfully started running 1 pipeline(s).
How would someone test whether a Channel root exists?
I think your question can be interpreted in two ways:
- Is a particular Element a Channel Root itself?
- Is there any Channel Root Element in an iModel for a particular "Channel key"?
1 - is covered by: iModelDb.channels.getChannelKey(elementId) returning a Channel key different than "shared"
We're missing an API for 2) e.g.
queryChannelRoots(channelKey: ChannelKey): Id64Set
A third case that we could add an API for is: What are the Channel Keys used in my iModel? e.g.
queryChannelKeys(): ChannelKey[]
- Is there any Channel Root Element in an iModel for a particular "Channel key"?
Yes, this is what i'm asking. Suppose you want to create a new model "in a channel". How can i find the channel root element, if it exists?
queryChannelRoots(channelKey: ChannelKey): Id64Set
How can this return more than one element? I think it should be:
queryChannelRoot(channelKey: ChannelKey): Id64String | undefined
What we have discussed is that we need multiple Channel Roots per Channel Key because an authoring app can store its data across multiple Partitions in the same iModel. That is:
- A user (admin) configures the expected Subject hierarchy for their iModels.
- An authoring app needs to be told where in the Subject hierarchy to store its data (we haven't finished the discussion on how that happens).
- An authoring app may have been designed to create its data across multiple partitions. E.g. a PhysicalPartition, a FunctionalPartition, a DefinitionPartition and a DocumentPartition. Each one of those 4 partitions are created under the correct Subject(s) from step 2), and all thsoe four need to be made ChannelRoot elements with the same ChannelKey (e.g. "myAppKey").
A "Channel" is then the union of those 4 model-branches, each one with a "ChannelRoot".
What we have discussed is that we need multiple Channel Roots per Channel Key because an authoring app can store its data across multiple Partitions in the same iModel. That is:
- A user (admin) configures the expected Subject hierarchy for their iModels.
- An authoring app needs to be told where in the Subject hierarchy to store its data (we haven't finished the discussion on how that happens).
- An authoring app may have been designed to create its data across multiple partitions. E.g. a PhysicalPartition, a FunctionalPartition, a DefinitionPartition and a DocumentPartition. Each one of those 4 partitions are created under the correct Subject(s) from step 2), and all thsoe four need to be made ChannelRoot elements with the same ChannelKey (e.g. "myAppKey").
A "Channel" is then the union of those 4 model-branches, each one with a "ChannelRoot".
I don't like this, it violates the theory that all channels are singularly rooted. That is important if we try to categorize changesets by the channels they modify (that will be important, i think). Why not have 4 channels?
That works for authoring apps. They will just need to use a different key per Partition (e.g. "myAppKey-Physical", "myAppKey-Documents", etc). If that's the rule, I'll enforce it in makeChannelRoot (i.e. not allowing duplicates) and update the md article on the topic.
BTW, the reusage of a ChannelKey across multiple ChannelRoots was discussed in light of LocalSync last year. I believe @swwilson-bsi is already reusing a ChannelKey on each Partition that gets created (mapped to an external DgnModel). That logic will have to change so unique ChannelKeys get assigned (e.g. "MstnLocalSyncKey-[dgnfilename-dgnmodelname]").
BTW, the reusage of a ChannelKey across multiple ChannelRoots was discussed in light of LocalSync last year. I believe @swwilson-bsi is already reusing a ChannelKey on each Partition that gets created (mapped to an external DgnModel). That logic will have to change so unique ChannelKeys get assigned (e.g. "MstnLocalSyncKey-[dgnfilename-dgnmodelname]").
Either that or he can make one channel root ("MstnLocalSync") and create models for each dgn in it. I don't see why he needs more than one channel.
If he creates one channel root, I assume you're referring to a single Subject applicable to multiple DgnFiles mapped via LocalSync.
That blocks the whole premise of a user being in control of the Subject hierarchy (i.e. being able to re-label/re-parent such Subject). That is, a user may want to move around where data (i.e. a model) mapped from a DgnFile-DgnModel via LocalSync is located WRT the overall Subject hierarchy.
One of the main issues discussed with @swwilson-bsi and the BWG regarding Batch Connectors is that they coupled their sync algorithm with a particular layout of the Subject hierarchy. Several users have expressed frustration that they can't change it (and the Subject hierarchy was originally intended to be in their hands anyway). The goal that came out of those discussions was to address such issue in LocalSync and authoring apps.
If he creates one channel root, I assume you're referring to a single Subject applicable to multiple DgnFiles mapped via LocalSync.
That blocks the whole premise of a user being in control of the Subject hierarchy (i.e. being able to re-label/re-parent such Subject). That is, a user may want to move around where data (i.e. a model) mapped from a DgnFile-DgnModel via LocalSync is located WRT the overall Subject hierarchy.
I don't see why. There can be Subjects below the channel root, right?
Legacy channels (created by Batch Connectors as Json on certain Subjects) can certainly have Subjects below them. But that approach prevents users from laying out a Subject hierarchy that works for them (ala Folder-structure in Projectwise), which was the original premise used when such concept was introduced in BIS during its early days.
Thus, for LocalSync and authoring apps, we're working with that premise in mind. So, if Subjects can be defined/rearranged by users, Subjects can't be coupled to software-based expectations. That brings us to Channels being rooted at Partitions instead (i.e. No Subjects under Partitions) or, in special cases, rooted at certain DefinitionContainers (when a Channel is needed at the repository-global/DictionaryModel level).
This introduces a breaking change of the worst kind: one that does not produce a compiler error, but causes your code to no longer work properly.
At a minimum, people who upgrade to this new version will need to add the following line to make it work:
iModel.channels.addAllowedChannel("shared")
Better, they would start using channels properly.
Besides our own apps, we know of third party connectors using the TypeScript APIs that will be impacted by this. @calebmshafer how do you want to proceed?
This introduces a breaking change of the worst kind: one that does not produce a compiler error, but causes your code to no longer work properly. At a minimum, people who upgrade to this new version will need to add the following line to make it work:
iModel.channels.addAllowedChannel("shared")Better, they would start using channels properly.Besides our own apps, we know of third party connectors using the TypeScript APIs that will be impacted by this. @calebmshafer how do you want to proceed?
I think the universe of affected applications is small, but definitely not zero. I suppose one approach to this is to add a new IModelHostOption field to disable channel checking altogether (that's the previous behavior). It would have to default to true, but iTS (and our tests) would set it to false.
Transformers will need to be reworked to become channel-aware, because what they're doing now (preserving channels) is definitely wrong.
I think the universe of affected applications is small, but definitely not zero. I suppose one approach to this is to add a new IModelHostOption field to disable channel checking altogether (that's the previous behavior). It would have to default to true, but iTS (and our tests) would set it to false.
I added something for this. It means that we'll have to tell the iTS team to add it to their startup options. Probably warrants an entry in nextversion too (which I didn't write). I made our backend tests override the default so they test the new behavior. I restored the examples to not allow shared channels and instead prove backwards compatibility.
@jchick-bentley fyi as this will affect the connector framework.
@aruniverse
@jchick-bentley fyi as this will affect the connector framework.
@aruniverse
Thanks Caleb - I've been monitoring.