TopicMappingService: Proactively check type compatibility
#83
Assignees
Labels
Area: Mapping
Relates to one of the `ITopicMappingService` interfaces or implementations.
Priority: 1
Severity 1: Minor
Status 5: Complete
Task is considered complete, and ready for deployment.
Type: Improvement
Improves the functionality or interface of an existing feature.
Milestone
Comments
JeremyCaney
added
Area: Mapping
JeremyCaney
added a commit
that referenced
this issue
Apr 5, 2021
The private `GetValidatedMappingType()` method identifies the mapping type—either by directly passing it directly, or inferring it from the `ContentType` of a source `Topic`—and validates that it can be assigned by the `expectedType`, which is the type of the property or collection it is being assigned to. If it can, it returns the mapping type is returned; otherwise null is returned. This will aid in validating type compatibility _before_ mapping a topic, thus reducing the overhead of using type compatibility as a shortcut for filtering, as called for in #83. This will be implemented in subsequent commits.
JeremyCaney
added a commit
that referenced
this issue
Apr 5, 2021
The `PopulateTargetCollectionAsync()` method takes topics from a collection on the source topic and maps them to models in a collection property on the target model. If the mapped topic type isn't compatible with the collection property type, then it is excluded, as a way of filtering out incompatible types. By incorporating the new `GetValidatedMappingType()` method (88c7fba), we pre-validate the type compatibility, instead of requiring that the model first be mapped—a potentially expensive operation. This satisfies relationships, children, and other collections, as called for by #83.
JeremyCaney
added a commit
that referenced
this issue
Apr 5, 2021
The `GetTopicReferenceAsync()` method takes a topic reference from the source topic and maps it to a property on the target model. If the mapped topic type isn't compatible with the target property type, then it is excluded, as a way of filtering out incompatible types. By incorporating the new `GetValidatedMappingType()` method (88c7fba), we pre-validate the type compatibility, instead of requiring that the model first be mapped—a potentially expensive operation. This satisfies the topic reference requirement called for in #83.
JeremyCaney
added a commit
that referenced
this issue
Apr 5, 2021
…elop Introduced a new `GetValidatedMappingType()` method which determines and validates the mapping type against a target type (88c7fba). This method is then integrated with both `PopulateTargetCollectionAsync()` (7caa832) and `GetTopicReferenceAsync()` (a31864a) as a way of pre-validating type compatibility _prior_ to mapping the source topic, thus offering a more efficient way of filtering out incompatible types. Previously, the source topics were first mapped and only then filtered out, thus potentially wasting resources mapping topics that would never be used. This could be especially expensive if the models those topics were being mapped to `[Include()]`d associations. This satisfies the requirements of #83.
JeremyCaney
added
Status 5: Complete
|
Introduced a new |
Merged
JeremyCaney
added a commit
that referenced
this issue
Apr 12, 2021
OnTopic 5.1.0 is a minor release which introduces support for mapping constructor parameters (#35) and defining what model to use when mapping associations via the `[MapAs()]` attribute (#41). Primarily, however, it is focused on bug fixes, and resolves a number of priority issues, such as an exception which occurs when deleting topics with associations (#47), topics with deleted references being treated as _unresolved_ (#46), and the inability to move a first child to another parent (#76). Finally, it also includes a number of improvements, such as checking type compatibility before mapping an association (#83), migrating the unit tests to xUnit.net (#66), and establishing integration tests for ASP.NET Core (#39). For a full rollup of new features, improvements, and bug fixes, see Pull Request #85.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, the
TopicMappingServicemaps associated topics, and then evaluates if they’re compatible with the target type. If they’re not, the association is discarded.Instead, it would be preferable to lookup the target
Typeahead of time, using theITypeLookupService, and only map the topic if the resulting type will be compatible. That way, unnecessary mappings aren’t performed.This has its drawbacks. It means looking up the type in each of the several places where we call to map associations. But with the type, we can call the overload of
MapAsync()that accepts aTypeparameter to avoid it needing to callITypeLookupServiceagain. And that prevents the need to conditionally call differentMapAsync()overloads, as we do today in order to support[MapAs()]. Plus, we may be able to centralize this logic via a helper method.This makes it easier to take an eager modeling strategy when the types are unknown, having confidence that type mismatches will be filtered out with comparatively little performance overhead. By contrast, the cost of this filtering today is expensive since the entire object—and, potentially, it’s associations!—needs to be mapped, independent of whether or not it’s used.
The text was updated successfully, but these errors were encountered: