Looking for urgent help with API

Were you trying to store a duplicate of a foreign record in the table? You might have got a circular dependency but as was said, more than likely you are creating a record with a duplicate key but by the wording, it might not be the primary key of the table in question. You likely had a uniqueness on the foreign relationship ie enforcing a one to one when it should be many to one.
 
Kosmik said:
Were you trying to store a duplicate of a foreign record in the table? You might have got a circular dependency but as was said, more than likely you are creating a record with a duplicate key but by the wording, it might not be the primary key of the table in question. You likely had a uniqueness on the foreign relationship ie enforcing a one to one when it should be many to one.
Click to expand...
EF Core > 2.2 doesn’t play fast and loose with relationships, they have to be well defined.
Delete Restrict on a FK for example would indicate to EF the column is NOT NULL so it can use INNER JOINS.

The repo approach also muddles things because if the context has no tracking of the entity it will try and insert it.
 
^

Yeah you know I couldn't quite figure it out because the client aka human client, gave me all the tests beforehand, the actual XUnit tests. These were created against the controllers that did not have asynchronous calls.

So what I did was in order to upgrade the project as per his advise, I did a whole bunch of stuff in the project and one of the things was to add asynchronous controllers.

And guess what, the tests still ran fine. Nothing broke, no errors. I'm going to actually go with you and kabal especially since you guys have pulled my butt out of the fire a few times in the past when I've been backed into a corner with dev stuff
 
Kosmik said:
Were you trying to store a duplicate of a foreign record in the table? You might have got a circular dependency but as was said, more than likely you are creating a record with a duplicate key but by the wording, it might not be the primary key of the table in question. You likely had a uniqueness on the foreign relationship ie enforcing a one to one when it should be many to one.
Click to expand...

I came right with it. It was something to do with having a list of bookings in the Rental class and Rental object in the Booking class if you catch what I'm saying. Then in the Booking class I also had a RentalId.

I think I got myself into some type of circular spaghetti mess. I'm still learning EF and will have deeper look once I get this small assignment out and sent off.
 
DA-LION-619 said:
EF Core > 2.2 doesn’t play fast and loose with relationships, they have to be well defined.
Delete Restrict on a FK for example would indicate to EF the column is NOT NULL so it can use INNER JOINS.

The repo approach also muddles things because if the context has no tracking of the entity it will try and insert it.
Click to expand...
You are the dumb 1 for making unneeded changes and change the returns to Tasks. It makes more sense to do this as a separate task and also fix the client calls to be async. The naming convention is also relevant here. Your pr has just been rejected. Go fix your mess.
 
The tests run fine but I am not convinced that the exception hiding issue is resolved even in Api calls.
 
DA-LION-619 said:
Async doesn’t not affect the API contract because it deals with responses.
When in doubt trust in @_kabal_

Dumb. Return yourself to the bakery.

There is no convention, async will not magically make things faster, it can actually make perf slower.
It does allow you to handle more requests/s which is the actual use case here, no one has async and non-async endpoints.
Click to expand...
This is an Api which does logically mean that it will be called from a vm other than its own. This means that for an enterprise app it is logical to assume a network call from a client on another vm to the api app. Async/await must always be used when a network jump is involved. There is a clear convention as to when to use async/await. Allowing more request per time unit does make things faster. 1 Async call might be slower that its equivalent sync call, but obviously its more about how the system performs under strain. Oh and I have seen many async and sync methods for the same method, I think the SQL bulk import lib is 1 example.
 
Benedict A55h0le said:
This is an Api which does logically mean that it will be called from a vm other than its own. This means that for an enterprise app it is logical to assume a network call from a client on another vm to the api app. Async/await must always be used when a network jump is involved.
Click to expand...
I see the dough is raising, it’s logical that Web API requires a network.
 
Solarion said:
The problem is really made apprent in the BookingService when trying to book. This is where the error is being thrown.

C#: 
Booking booking = new Booking()
{
    Nights = request.NumberOfNigths,
    Start = request.StartDate.Date,
    Rental = [B]rental[/B]
};
Click to expand...
Did you retrieve the rental via EF prior to creating this booking, or did you create the rental on the fly? The former results in no EF exception, the latter EF exception
 
Are you sure the issue is not the “no tracking” in the “repo”

Side note, why is there an interface for nearly every class that isn’t a data class/model?
 
_kabal_ said:
Are you sure the issue is not the “no tracking” in the “repo”

Side note, why is there an interface for nearly every class that isn’t a data class/model?
Click to expand...

I will check that out yeah that may do it.

Him can you give an example of which interfaces? I only inject the unitofwork, repositories and service classes usually, unless I messed up. Was severe lack of sleep when I made this topic lol!
 
battletoad said:
Did you retrieve the rental via EF prior to creating this booking, or did you create the rental on the fly? The former results in no EF exception, the latter EF exception
Click to expand...

_kabal_ said:
Are you sure the issue is not the “no tracking” in the “repo”

Side note, why is there an interface for nearly every class that isn’t a data class/model?
Click to expand...

This is with all the tests passing. I had to remove the Rental reference from the Booking class to get it to work but I'm still going to spend the day going over it and figure out why it wouldn't work. I managed to create a workaround but it's not ideal though, basically added another class that pulls Rental and Booking together in one place.

github.com

GitHub - PushKlap/RentalSoftware: RentalSoftware

RentalSoftware. Contribute to PushKlap/RentalSoftware development by creating an account on GitHub.
github.com github.com
 
100% the following is the issue

Code: 
        public async Task<Rental> GetRentalById(int rentalId)
        {
            return await Context.Rentals
                    .Where(x => x.Id == rentalId).AsNoTracking()
                    .FirstOrDefaultAsync();
        }

Code: 
      var rental = await _rentalService.GetByRentalId(request.RentalId); //which basically just does _rentalRepo.GetRentalById
Booking booking = new Booking()
                {
                    Nights = request.NumberOfNigths,
                    Start = request.StartDate.Date,
                    Rental = rental
                };

                await _unitOfWork.BookingRepository.AddAsync(booking);
                await _unitOfWork.Complete();

when you you try save that booking, because the rental has "AsNoTracking", it will try to save it to the DB too, but it will use the ID value already populated in in the "rental"
 
_kabal_ said:
100% the following is the issue

Code: 
        public async Task<Rental> GetRentalById(int rentalId)
        {
            return await Context.Rentals
                    .Where(x => x.Id == rentalId).AsNoTracking()
                    .FirstOrDefaultAsync();
        }

Code: 
      var rental = await _rentalService.GetByRentalId(request.RentalId); //which basically just does _rentalRepo.GetRentalById
Booking booking = new Booking()
                {
                    Nights = request.NumberOfNigths,
                    Start = request.StartDate.Date,
                    Rental = rental
                };

                await _unitOfWork.BookingRepository.AddAsync(booking);
                await _unitOfWork.Complete();

when you you try save that booking, because the rental has "AsNoTracking", it will try to save it to the DB too, but it will use the ID value already populated in in the "rental"
Click to expand...

Alright thanks, let me make those changes. I'll also wind the project back a little to when I was getting this error. I'll commit the changes when done.
 
I've pulled this from a book by Packt Publishing.

C#: 
// GET: api/Products/5
[HttpGet("{id}")]
public Task<Product> Get(int id)
    => _productService.GetOrderAsync(id);

Service Layer (Demo purpose for book)
C#: 
public Task<Product> GetOrderAsync(int id)
{
    // Get from database and return.
    return Task.Run(() => new Product(id, "Noddles", 12.9m));
}

But then I've seen somebody do it this way
C#: 
// GET api/v1/jobs/5
[HttpGet(ApiRoutes.Jobs.GetById)]
public async Task<IActionResult> GetById(int Id)
{
    var result = await _jobService.GetByIdAsync(Id);

    if (result == null)
    {
        return NotFound(result.Message);
    }

    return Ok(result.Job);
}

And then this guy does it this way
github.com

Ecommerce.Api/src/Streetwood.API/Controllers/OrdersController.cs at master · marcinstelmach/Ecommerce.Api

Ecommerce in .net core 2.2, C#, MediaTr, CQRS, DDD, Autofac, DI, Entity Framework Core, Memory Cache, Serilog - marcinstelmach/Ecommerce.Api
github.com github.com

C#: 
[HttpGet("{id}")]
[Authorize]
[ProducesResponseType(typeof(OrderDto), (int)HttpStatusCode.OK)]
public async Task<IActionResult> GetOrderAsync(int id)
    => Ok(await bus.SendAsync(new GetOrderQueryModel(id).SetUserId(User.GetUserId()).SetUserType(User.GetUserType())));

It gets a bit confusing as to return types from an API because some guys don't use the usual returns like OK or NotFound etc and it looks as if they just let the API return a 200 for everything.

Would a safe bet be to just stick to Microsoft's standard return types?

docs.microsoft.com

Controller action return types in ASP.NET Core web API

ActionResult vs IActionResult
docs.microsoft.com
 
The best/correct way to do it is as Microsoft shows in their docs, and how anyone expects API's to work

Endpoints that return collections ALWAYS return a collection, with 0..N items, with a 200 status code, OR if there is an error, then the error is contained with an appropriate status code != 200
Endpoints that return a single item return the item + 200 OR a 404, OR if there is an error, then the error is contained with an appropriate status code != (200 || 404)
 
_kabal_ said:
The best/correct way to do it is as Microsoft shows in their docs, and how anyone expects API's to work

Endpoints that return collections ALWAYS return a collection, with 0..N items, with a 200 status code, OR if there is an error, then the error is contained with an appropriate status code != 200
Endpoints that return a single item return the item + 200 OR a 404, OR if there is an error, then the error is contained with an appropriate status code != (200 || 404)
Click to expand...

Prefect, thank you for clearing that up for me. I'll use that in future :)
 
_kabal_ said:
Endpoints that return collections ALWAYS return a collection, with 0..N items, with a 200 status code, OR if there is an error, then the error is contained with an appropriate status code != 200
Click to expand...
what about 404 if the parent id does not exist and the collection is supposed to be the parent‘s child objects?
 
Spacerat said:
what about 404 if the parent id does not exist and the collection is supposed to be the parent‘s child objects?
Click to expand...
This is actually a really interesting question, one that I am not 100% sure of the answer.

I was obviously referring to a “valid resource” - return empty array, not null or 404

In that case I don’t think 404 is correct. I think that is more a “bad request”, but I could discuss it further and fall either way.

Same question as “get orders by customer id” - give an Id not in the system, does it return, a) empty list, b) null, c) an error of some type (exception, left of an either-type, etc)?
 
You must log in or register to reply here.
Back
Top
Sign up to the MyBroadband newsletter
X