Problem/Motivation

The code that converts the YAML query definition into an actual Graph QL query has limitations. Arguments for example can only be defined on the top level field. It would be nice if we can define more complex queries. An example of a structure that we need that does not work right now is

  query:
    search:
      listings:
        arguments:
          locale: nl_BE
          metadata:
            size: 400
          publication:
            changedDateFrom: '2023-08-12T00:00:00.000Z'
        fields:
          - metadata:
            - currentPage
            - totalPages
          - listings:
            - id
            - details:
              - vehicle:
                - classification:
                  - make:
                    - formatted
                  - model:
                    - formatted
                  - modelVersionInput
                - numberOfDoors

Steps to reproduce

Might not be that easy as you need an GraphQL API that has a complex structure. :-)

Proposed resolution

Not sure yet. We could introduce a structured that would translated into a nested structure with associative arrays. But it would also be nice if we can keep the YAML structure easy an straight forward for APIs that are not that complex.

Remaining tasks

- Design solution
- Create MR
- Review MR
- Merge MR

User interface changes

N/A

API changes

Changes in YAML structure to use for modelling the query in the migration definition.

Data model changes

N/A

Issue fork migrate_source_graphql-3381675

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

ricovandevin created an issue. See original summary.

reinchek’s picture

reinchek
Location Napoli 🌋 🇵🇸
commented

Hi @ricovandevin,
the first thing (maybe simple) that comes to my mind is to embed some recursive logic to \Plugin\migrate\source\GraphQL::getGenerator() buildQuery() instead to call directly $this->buildQuery() we could call $this->buildQueryRecursive (but it would works only for first "arguments" match) that will builds the query recursively.
Somethings like this:

  private function buildQueryRecursive(string $queryName, array $query, ?Query &$qlQuery = null) {
    $arguments = NULL;
    $keys = array_keys($query);

    if (in_array('fields', $keys)) {
      // $queryName has "fields"
      $fields = $query['fields'];
      if (in_array('arguments', $keys)) {
        // $queryName has "arguments"
        $arguments = $query['arguments'];
      }

      $subQlQuery = $this->buildQuery($queryName, $fields, $arguments);
      $qlQuery->setSelectionSet([$subQlQuery]);
    } else {
      if (is_null($qlQuery)) {
        $qlQuery = new Query($queryName);
        $subQlQuery = $qlQuery;
      } else {
        $subQlQuery = (new Query($queryName));
        $qlQuery->setSelectionSet([$subQlQuery]);
      }

      return $this->buildQueryRecursive(array_key_first($query), reset($query), $subQlQuery);
    }
  }

So, using this code, i tried to launch your migration (just for obtains compiled query) and the query generated was as follows:

query {
  search {
    listings(
      locale: nl_BE
      metadata: { size: 400 }
      publication: { changedDateFrom: "2023-08-12T00:00:00.000Z" }
    ) {
      metadata {
        currentPage
        totalPages
      }
      listings {
        id
        details {
          vehicle {
            classification {
              make {
                formatted
              }
              model {
                formatted
              }
              modelVersionInput
            }
            numberOfDoors
          }
        }
      }
    }
  }
}

Maybe this could be the way, what do you think?

reinchek’s picture

reinchek
Location Napoli 🌋 🇵🇸
commented

Edit:
the above code breaks simple query parsing like as the following one:

  query:
    posts:
      fields:
        -
          data:
            - id
            - title
            - body

This code, instead, will work with anyway:

  private function buildQueryRecursive(string $queryName, array $query, ?Query &$qlQuery = null) {
    $arguments = NULL;
    $keys = array_keys($query);

    if (in_array('fields', $keys)) {
      // $queryName has "fields"
      $fields = $query['fields'];
      if (in_array('arguments', $keys)) {
        // $queryName has "arguments"
        $arguments = $query['arguments'];
      }

      $subQlQuery = $this->buildQuery($queryName, $fields, $arguments);
      // If $qlQuery is null means that we found 'fields' || 'arguments' at the first recursive operation...
      if (is_null($qlQuery)) {
        $qlQuery = $subQlQuery;
      } else {
        $qlQuery->setSelectionSet([$subQlQuery]);
      }
    } else {
      if (is_null($qlQuery)) {
        $qlQuery = new Query($queryName);
        $subQlQuery = $qlQuery;
      } else {
        $subQlQuery = (new Query($queryName));
        $qlQuery->setSelectionSet([$subQlQuery]);
      }

      return $this->buildQueryRecursive(array_key_first($query), reset($query), $subQlQuery);
    }
  }
ricovandevin’s picture

ricovandevin commented

@reinchek Thanks for looking into this so quickly. I have also been looking into this and came up with a solution that involves defining a "path" from the query name to the level holding the actual field list. That was after trying to get a recursive solution working. Good to see that you did. I have tried the compiled query you have posted in #2 using the API suppliers testing tool and it works great.

Other issues I ran into while working on this where the fields() method which expects the field list to be on a "higher level" in the query and the actual fetching of the result rows in the getGenerator() method. Both were solved using the "path" configuration mentioned in the previous paragraph.

As a next step I will try to integrate your recursive query compilation code with solutions to this issues in the fields() and getGenerator() methods. Although I'm not sure whether we really need fields()? I will check some other source plugins to see how they implement the method.

ricovandevin opened merge request !7

ricovandevin’s picture

ricovandevin commented
Version: 2.0.5 » 2.0.6
Issue summary: View changes
Status: Active » Needs review

The recursive code works great for my original example query.

  query:
    search:
      listings:
        arguments:
          locale: nl_BE
          metadata:
            size: 400
          publication:
            changedDateFrom: '2023-08-12T00:00:00.000Z'
        fields:
          - listings:
            - id
            - details:
              - vehicle:
                - classification:
                  - make:
                    - formatted
                  - model:
                    - formatted
                  - modelVersionInput
                - numberOfDoors

(I have removed the metadata part under fields as it turned out we do not need it.)

I have put the code in a MR and added another change in \Drupal\migrate_source_graphql\Plugin\migrate\source\GraphQL::getGenerator() to let the generator find the actual result rows in a nested structure. I have done this by allowing data_key to contain a path to the results using the property separator as defined in \Drupal\migrate\Row::PROPERTY_SEPARATOR (which is currently a forward slash). For the example above this would be:

  ...
  auth_parameter: '***'
  data_key: listings/listings
  query:
    search:
  ...

With these changes a can do a successful migration. I have left fields() out-of-scope for now as it doesn't seem to do much. At least it does not prevent the migration from working.

I'm curious to whether the solution works for other more complex queries too. It would be nice to get this in the module

reinchek’s picture

reinchek
Location Napoli 🌋 🇵🇸
commented

@ricovandevin thanks you too.

Sorry because i avoided to paste other code changes (to adapt, correctly the buildQueryRecursive method) but the intent was just for show you a possible (i tried as you done and can say...) working solution.

About the "fields" property (as i wroted in "The migration" paragraph of the module's "documentation"):

The only difference between the GraphQL query and the YAML transposition is the mandatory property fields, the usefulness of which is solely a matter of choice for the developer.

So, we could do some semantics changes to the code to remove the priority "fields" property role. But, yes, i'm agreed with you, for now, let it out.

I also agree with data_key solution. 👌

I've tested and then merged the PR. I've also added a new doc section (you can read in module's home "From 2.0.7 version (issue-3381675)").

Thanks again ;)

reinchek’s picture

reinchek
Location Napoli 🌋 🇵🇸
commented
Status: Needs review » Fixed
reinchek’s picture

reinchek
Location Napoli 🌋 🇵🇸
commented
Status: Fixed » Closed (fixed)
reinchek’s picture

reinchek
Location Napoli 🌋 🇵🇸
commented

New features inside v. 2.0.7.