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

Update getting-started.md #296

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

michelcedric
Copy link

@michelcedric michelcedric commented May 22, 2023

Adapt get method by key

An odata endpoint must return a IQueryable.
If you evaluate it directly in the return of the controller method, the odata query feature (example : expand) can't working.
If you directly return the queryable you return an array with only one element.
You can fix that with the usage of
SingleResult.Create method form odata

Adapt get method by key
@learn-build-service-prod
Copy link
Contributor

Learn Build status updates of commit d8c75bd:

✅ Validation status: passed

File Status Preview URL Details
Odata-docs/webapi-8/getting-started.md ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@learn-build-service-prod
Copy link
Contributor

Learn Build status updates of commit 4933754:

✅ Validation status: passed

File Status Preview URL Details
Odata-docs/webapi-8/getting-started.md ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@learn-build-service-prod
Copy link
Contributor

Learn Build status updates of commit 3602f07:

✅ Validation status: passed

File Status Preview URL Details
Odata-docs/webapi-8/getting-started.md ✅Succeeded View
Odata-docs/webapi-8/tutorials/basic-crud.md ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@michelcedric
Copy link
Author

@microsoft-github-policy-service agree

@learn-build-service-prod
Copy link
Contributor

Learn Build status updates of commit e9d9d88:

✅ Validation status: passed

File Status Preview URL Details
Odata-docs/webapi-8/getting-started.md ✅Succeeded View
Odata-docs/webapi-8/tutorials/basic-crud.md ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@@ -148,16 +148,10 @@ namespace Lab01.Controllers
}

[EnableQuery]
public ActionResult<Customer> Get([FromRoute] int key)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason for this change. $expand works okay even when the return type is ActionResult<Customer>.

@@ -376,20 +376,14 @@ The following JSON payload shows the expected response:
## Request a single entity
To support this request, we add a controller action named `Get` (or `GetCustomer`) to the `CustomersController` class. The action should accept a single parameter named `key` of type `int` - same type as the entity's key property:
```csharp
public ActionResult<Customer> Get([FromRoute] int key)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason for this change. $expand works okay even when the return type is ActionResult<Customer>.


return Ok(customer);
var item = customers.AsQueryable().Where(c => c.Id == id);
return SingleResult.Create(item);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason for this change. $expand works okay even when the return type is ActionResult.

@gathogojr
Copy link
Collaborator

@michelcedric There's no reason for the changes that you proposed. Query options work okay even when the return type is ActionResult or ActionResult<T>. If you have reasons to believe that they don't, please report the issue together with a simple repro on https://github.com/OData/AspNetCoreOData/issues.

@michelcedric
Copy link
Author

@gathogojr
I'm sorry, I will give more details.
But If you use a real Database as Sql Server with EF.
When you execute.
var customer = db.Customers.SingleOrDefault(d => d.Id == key);
The query is immediately evaluated and executed on the database.
Customer object is loaded in memory and in this case expand is not possible.
It's that why the return of the controller must be a queryable to delay to Odata the execution of the query.
For an id the return is not a queryable but only the entity. It's seems the reasons why SingleResult of Odata exists.
var item = customers.AsQueryable().Where(c => c.Id == id); return SingleResult.Create(item);
I created a repository to exactly show what I mean
https://github.com/michelcedric/OdataInclude/blob/main/OdataInclude/Controllers/BooksController.cs
https://github.com/michelcedric/OdataInclude/blob/main/OdataInclude/OdataInclude.http
image
1 Get books to retrieve the first ID
2 Get book with the ID and expand MainAuthor=> MainAuthor null
3 Get book with the ID and expand MainAuthor version 2 => MainAuthor filled
4 Get book with bad ID and expand MainAuthor=> Not found 404.

@gathogojr
Copy link
Collaborator

@gathogojr I'm sorry, I will give more details. But If you use a real Database as Sql Server with EF. When you execute. var customer = db.Customers.SingleOrDefault(d => d.Id == key); The query is immediately evaluated and executed on the database. Customer object is loaded in memory and in this case expand is not possible. It's that why the return of the controller must be a queryable to delay to Odata the execution of the query. For an id the return is not a queryable but only the entity. It's seems the reasons why SingleResult of Odata exists. var item = customers.AsQueryable().Where(c => c.Id == id); return SingleResult.Create(item); I created a repository to exactly show what I mean https://github.com/michelcedric/OdataInclude/blob/main/OdataInclude/Controllers/BooksController.cs https://github.com/michelcedric/OdataInclude/blob/main/OdataInclude/OdataInclude.http image 1 Get books to retrieve the first ID 2 Get book with the ID and expand MainAuthor=> MainAuthor null 3 Get book with the ID and expand MainAuthor version 2 => MainAuthor filled 4 Get book with bad ID and expand MainAuthor=> Not found 404.

@michelcedric The sample code in this tutorial uses an in-memory collection as a data store.
There's therefore no value in returning a SingleResult in this case.
There's also no value in casting the collection to AsQueryable.

In my opinion, it'd be more ideal to have a different article with code samples that are relevant to an SQL Server data store where we can employ the use of SingleResult.Create to construct and execute the query for the expanded entity at the database level...

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

Successfully merging this pull request may close these issues.

2 participants