microsoftgraph / msgraph-sdk-ruby

Microsoft Graph Ruby client library for v1 APIs
https://graph.microsoft.com
MIT License
99 stars 68 forks source link

Ruby delta request builder does not implement deltaToken & skipToken params #143

Closed j15e closed 11 months ago

j15e commented 11 months ago

From my understanding of the Delta API and other languages implementations, I think classes for building delta request parameters (ex. MicrosoftGraph::Groups::Delta::DeltaRequestBuilder::DeltaRequestBuilderGetQueryParameters) should support a skipToken and a deltaToken to make subsequent requests.

Are you aware of an issue in Kiota that could relate to this? Or am I not following the correct solution? I could not find much references of skipToken/skip_token in the Ruby lib.

I would expect I would be able for example to do the following two requests (given there is a nextToken):

group_id = "cd6e0472-971c-402e-8040-dbbf53652e35"
group_params = MicrosoftGraph::Groups::Delta::DeltaRequestBuilder::DeltaRequestBuilderGetQueryParameters.new
group_params.select = ["id", "displayName", "members"]
group_params.filter = "id eq '#{group_id}'"
request_configuration = MicrosoftKiotaAbstractions::RequestConfiguration.new
request_configuration.query_parameters = group_params

resp = graph_client.groups.delta.get(request_configuration).resume

# Do something with the first delta set
# ... 
# Extract skipToken
skip_token = CGI.parse(URI(resp.odata_next_link).query)["$skiptoken"]

# Generate new request with skip token
group_params = MicrosoftGraph::Groups::Delta::DeltaRequestBuilder::DeltaRequestBuilderGetQueryParameters.new
# Doesn't exist! Only skip, but this is not the expected parameter.
group_params.skip_token = skip_token 
request_configuration = MicrosoftKiotaAbstractions::RequestConfiguration.new
request_configuration.query_parameters = group_params
graph_client.groups.delta.get(request_configuration).resume
baywet commented 11 months ago

Thanks for reaching out and for the interest in the Ruby SDK! delta and skip token query parameters are not exposed because they are not supposed to be used by themselves. Whenever a response comes back with a next or delta link, we expect the client to treat it as opaque and issue the request with that link directly, not to try to parse it.

To enable this, we have a pending work item for ruby in kiota so clients can pass the raw URL directly https://github.com/microsoft/kiota/issues/2206

j15e commented 11 months ago

Ah thanks for pointing that issue, that is indeed the limitation I am facing and would love to see resolved.

Making the request not using the SDK is indeed an hassle because we loose all the request phase (auth) & response phase (object parsing) heavylifting done by the SDK.

Maybe I could hack my way around and find away to pass the request-raw-url down to the request being built.

I plan to use a lot of the delta API so I'll sure find a way 😆

baywet commented 11 months ago

The workaround you could use for now would be.

  1. Instantiate the request builder with dummy parameters
  2. Call the ToGetRequestInformation to get the request information
  3. set the URI on the request information
  4. Copy some of the code in the get method of the request builder so you can pass your request information

No super pretty but it should get the job done. Of course if you prefer instead PR to kiota to implement that gap. We'll be more than happy to review and merge that PR, and later on release a new version of the SDK here.

j15e commented 11 months ago

I found a hack around (writing to a private instance variable), you can set the request-raw-url in the Ruby client this way:

graph_client.instance_variable_set(:@path_parameters, { "request-raw-url" => resp.odata_next_link })
# This next request will be make to the resp.odata_next_link
graph_client.groups.delta.get.resume
# Reset to default after, otherwise all requests will be made to this raw url 
graph_client.instance_variable_set(:@path_parameters, {})

But that is not ideal, I'll check how this could be properly implemented now that I better understand the flow of the request parameters.

j15e commented 11 months ago

Could you give us a pseudo-code example of how you would like this to work in the Ruby SDK?

Would it be like this?

# Initial request
group_id = "cd6e0472-971c-402e-8040-dbbf53652e35"
group_params = MicrosoftGraph::Groups::Delta::DeltaRequestBuilder::DeltaRequestBuilderGetQueryParameters.new
group_params.select = ["id", "displayName", "members"]
group_params.filter = "id eq '#{group_id}'"
request_configuration = MicrosoftKiotaAbstractions::RequestConfiguration.new
request_configuration.query_parameters = group_params

resp = graph_client.groups.delta.get(request_configuration).resume

# Pass the next URL as a string and the SDK pass it as the `request-raw-url` on the RequestInformation?
resp = graph_client.groups.delta.get(resp.odata_next_link).resume

So the get methods (and only on get methods) on request builders generated by Kioata would have to be updated to something like this:

module MicrosoftGraph
    module Groups
        ## 
        # Provides operations to manage the collection of group entities.
        class GroupsRequestBuilder < MicrosoftKiotaAbstractions::BaseRequestBuilder
        # ...
        def get(request_configuration_or_raw_url=nil)
          # Updated here to support raw url as the argument
          if request_configuration.is_a?(String)
            request_info = self.to_get_request_information
            request_info.uri = request_configuration_or_raw_url
          else
            request_info = self.to_get_request_information(request_configuration_or_raw_url)
          end
          error_mapping = Hash.new
          error_mapping["4XX"] = lambda {|pn| MicrosoftGraph::Models::ODataErrorsODataError.create_from_discriminator_value(pn) }
          error_mapping["5XX"] = lambda {|pn| MicrosoftGraph::Models::ODataErrorsODataError.create_from_discriminator_value(pn) }
          return @request_adapter.send_async(request_info, lambda {|pn| MicrosoftGraph::Models::GroupCollectionResponse.create_from_discriminator_value(pn) }, error_mapping)
      end
baywet commented 11 months ago

Actually giving a look at the current state of things, it should already work.

https://github.com/microsoft/kiota-abstractions-ruby/blob/392254e920a4a912ed8f9c465525b061e0c19172/lib/microsoft_kiota_abstractions/base_request_builder.rb#L29

https://github.com/microsoft/kiota-abstractions-ruby/blob/392254e920a4a912ed8f9c465525b061e0c19172/lib/microsoft_kiota_abstractions/request_information.rb#L34

So you should be able to do something like

request_builder = MicrosoftGraph::Groups::Delta::DeltaRequestBuilder::DeltaRequestBuilder.new("next/delta link", requestAdapter, "bar")
resp = request_builder.get.resume

Let me know if this works, I'll close the kiota issue this way.

j15e commented 11 months ago

I confirm I can get my way around with:

# response1 being the first delta request response
request_builder = 
  MicrosoftGraph::Groups::Delta::DeltaRequestBuilder.new(response1.odata_next_link, graph_request_adapter)
# It does a query to resp.odata_next_link and it parses the response correctly
resp = request_builder.get.resume

There are a lof of layers of abstractions going on with Kiota so it was kind of hard to just think of this solution (which isn't that complex in the end). I get a better understanding of the internals nows, so it should be easier next time I face an issue.

It is a bit an hassle that we can't do it via the graph client instance and that we have to keep a reference to the adapter (it is set on the graph client, but private). I think the Ruby client could still be improved to better handler this without the need for building a custom request and the need to re-use the request adapter everywhere you need to page.

But it works, thanks, it will do the job for now👌

baywet commented 11 months ago

Thanks I'm going to go ahead and close this issue and the one in Kiota. Also, in "finished" SDKs, the client has a property that gives access to the request adapter so you don't need to carry both references around. This is something that still needs to be added in Ruby at this time.