Skip to content

Provide argument resolver for KeysetScrollPosition #3148

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

Closed
vpavic opened this issue Sep 6, 2024 · 7 comments
Closed

Provide argument resolver for KeysetScrollPosition #3148

vpavic opened this issue Sep 6, 2024 · 7 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@vpavic
Copy link
Contributor

vpavic commented Sep 6, 2024

In order to make KeysetScrollPosition easier to use for developers, especially having in mind its non-trivial nature (in contrast to OffsetScrollPosition), Spring Data Commons should provide the appropriate argument resolver.

Typically something like KeysetScrollPosition would be serialized to JSON and then base64 encoded.

@vpavic vpavic changed the title Provide argument resolver for KeysetScrollPosition Provide argument resolver for KeysetScrollPosition Sep 6, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 6, 2024
@mp911de
Copy link
Member

mp911de commented Sep 9, 2024

KeysetScrollPosition is a combination of key-value pairs using the correct data type. Anything that isn't String already starts losing typing information. Any number could be int or long, a floating point number double or float. Dates are typically rendered into numbers. With reading data into a Map, we require much more type information and so an encoded variant of KeysetScrollPosition ends up being a pretty long string. Also, we do not have a natural way to represent KeysetScrollPosition. FWIW, Spring GraphQL has some utilities to encode keyset scroll positions.

@mp911de mp911de added the status: waiting-for-feedback We need additional information before we can continue label Sep 9, 2024
@quaff
Copy link
Contributor

quaff commented Sep 10, 2024

My proposal: #2856 (comment)

The key problem is how to convert keys to correct type, maybe we should introduce an annotation to extract actual type from underlying domain object, for example

@TypeHints(from = User.class) KeysetScrollPosition position

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 10, 2024
@mp911de
Copy link
Member

mp911de commented Sep 10, 2024

Generally speaking, I would like to see some folks implementing serialization and deserialization for these types. As we're working with message structures that could potentially reveal application internals, integrity and encryption are likely to be part of such a story. For the time being, we do not want to include such a change.

We might come back at a later time to revisit this topic.

@mp911de mp911de closed this as not planned Won't fix, can't repro, duplicate, stale Sep 10, 2024
@mp911de mp911de added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Sep 10, 2024
@vpavic
Copy link
Contributor Author

vpavic commented Sep 10, 2024

I don't understand the reasoning to close this issue, as some support around KeysetScrollPosition is clearly needed in order to make the feature more approachable to developers, similarly to what is provided for other building blocks provided by Spring Data Commons and especially those that get exposed over the web.

It's understandable that you might not want (or can) address it immediately, but that shouldn't prevent this from at least being in the backlog. FWIW I had planned to share a sample of what we're doing at the moment.

@quaff
Copy link
Contributor

quaff commented Sep 11, 2024

As we're working with message structures that could potentially reveal application internals

Could you elaborate more? as far as I know, key names is not sensitive information.

integrity and encryption are likely to be part of such a story.

I think it's not restful, we should allow client construct query by specifying parameters.

@mp911de
Copy link
Member

mp911de commented Sep 11, 2024

We can always reopen closed tickets. It would be great to have a bit of sample code to get a different perspective.

@mp911de
Copy link
Member

mp911de commented Sep 11, 2024

Could you elaborate more? as far as I know, key names is not sensitive information.

Indeed, key (property) names are not sensitive. However, the key values might be sensitive and not exposed. We require keys and values to reconstruct the query.

I think it's not restful, we should allow client construct query by specifying parameters.

No one said it has to be restful. Think about ScrollPosition as cursor that is generated on the server side. A good comparison is Cassandra's PagingState that looks to users like a bunch of random bytes to continue a scrolling session. As external client, you cannot expect applications to present a scroll position that the application has never generated in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

4 participants