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.

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

How to support creating links for sub-collections in the PagedListRepresentation sample? #42

Open
jasonmitchell opened this issue Feb 23, 2021 · 6 comments
Labels
question Further information is requested

Comments

@jasonmitchell
Copy link
Owner

The Hallo.Sample project includes an example PagedListRepresentation<T> which accepts the baseUrl for the collection. Below is the snippet of how this is constructed.

public class PersonListRepresentation : PagedListRepresentation<Person>
{
public PersonListRepresentation(PersonRepresentation personRepresentation)
: base("/people", personRepresentation) { }
}

This works fine for constant base URLs like in this example but is more difficult when the base URL is dynamic. For example, if the URL to the collection was /race/123/cars (where 123 is the resource id) how do we support getting the race id into the base URL?

@jasonmitchell
Copy link
Owner Author

jasonmitchell commented Feb 23, 2021

This is pretty fiddly. I think it's going to be tricky to come up with a universal solution for this which remains inline with the intention that Hallo requires no modification to controllers or models, and which doesn't have an over-complicated setup.

Two very similar options are below.

The first requires IActionContextAccessor as a parameter and then accesses the RouteData to find the raceId. This is highly MVC-specific as this interface comes from the Microsoft.AspNetCore.Mvc namespace.

public class CarListRepresentation : PagedListRepresentation<Car>
{
    public CarListRepresentation(CarRepresentation carRepresentation, IActionContextAccessor actionContextAccessor)
        : base($"/races/{actionContextAccessor.ActionContext.RouteData.Values["raceId"]}/cars", carRepresentation)
    { }
}

The second option is basically the same but makes use of Microsoft.AspNetCore.Http.IHttpContextAccessor and so is not specific to MVC though I haven't tested this outside of MVC yet.

public class CarListRepresentation : PagedListRepresentation<Car>
{
    public CarListRepresentation(CarRepresentation carRepresentation, IHttpContextAccessor httpContextAccessor)
        : base($"/races/{httpContextAccessor.HttpContext!.GetRouteValue("raceId")}/cars", carRepresentation)
    { }
}

Both options require intimate knowledge of the route values from the endpoint which could potentially be a fragile dependency - someone might do a seemingly harmless rename on the route value which would quietly break this. It's also pretty ugly looking but it could be tidied up with a developers own abstraction if needs be.

This also increases the risk of weird lifestyle issues if using the ASP.NET Core dependency injection mechanism and a developer registers Hal<T> services as singletons because the base URL in this example would always have the first raceId handled by the endpoint. This could be avoided by a different implementation of PagedListRepresentation<T> which resolves the baseUrl differently (e.g. by an abstract method).

Lastly these options really limit the portability of representations. If we wanted to reuse this same representation for a sub-collection under another resource then we couldn't use the same CarListRepresentation across all (and perhaps this is ok).

I will keep experimenting with options for this though I think some trade offs will have to be accepted in this scenario.

@jasonmitchell
Copy link
Owner Author

Example resolving baseUrl via abstract method:

CarListRepresentation:

public class CarListRepresentation : PagedListRepresentation<Car>
{
    private readonly IHttpContextAccessor _httpContextAccessor;

    public CarListRepresentation(CarRepresentation carRepresentation, IHttpContextAccessor httpContextAccessor)
        : base(carRepresentation)
    {
        _httpContextAccessor = httpContextAccessor;
    }

    protected override string GetBaseUrl() =>
        $"/races/{_httpContextAccessor.HttpContext!.GetRouteValue("raceId")}/cars";
}

@jchannon
Copy link
Contributor

@jchannon
Copy link
Contributor

Actually you won't want that as you'll want relative paths but you could just use the request path?

@jasonmitchell
Copy link
Owner Author

Yeah you could. I think there are lots of options for this and I'm not sure that the library can cover all scenarios for everyone. What might help is a better sample which shows a more complex scenario which can at least provide a starting point for people to adapt.

@jchannon
Copy link
Contributor

jchannon commented Feb 25, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants