[Incomplete] CosmosPagedResults<T> NextPageToken is now base64 encoded

[coman3]

Hey there,

Firstly, I'm not entirely sure if you will want to merge this, this is more of a discussion based pull request and I'm happy to modify to your suggestions.

I found that while using the CosmosPagedResults model, the NextPageToken was a JSON formatted string meaning that the token is full of quotes and special characters, requiring character escapes and URL encoding so that it can be safely consumed from an endpoint. This causes the string to become quite long after encoding in comparison. What I am referring to is when the token is passed via a URL Query we end up with this:

INVALID REQUEST: 
https://blah.com/api/sample?token={"token":"+RID:7c9mANLy5WhIAAAAAAAAAA==#RT:1#TRC:10#RTD:jesyMDE4LTEwLTA3VDA2OjI1OjI2LjAzMzMyNzVa#FPC:AT4AAAAAAAAASwAAAAAAAAA=","range":{"min":"","max":"FF"}}

VALID REQUEST (265 Chars):
https://blah.com/api/sample?token=%7B%22token%22%3A%22%2BRID%3A7c9mANLy5WhIAAAAAAAAAA%3D%3D%23RT%3A1%23TRC%3A10%23RTD%3AjesyMDE4LTEwLTA3VDA2OjI1OjI2LjAzMzMyNzVa%23FPC%3AAT4AAAAAAAAASwAAAAAAAAA%3D%22%2C%22range%22%3A%7B%22min%22%3A%22%22%2C%22max%22%3A%22FF%22%7D%7D

when we could end up with a much more friendly url like this instead:

INVALID REQUEST: (246 Chars)
https://blah.com/api/sample?token=eyJ0b2tlbiI6IitSSUQ6N2M5bUFOTHk1V2hJQUFBQUFBQUFBQT09I1JUOjEjVFJDOjEwI1JURDpqZXN5TURFNExURXdMVEEzVkRBMk9qSTFPakkyTGpBek16TXlOelZhI0ZQQzpBVDRBQUFBQUFBQUFTd0FBQUFBQUFBQT0iLCJyYW5nZSI6eyJtaW4iOiIiLCJtYXgiOiJGRiJ9fQ==

VALID REQUEST (244 Chars)
https://blah.com/api/sample?token=eyJ0b2tlbiI6IitSSUQ6N2M5bUFOTHk1V2hJQUFBQUFBQUFBQT09I1JUOjEjVFJDOjEwI1JURDpqZXN5TURFNExURXdMVEEzVkRBMk9qSTFPakkyTGpBek16TXlOelZhI0ZQQzpBVDRBQUFBQUFBQUFTd0FBQUFBQUFBQT0iLCJyYW5nZSI6eyJtaW4iOiIiLCJtYXgiOiJGRiJ9fQ

Problems & Questions currently:

• The base64 padding will need to be trimmed and the length inferred when reading the token
  • = = bad
  • This is an easy fix: https://stackoverflow.com/a/26354677
• What if the client wants to parse the token from JSON?
  • This is still possible, how ever it will require them to decode the token, infer the length etc themselves
  • Is this something that people will ever need to do?
  • Could we potentially add a NextPageTokenRaw property?

[Elfocrash]

Hello @coman3 and thanks for submitting this!

I am not sure about this one. You see, Cosmonaut is targeting the Standard because it's agnostic of where it will be used and I feel like it should be. Your suggestion, even though sound, is assuming that everyone will be using this in a web api scenario which is not necessarily the case.

To me it makes more sense that the method stays as it is and the library user can write their own extension method to do all of that conversion at a higher level in their project.

What are your thoughts?

[coman3]

Nope that makes perfect sense, keeping the project broad is definitely the way to go!

Maybe we could instead provide a public constructor for the CosmosPagedResults? As NextPageToken is get only (and rightfully so), this forces the user to create their own version of the model to be returned. (i am having to do this in my projects currently)

[Elfocrash]

Can you show me a code example of what you mean? Ideally, Cosmonaut should be the only thing that knows how to create a CosmosPagedResults but if I can see a usecase I can add a constructor.

[coman3]

Okay so a use case would be as such: At my endpoint currently:

        [HttpGet]
        [HttpGet("{token}/{itemCount}")]
        public async Task<IActionResult> GetPagedResults(string token = null, int itemCount = 10)
        {
            var query = DbService.Images.Query();
            query = token is null ? query.WithPagination(1, itemCount) : query.WithPagination(Encoding.ASCII.GetString(Convert.FromBase64String(token)), itemCount);
            query = query.OrderBy(x => x.CreatedAt);
            var response = await query.ToPagedListAsync();
            return Ok(new PagedResponseModel<ServerImage>(response));
        }

you will see i have to return a PagedResponseModel instead which looks like so:

 public class PagedResponseModel<TItem>
    {
        public List<TItem> Results { get; set; }
        public bool HasNextPage { get; set; }
        public string NextPageToken { get; set; }

        public PagedResponseModel(CosmosPagedResults<TItem> pageResults)
        {
            Results = pageResults.Results;
            HasNextPage = pageResults.HasNextPage;
            NextPageToken = pageResults.NextPageToken;
            if (HasNextPage && !(pageResults.NextPageToken is null))
            {
                NextPageToken = Convert.ToBase64String(Encoding.ASCII.GetBytes(pageResults.NextPageToken));
            }
        }
    }

(a direct copy of the CosmosPagedResults, just with base 64 encoding (which can easily be done within the endpoint))

To be clear, (and as we can see) its nothing pressing at all! Especially because you want to keep the token as is. I just think that for api based usage, not having an extra model and being able to use an already fit model, in the cluster of models that already appears in a larger scale web app, can be useful 😄

[Elfocrash]

query = token is null ? query.WithPagination(1, itemCount) : query.WithPagination(Encoding.ASCII.GetString(Convert.FromBase64String(token)), itemCount);

Just a note on this line. Passing an null or string empty token always returns the first page by default.

You found a bug where null will actually ignore the item count.

Thanks for that!