This repository has been archived by the owner. It is now read-only.

Convert null-valued results to NotFoundResult in ActionResult<> #6848

Closed
pranavkm opened this Issue Sep 19, 2017 · 8 comments

Comments

@pranavkm
Copy link
Member

pranavkm commented Sep 19, 2017

Consider the following API:

public async Task<ActionResult<Contact>> Get(int id)
{
    var contact = await DbContext.Contacts.Where(item => item.ContactId == id).SingleOrDefaultAsync();
    if (contact == null)
    {
        return NotFound();
    }
    
    return contact;
}

We could reduce this by assuming ActionResult(null) indicates a NotFoundResult rather than an ObjectResult with a null value (which is what we do now). Resulting code:

public Task<ActionResult<Contact>> Get(int id)
{
    return DbContext.Contacts.Where(item => item.ContactId == id).SingleOrDefaultAsync();
}
@pranavkm

This comment has been minimized.

Copy link
Member

pranavkm commented Sep 19, 2017

Since the type is new, we could build this behavior in to ActionResult<T>: https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Core/ActionResultOfT.cs#L53-L59. However, it would be nicer if it had bells and whistles to disable this behavior.

@pranavkm pranavkm changed the title ApiController: Convert null-valued results to NotFoundResult in ActionResult<> Convert null-valued results to NotFoundResult in ActionResult<> Sep 19, 2017

@Eilon

This comment has been minimized.

Copy link
Member

Eilon commented Sep 20, 2017

This is specifically for [ApiController] conventions?

cc @rynowak

@Eilon Eilon added the super-triage label Sep 20, 2017

@rynowak

This comment has been minimized.

Copy link
Member

rynowak commented Sep 20, 2017

I'm really not a big fan of this, it's been brought up before and I didn't like it then

@rynowak rynowak added this to Design in KodKod 2.1.0 Sep 21, 2017

@rynowak

This comment has been minimized.

Copy link
Member

rynowak commented Oct 30, 2017

Closing this, we've discussed it and feel like it's a little inconsistent and non-obvious. We can't always guess whether you wanted a 200, 204 or 404.

@rynowak rynowak closed this Oct 30, 2017

@rynowak rynowak added wontfix and removed super-triage labels Oct 30, 2017

@rynowak rynowak removed this from Design in KodKod 2.1.0 Oct 30, 2017

@pranavkm pranavkm removed the wontfix label Jun 4, 2018

@pranavkm

This comment has been minimized.

Copy link
Member

pranavkm commented Jun 4, 2018

Reopening for reconsideration

@pranavkm

This comment has been minimized.

Copy link
Member

pranavkm commented Jul 31, 2018

Moving this out to preview2 for design work

@rynowak

This comment has been minimized.

Copy link
Member

rynowak commented Aug 1, 2018

reconsideration

@pranavkm

This comment has been minimized.

Copy link
Member

pranavkm commented Sep 13, 2018

Closing again. We can review this later if there's user ask.

@pranavkm pranavkm closed this Sep 13, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.