LinkCollectionExtensions.ResolveAndAdd by (object, relation, context) calls wrong IOpenApiWebLinkResolver.Resolve overload
idg10 opened this issue · 1 comments
Describe the bug
The LinkCollectionExtensions
class defines three overloads of ResolveAndAdd
. One of these is meant to identify the relevant link by the combination of the owner (which typically means a particula content type), the relation, and the context. This method depends on an IOpenApiWebLinkResolver
, which offers a Resolve
method with three equivalent overloads, and it should invoke the corresponding overload (i.e., the one that keys off owner, relation, and context). Unfortunately it calls the wrong one: it calls the one that resolves by owner and relation type. Worse, it lasses the context as the argument that is meant to contain the relation type!
It is my (@idg10 ) view that this is symptomatic of a design issue in Menes: we are using method overloading in an error-prone fashion. There are three different modes of link resolution, but they are all presented through methods of the same name. (In IOpenApiWebLinkResolver
all three methods are called Resolve
; the LinkCollectionExtensions
defines three ResolveAndAdd
methods.) This might not be a problem if it weren't for the fact that the argument types by which the mechanism gets selected are specific combinations of string
and object
.
It's very easy to call the wrong overload by mistake, as demonstrated by the existence of this bug.
I think we should consider these alternatives:
- introduce new types to represent relations and contexts instead of using
string
for both; this would make overload resolution less dependend on arbitary minor differences between method signatures - use different method names for the different cases (e.g., specify the mode of resolution in the method name)
Based on internal discussion, it looks like 2 is currently prefered.
To Reproduce
Steps to reproduce the behavior:
- Create a project based on Menes
- Register a (relation, context, operationId) mapping in
IOpenApiHostConfiguration.Links
- Attempt to use that mapping through the corresponding
LinkCollectionExtensions.ResolveAndAdd
extension method - A
OpenApiLinkResolutionException
will be thrown.
For example, here's a mapping registration:
host.Links.Map<RedactedSummaryResponse>("prev-period", nameof(RedactedServices.GetRedactedByWeek), nameof(RedactedServices.GetRedactedByWeek));
This is from a real example, which is using the operation id as the context, which makes things mildly confusing when debugging, but does actually make sense for the application in question. (It wants different mappings for the same content type in different operations.) Here's the attempt to use it that fails:
responseHalDocument.ResolveAndAdd(this.linkResolver, response, "prev-period", nameof(GetRedactedByWeek), ("userId", userId), ("year", previousPeriodYear), ("weekNumber", previousWeekNumber));
Expected behavior
The exception should not be thrown. The service should be able to produce a response with correctly populated links.