Release/2.0.0 #1
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
.NET Standard 2.1 is no longer compatible with .NET Framework—but OnTopic 5.0.0 won't be either, so this is an expected limitation. Moving forward, OnTopic will be targeting .NET Core 3.x and .NET 5.x, both of which are compatible with .NET Standard 2.1.
We'll need to upgrade to the final version of OnTopic 5.0.0 once it releases. Until then, however, this allows us to begin integration testing and development of new features dependent on OnTopic 5.0.0 capabilities, such as Topic References.
…e()` In OnTopic 5.0.0, the legacy entry points to the `TopicRelationshipMultiMap`—as exposed by `Topic.Relationships`—have been changed from `GetTopics()` and `SetTopic()` to `GetValues()` and `SetValue()`. While not as specific, this generalization of the nomenclature unifies the terminology across `Topic.Attributes`, `Topic.References`, and `Topic.Relationships`, and is more consistent with e.g. `TryGetValue()` used by the .NET Framework for similar types of operations.
The order of parameters in `TopicFactory.Create()` was changed to map to the constructor of `Topic`. Notably, this switched the order of `parent` and `id`. This usually only affects `ITopicRepository` implementations and unit tests, since most day-to-day operations won't ever be assigning an `Id`—which, consequently, is a good reason for the `Id` to go last. (Aside, there was previously an overload that allowed the `parent` to be defined without the `id`; as part of this change, that overload was merged.)
In OnTopic 5.0.0, the name of the `AttributeValue` type is changed to `AttributeRecord` as part of a broader effort to unify the naming conventions for combining both data (i.e., `Key`, `Value`) and metadata (i.e., `IsDirty`, `LastModified`). This is also used for the new `TopicReferenceRecord` type. Further, it mitigates the awkward `AttributeValue.Value` syntax.
The semantics of `Topic.DerivedTopic` were not just confusing, but wrong: The "derived topic" referred to the topic that was being derived _from_,, which wasn't captured by the identifier. The new `BaseTopic` better captures that.
As of OnTopic 5.0.0, the `Topic.Relationships` type is a new `TopicRelationshipsMultiMap`, which implements an `IReadOnlyDictionary<>` of `KeyValuesItem`s. As part of this, the name of each relationships is renamed from `Name` to `Key` (which is more consistent with `relationshipKey`) and the values are nested into a new `Values` property.
The new `Clear()` method _requires_ a specific `relationshipKey`, which it uses to clear an individual collection, instead of _removing_ all relationships. This provides improved tracking, thus allowing those relationships to be deleted on `ITopicRepository.Save()`, whereas previously there was no way of tracking a relationship that had been removed entirely. (I.e., if there is a relationship named e.g. 'Related', and it's empty, then `ITopicRepository.Save()` knows to delete all topics in that relationship. If the `Related` relationship is removed entirely, it doesn't evaluate that relationship at all.)
As part of OnTopic 5.0.0, we're adhering to CA1002, which advises using `Collection<>` over `List<>` for externally visible types. This provides a familiar and consistent API that's easier to work with.
In OnTopic 5.0.0, the default `LastModified` value is now set (correctly) using `DateTime.UtcNow` instead of `DateTime.Now`, thus avoiding potential discrepancies with a computer's timezone. Generally, servers are set to use UTC anyway, so this doesn't make a difference. Development machines, however, will typically use local time, thus creating a discrepancy in the test.
In OnTopic 5.0.0, the `BaseTopic` (previously `DerivedTopic`) is no longer stored in the `Topic.Attributes` collection as a `TopicID` attribute. Instead, it is now stored in the strongly typed `Topic.References` collection. As such, checking the `TopicID` is no longer expected to be a valid check. However, the existing equality check against `baseTopic` continues to work. While I was at it, I renamed the key for base topics from `Derived` to `Base`. This doesn't impact the tests, but is more intuitive as a name for human readers of the test or its debugging data.
Upgraded to the release candidate of OnTopic 5.0.0. This includes quite a few breaking changes to the API. Most notably, calls to `Topic.Relationships` now use `GetValues()` and `SetValue()` instead of `GetTopics()` and `SetTopic()`, and `Topic.DerivedTopic` has been renamed to `Topic.BaseTopic`. Further, the `Topic.Relationships` collection now operates off of a new `KeyValuesPair`, instead of a `RelatedTopicCollection`, which changes the property names from `Name` to `Key`, and the values from `Items` to `Values`.
This aligns with the change of `Topic.DerivedTopic` to `Topic.BaseTopic` in OnTopic 5.0.0 (55e952b). This will break backward compatibility with previous JSON files. That will be addressed in a subsequent update.
Previously, all deserialization tests evaluated a `BaseTopicKey` of `null`. As we work with this property, we want to ensure it continues to be properly evaluated.
In OnTopic 5.0.0, the `Topic.DerivedTopic` was renamed to `Topic.BaseTopic` (55e952b). To maintain consistency, the `TopicData.DerivedTopicKey` was renamed to `TopicData.BaseTopicKey` (22b9866). But previously exported data will still have the legacy `DerivedTopicKey` property set in the JSON. This test determines whether that is correctly imported and associated with the new `BaseTopicKey` property. Note: This test will fail as of this commit, as there's currently no effort to handle this special requirement. Subsequent commits will resolve this issue.
In a previous commit, the `DerivedTopicKey` property was renamed to `BaseTopicKey` to maintain parity with OnTopic 5.0.0's change of `Topic.DerivedTopic` to `Topic.BaseTopic`. Unfortunately, however, that prevents previously exported JSON files from being properly imported. To mitigate that, I've reintroduced a (deprecated) `DerivedTopicKey` property, and set `BaseTopicKey`'s getter to fallback to it if `BaseTopicKey` isn't otherwise defined. This has the unfortunate side-effect of writing a null `DerivedTopicKey` property to the exported JSON. This can be mitigated by including the `IgnoreNullValues` on the `JsonSerializerOptions`, which we recommend doing, and will later add in to the OnTopic Editor's integration. Later, when we upgrade to .NET 5.x, we'll be able to handle this better by treating `DerivedTopicKey` as a private field with the `[JsonInclude]` attribute along with the `[JsonIgnore(JsonIgnoreCondition.WhenWritingDefault)]` attribute. But, for now, we want to maintain backward compatibility with .NET Core 3.x, and so those aren't yet available.
This is the preferred serialization setting that we recommend, in part due to legacy properties such as `DerivedTopicKey` (4bbeedc).
In OnTopic 5.0.0, the `Topic.DerivedTopic` property was renamed to `BaseTopic`, which makes more sense. To align with this, a new `BaseTopicKey` property was added to the `TopicData` class. Legacy data will still use `DerivedTopicKey`, and so that property is still being maintained; in fact, if a value for `BaseTopicKey` isn't set, it will now fallback to `DerivedTopicKey`. Unfortunately, .NET Core 3.x doesn't permit this property to be `private`, nor for it to be excluded from serialization. As such, serialization of the `TopicData` class will include _both_ the `BaseTopicKey` _and_ the legacy `DerivedTopicKey`. This can be avoided by using the `IgnoreNullValues` options on `JsonSerializerOptions` when calling `Serialize()`. If that isn't done, however, the legacy property will pollute the interface as a null value. Later, once we upgrade the library to .NET 5.x, we'll be able work around this by marking `DerivedTopicKey` as `private`, and applying the `[JsonInclude]` and `[JsonIgnore()]` attributes, thus allowing it to be set, but optionally hiding it from serialization if it's `null`. For now, however, there isn't any other compelling reason to update OnTopic to .NET 5.x, since a) we want to maintain backward compatibility with .NET 3.x, and b) this is one of the few features we've identified that would justify e.g. multi-targeting .NET 5.
This can reuse the existing `AttributeDataCollection` and `AttributeData` item type since topic references get serializes with a string `Value` and are tracked via a `LastModified` value. In the future, we may want to update the name of the `AttributeDataCollection` and `AttributeData` classes to make them more generic, but we'll address that separately. (It doesn't impact either the serialization or the deserialization directly.)
This ensures that the serialization unit tests correctly expect a(n empty) `References` array, as per the introduction of the `References` collection (aac6394).
We have `DeleteUnmatched` options for `Attributes`, `Relationships`, `NestedTopics`, and `Children`. Now, with the introduction of `References`, we need a corresponding `DeleteUnmatchedReferences`, to extend that capability.
Write to the new `TopicData.References` collection (aac6394) during serialization via the `Export()` extension method. This is very similar to the handling of attributes, since they are both sources from a `TrackedRecordCollection` and both write to an `AttributeDataCollection`.
Read from the new `TopicData.References` collection (aac6394) during deserialization via the `Import()` extension method. This is very similar to the handling of attributes, since they are both sourced from an `AttributeDataCollection` and both write to a `TrackedRecordCollection`.
Previously, the (local) `unresolvedRelationships` collection handled tracking of relationships that could not be resolved on the initial pass, and would attempt to wire them up in a subsequent pass. This has now been generalized to support both `Topic.Relationships` and `Topic.References`. To acknowledge that, the variable has been renamed to `unresolvedAssociations`.
In OnTopic 5.0.0, the `BaseTopic` (previously `DerivedTopic`) is now handled via the (new) `Topic.References` feature. This prevents the need to specially account for it, as there's now first-class support for this _type_ of association, independent of the specifics of `BaseTopic`. With topic references now accounted for during the `Import()` process (6d5938b), there's also no need for specialty handling of `BaseTopic`, so long as it's passed via a `TopicData.References`. This test sets up an `AttributeData` with a key of `BaseTopic`, adds it to the `TopicData.References` collection, and ensures that it is successfully wired up.
With the rename of `DerivedTopicKey` to `BaseTopicKey`, the alignment of the variables was off. While I was at it, I also updated one of the assignments to use the root value from the `TopicData` object, so as to not repeat string values. (This is consistent with how we handle this situation in other tests.)
If the `BaseTopicKey` is set, but the reference cannot be resolved, there's no need to provide special handling for this situation. If we store the key of the `unresolvedAssociations` as `BaseTopic`, then the standard handling of topic references will take care of it without any (further) special attention (6d22172).
For new exports, there's no need to set the `BaseTopicKey`; this will be handled automatically by the `TopicData.References`, since `Topic.BaseTopic` corresponds to a `Topic.References` entry. In fact, really, we only need the `BaseTopicKey` for legacy support—which calls into question the entire basis for introducing the `BaseTopicKey` to begin with (4705b29). We'll likely address that in a subsequent update. As part of this, I also updated the unit test to look for the `BaseTopic` inside of the `TopicData.References` collection, instead of within the `BaseTopicKey`.
Since `TopicData.References` is now the preferred way of handling `Topic.BaseTopic` (7c20dd0), the handling of `BaseTopicKey` is really only present for backward compatibility. Given that, instead of separately handling the `BaseTopicKey`, we should just use it to establish a `BaseTopic` entry in `TopicData.References`, if one doesn't already exist. This has a few benefits. First, it ensures that the `ImportOptions` are honored, such as honoring `ImportStrategy.Add`, if set, instead of always overwriting the `BaseTopic`—and, therefore, the associated entry in `Topic.References`. Second, it better accounts for unexpected scenarios where both the `BaseTopicKey` and a `BaseTopic` reference are set (in that case, the newer `BaseTopic` reference should always be preferred). Third, it centralizes the lookup of referenced topics to one location.
Now that `BaseTopicKey` is no longer set as part of `Export()`—and is therefore not expected from any OnTopic Data Transfer 3.0.0 library exports—the deserialization test that evaluates `BaseTopicKey` should, instead, explicitly evaluate `DerivedTopicKey` with the sole intention of verifying backward compatibility with the legacy JSON format from the OnTopic Data Transfer 2.x library.
These maintain feature parity, but moving forward only `GitVersion.MsBuild` will continue to be maintained.
This ensures that the symbols published as part of the symbols package can be tied back to specific commits of specific source files on GitHub, and thus should allow implementors to step into OnTopic code when debugging their applications. This is a build-only dependency, and won't introduce additional dependencies for implementors.
As part of our build process, we ensure that a separate symbols package is generated. As such, this doesn't technically need to be repeated here. But as this is a best practice and one we want to enforce, we're making sure this is explicit.
There aren't currently any untracked resources in these projects, but if any are introduced later, we want them to be tracked by SourceLink.
This helps SourceLink determine that this is a _deterministic_ build.
Introduced Microsoft `SourceLink`, along with recommended configuration settings. Alongside the symbols package, this will help ensure that consumers of the NuGet packages will be able to step into OnTopic code during debugging by associating it with specific commits of specific files on GitHub. While I was at it, I also migrated from the deprecated `GitVersionTask` to `GitVersion.MsBuild`. This maintains feature parity, while giving us access to new builds going forward. I also reverted the previous update to the `GitVersion.yml`, which didn't behave as intended. Together, these improve our build process.
For consistency with other projects, the `Icon.png` is placed within the root of the solution, even though it's only needed within the `OnTopic.Data.Transfer` project itself. For now, we're using the Ignia logo as the icon, and have set it to 128x128 per NuGet recommendations.
We previously hard-code this, but SourceLink can instead automatically generate the git repository reference. This is needed for SourceLink to provide a reference to the exact code referenced by the symbols package. As part of this, combined two `<PropertyGroup/>`s which were both related to NuGet package metadata.
We are diligent about maintaining XML Docs for all types and members. But we don't currently _do_ anything with that documentation. By auto-generating it on compilation, we at least ensure that it will end up being included in NuGet packages—and thus available to IntelliSense for implementers. In addition, this will allow us to use e.g. DocFX in the future as part of our CI/CD process, should we choose. As part of this, accept the default XML output path. This is the same as the one we have configured, and there's no real benefit to being explicit in this case.
This isn't strictly necessary for the OnTopic Data Transfer solution since, currently, it only has one project. By separating out the common properties into the `Directory.Build.props` file, however, we ensure consistency and familiarity with other OnTopic projects, while simultaneously keeping the `csproj` file focused on the properties that are truly specific to it.
Improved metadata associated with NuGet packages. This includes: - Added the Ignia icon to the package - Include the XML Docs with the package in order to maintain IntelliSense support for implementers - Used SourceLink's `PublishRepositoryUrl` option to automatically inject the Git repository URL - Centralized common properties into the root `Directory.Build.props` for consistency with other projects
Some type and member references were not properly resolved. This mostly included missing namespaces and outdated method signature. This resolves CS1574.
Resolved CS1570.
This resolves CS1573, CS1591.
When we started auto-generating documentation on build, code analysis identified a number of issues with our XML Docs, such as broken references and malformed XML. These issues have been resolved.
In the context of unit test assertions, this is almost always resolved by using the null-conditional operator (`?.) and updating the reference type to accept nulls. This resolves CS8602.
In context of unit test assertions, this can almost always be resolved by simply updating the generic type parameter to accept nulls. Resolves CS8604.
With the introduction of the common `Directory.Build.props` file, which includes the `<Nullable />` property, the unit test is now subject to nullable reference analysis. That's a good thing, and something we needed to implement anyway, as it helps account for potential null references, and ensures we're honoring the contracts of the members we're testing. Given this, the unit tests have been updated to be nullability aware—which, in most cases, simply meant updating the assertions to accept potentially null types, and potentially using the null-conditional operator.
This only impacts the build and test process, and has no impact on downstream consumers. All unit tests continue to execute as expected after this update.
While we'd like to use Coverlet, we previously ran into notable bugs on their web application interface, thus preventing us from adopting it. Until we have time to investigate further, we're removing it.
This will correspond with the release of OnTopic Data Transfer 2.0.0 Beta 4. This isn't expected to have any impact on the Data Transfer library itself, as the changes since Beta 1 are outside the scope of the Data Transfer dependencies.
Updated dependencies to their latest version. Notably, this includes updating to OnTopic 5.0.0 Beta 4, to align with the release of OnTopic Data Transfer 2.0.0 Beta 4. In addition, I also removed Coverlet since we're not using it, and bumped up the version of the .NET Test SDK.
This only affects the unit tests, and has no downstream impact on implementers. Ensured all unit tests continue to operate correctly.
Updated to latest version of Microsoft's Test Framework. Reran unit tests to confirm everything continues to operate correctly. As this only impacts the unit test projects, this has no impact on consumers of the NuGet packages.
Updated to the final release version of OnTopic 5.0.0.
Previously, the OnTopic Data Transfer library has been operating off of preview releases of OnTopic 5.0.0. With the final release version now available, we're able to update to the RTM version.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
OnTopic Data Transfer 2.0.0 is a major release primarily focused on support for OnTopic 5.0.0's new topic references feature. It includes changes to the JSON serialization format—though it also maintains backward compatibility with the legacy format.
Contents
New Features
General
BaseTopicand merging topic references based onLastModified(2cfbc52, d0cb983).Import()now tracks any associations that it is not able to resolve and attempts to resolve them after the initialImport()has completed, thus accounting for associations to topics that weren't yet imported because they occur later in the interchange data (4a75ff6). This prevents the need for callingImport()twice to ensure that there aren't any unresolved associations.NuGet Package
SourceLinkwith references to GitHub commits so that packages can be properly debugged by implementers (8e1a5cc).Breaking Changes
The following changes impact the interchange format, but maintain backward compatibility for derserializing legacy JSON files.
DerivedTopicKey: MarkedDerivedTopicKeyas deprecated, and no longer write it duringExport()(4705b29, 6c33fb8).AttributeData(Collection): RenamedAttributeDatatoRecordDataandAttributeDataCollectiontoRecordDataCollectionto maintain parity with theTrackedRecord(Collection)in OnTopic, and to provide base support for both attributes as well as topic references (a5df9b1).RelationshipData(Collection): RenamedRelationshipDatatoKeyValuesPairandRelationshipDataCollectiontoMultiMapto maintain parity with theKeyValuesPair<T>andTopicRelationshipMultiMapin OnTopic (bc15631). The JSON now writes toRelationships: [{ …, Values: […]}]instead ofRelationships: [{ …, Relationships: […]}].Code Improvements
Topic.DerivedTopictoBaseTopicandTopic.Relationships.SetTopic()toSetValue()(2f75a73).System.Json.Text5.0.0, which allows us to take advantage of .NET 5 annotations, such as[JsonInclude]and[JsonIgnore()], while maintaining compatibility with .NET Core 3.x (439b41c).NetAnalyzers), as well as the feedback it exposed, including nullable reference annotations (7fa983c).