Add pagination to resources API

[1nf0rmagician]

Summary

Adding pagination to the actions in the ResourceModificationController that return sets of data. This PR may be used as an example to change all APIs in the MORYX Framework to make use of pagination in the endpoints.
The following changes are part of this PR:

• Adding a common model for pagination requests and respones within the Framework
• Adding a second controller action for the resource query with identical method and path
• Adding ActionSelectorMethodAttribute implementations to differentiate between API calls using the new pagination responses and legacy calls
• Adding an example to configure Swagger to cope with the duplicated action method
• Updating the generated API client in the Web project

Open TODOs:

• Add ObsoleteAttributes
• Add propper usage of pagination in the web project
• Add pagination to other methods in the controller that return sets of items
• Move pagination request and result objects to a common namespace for all controllers

Linked Issues

Partially resolves #703

Checklist for Submitter

• I have tested these changes locally
• I have updated documentation as needed
• I have added or updated tests as appropriate
• I have used clear and descriptive commit messages

Review

Typical tasks

• Merge request is well described
• Critical sections are documented in code
• Tests are extended
• Documentation is created / updated
• Running in test environment
• Ports to other maintained versions are created

Clean code

• All unused references are removed
• Clean code rules are respected with passion (naming, ...)
• Avoid copy and pasted code snippets

[seveneleven]

Could be Count or ItemsCount (looking at TotalCount). I'd propose something like

• Page(Number)
• Page(s)Count
• (Items)Count *of this set
• TotalCount

[1nf0rmagician]

I like PageSize as it is very telling in the context of pagination, while I think that

• Page(Number) -> to close to PageNumber which already exists
• Page(s)Count -> without the apostrophe in page's count think it is misleading
• (Items)Count *of this set -> is to long
• TotalCount -> already exists for the total number of items

[seveneleven]

Oh sorry, I messed this up... This is what my idea was::

PagedResult {
    long Page(Number); // Could be Page but I don't have a strong opinion on changing this
    long Page(s)Count; // i.e. Pages/PageCount/PagesCount
    long (Items)Count; // of this set (instead of PageSize)
    long TotalCount; // Keeping it
    [] Items; 
}

That way, everything countable could be *Count. The 'pagination' could read sth like Page x of n Pages

[seveneleven]

Shouldn't it be possible to extend a paged result with other meta data? For example errors instead of items?

[1nf0rmagician]

For errors I would prefer to commit to using the RFC7807 standard conform ProblemDetails from now on, I found a nice blog post explaining it and I would definitely prefer it to any MORYX custom error API responses 🙂

[seveneleven]

I like that! Regarding my comment: Erros might have been a bad example. I just want to make sure that there is absolutely no reason to derive from the PagedResult.

[1nf0rmagician]

Unsealing the class is a lot easier than the other way around, thats why i sealed it for now 😅

[seveneleven]

I'm not sure about numbers. I think common values are 25, 50, 100, 200 (or 250?). While the max value depends on the use case, I'd tend to go for 200 and would also use max as a default.
Min should be 1.

[1nf0rmagician]

Agreed, should be 1 and 200 (or something reasonable that also works in the tree structures we have) 👍

[seveneleven]

Why is this 10k, while the endpoints max is only 100?

[1nf0rmagician]

Good spot, wanted to circumvent implementing a propper pagination. Also that might be hard in this tree structure... Maybe we need a bigger max than 200?

[seveneleven]

Having a bigger max would mitigate the feature itself. It should prevent users to request tons of data (not only to have a responsive UI, but also reduce server load).
You could ask for multiple pages (which is not necessarily better, but should be load 'balanceble').

[1nf0rmagician]

It should prevent users to request tons of data (not only to have a responsive UI, but also reduce server load).

I think this is only an argument if big requests would cause more load on the backend side, but since for all our modules we hold the working set of items in the memory anyway this is not really the case for us, is it?

[seveneleven]

As discussed we agreed on 200 as the maximum value.

[seveneleven]

Is it necessary to require a page number? Isn't 1 is a legit default?

[1nf0rmagician]

True, could go for page size. We just need a parameter here for the minor conform change. I would drop the duplicate action (and the corresponding attributes) in the next major then 🙂

[seveneleven]

Some Ideas:

• Could we introduce attributes, that define things like for example maximum page size to bring it to the api spec/doc?
• Should we already think about sorting?

[1nf0rmagician]

Good ideas:

• Yes at least swagger respects the usual DataAnnotation attributes like Range, so we should make use of them
• Somewhat, the parameter class makes it extendable to also include sorting parameters, so I would make it two separate PRs

[seveneleven]

Could we put the query parameters to the query instead of the request body and make these GET requests?

[1nf0rmagician]

Not completely sure, but as far as I understand no, because the body is a deep object 🤔

[seveneleven]

parameters should be updated depending on the actual data, instead of just passing them back.

Ok, I see this MR is right now rather defining the API and doesn't have any further logic yet.

[1nf0rmagician]

Yes exactly, up till now I just wanted to prove that we can make it Minor conform. Nevertheless, I agree the metadata should be updated according to the actual content 🙂

[seveneleven]

After browsing the controller, I feel like there should be a root level GET (in a future version), that combines

• filtering
• sorting
• pagination