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

Boiler plate when iterating through collections #1965

Open
Moh-inc opened this issue Apr 25, 2024 · 4 comments
Open

Boiler plate when iterating through collections #1965

Moh-inc opened this issue Apr 25, 2024 · 4 comments
Labels
type:enhancement Enhancement request targeting an existing experience

Comments

@Moh-inc
Copy link

Moh-inc commented Apr 25, 2024

Hello, i have a small question.

https://github.com/microsoftgraph/msgraph-sdk-java/blob/dev/docs/upgrade-to-v6.md#pageiterator

  • With the new page iterator there is a boilerplate associated with each request and iterating through it. Since a new page iterator would need to be created for each request.
  • I was wondering if there is a generic way to create an iterator to run on collection requests and delta collection requests.

So I created this code, from looking at the generated code in the msft graph sdk:

  public static <T extends Parsable> void genericIterator(
      GraphServiceClient graphClient,
      BaseDeltaFunctionResponse res,
      Consumer<T> processEntity, 
      Consumer<String> processDeltaLink) {
    PageIterator<T, BaseDeltaFunctionResponse> pageIterator;
    try {
      pageIterator = new PageIterator.Builder<T, BaseDeltaFunctionResponse>()
              .client(graphClient)
              .collectionPage(res)
              .collectionPageFactory(BaseDeltaFunctionResponse::createFromDiscriminatorValue)
              .processPageItemCallback(entity -> {
                processEntity.accept(entity);
                return true; 
              }).build();
      pageIterator.iterate();
      processDeltaLink.accept(pageIterator.getDeltaLink());
    } catch (ApiException | ReflectiveOperationException e) {
      log.error("Error ", e);
    }
  }

  public static <T extends Parsable> void genericIterator(
      GraphServiceClient graphClient,
      BaseCollectionPaginationCountResponse res,
      Consumer<T> processGroupOwner) {
    PageIterator<T, BaseCollectionPaginationCountResponse> pageIterator;
    try {
      pageIterator = new PageIterator.Builder<T, BaseCollectionPaginationCountResponse>()
              .client(graphClient)
              .collectionPage(res)
              .collectionPageFactory(BaseCollectionPaginationCountResponse::createFromDiscriminatorValue)
              .processPageItemCallback(entity -> {
                processGroupOwner.accept(entity);
                return true; 
              }).build();
      pageIterator.iterate();
    } catch (ApiException | ReflectiveOperationException e) {
      log.error("Error ", e);
    }
  }

This second method works with general collectors, but not the first one for delta collections.

I was wondering if this is even recommending considering that the sdk is based on code generations and/or if there is a way to iterate over collections in a more concise manner than the current page iterator.

Error received from the generic delta collection iterator:

DeltaGetResponse res2 = graphServiceClient.groups().delta().get();
genericIterator(graphServiceClient, res2, (Group group) -> {
  log.info("Group: {} {}", group.getId(), group.getDisplayName());
}, (String deltaLink) -> {
  log.info("DeltaLink: {}", deltaLink);
});
java.lang.IllegalAccessException: NO_COLLECTION_PROPERTY_ERROR
        at com.microsoft.graph.core.tasks.PageIterator.extractEntityListFromParsable(PageIterator.java:300)
        at com.microsoft.graph.core.tasks.PageIterator.interpageIterate(PageIterator.java:250)
        at com.microsoft.graph.core.tasks.PageIterator.iterate(PageIterator.java:272)
@Ndiritu
Copy link
Contributor

Ndiritu commented Apr 29, 2024

@Moh-inc thank you for your feedback and proposed improvements. These are really helpful to the future of this SDK.

The exception is thrown because the HTTP response returned after hitting the nextLink URL contains a "value" property but the collectionPageFactory set to BaseDeltaFunctionResponse does not handle deserialization of the value property.

It would be ideal if the collectionPageFactory is a parameter of the methods so that a derived DeltaFunctionResponse type can be passed and the value property is correctly deserialized e.g. a Group
DeltaGetResponse

The same issue is likely to affect your implementation for the generic collections if a next page is returned.
The collectionPageFactory guides the deserializer on how to deserialze the value property.

@Ndiritu
Copy link
Contributor

Ndiritu commented Apr 29, 2024

I do however like the idea of adding a convenience method to reduce the boilerplate to instantiate a PageIterator. This is worth exploring further.

@Ndiritu Ndiritu added the type:enhancement Enhancement request targeting an existing experience label Apr 29, 2024
@etcoyvindf
Copy link

Hello @Moh-inc , I just wanted to chime in here, since we are using something similiar in our project.

We use the following method to iterate and put elements into a list. Please Excuse the random ordering of the parameters. Hopefully this can be useful for you. We are able to use this method for both normal Collection requests and Delta Collection requests.

public static <TEntity extends Parsable, TCollectionPage extends Parsable & AdditionalDataHolder> String getList(
            @Nonnull List<TEntity> theList,
            @Nonnull GraphServiceClient gc,
            TCollectionPage pageResponse,
            @Nonnull ParsableFactory<TCollectionPage> collectionPageFactory
    )
            throws ReflectiveOperationException {
        if(pageResponse== null) {
            return null;
        }
        var pageIterator = new PageIterator.Builder<TEntity, TCollectionPage>()
                .client(gc)
                .collectionPage(pageResponse)
                .collectionPageFactory(collectionPageFactory)
                .processPageItemCallback((item) -> {
                    theList.add(item);

                    return true;
                }).build();
        pageIterator.iterate();
        return pageIterator.getDeltaLink();

Example usage:

String deltaString = "... the delta string from last time";
GraphServiceClient graphClient = ... //get your client
List<Group> groupList= new ArrayList<>();
DeltaGetResponse groups = graphClient.groups().delta().withUrl(deltaString).get();
String groupDeltaLink = GraphUtil.getList(groupList, graphClient, groups, DeltaGetResponse::createFromDiscriminatorValue);

@Moh-inc
Copy link
Author

Moh-inc commented Apr 29, 2024

@etcoyvindf Looks good thank you for sharing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Enhancement request targeting an existing experience
Projects
None yet
Development

No branches or pull requests

3 participants