How to correctly handle multiple deferred / promise?

[mfn]

In a resolver I need to access:

• $model->relation1 => so I use a dataloader
• however after relation1 is loaded, I need another relation on that one
• $relation1->relation2 => again via dataloader

There is incentive using the dataloader also for relation2 because this may be required in other resolvers too (without relation1)

I came up with a resolver looking like this:

'resolve' => function (Model $model, $_) {
    $dataloader = $this->getDataLoaderFor('relation1');
    $dataloader->batch($model->relation1_id);

    $deferred = new Deferred(function () use ($model, $dataloader) {
        return $dataloader->load($model->relation1_id);
    });

    $syncPromise->then(function (Relation1 $relation1) use () {
        $dataloader = $this->getDataLoaderFor('relation2');
        $dataloader->batch($relation1->relation2_id);

        return new Deferred(function () use ($relation1, $dataloader) {
            return $dataloader->load($relation1->relation2_id);
        });
    });

    return new Deferred(function () use ($syncPromise) {
        return $syncPromise;
    });
},

I know closures are verbose in PHP already 😄 however I realized when I need dependent deferred using ->then(), I need to wrap it in yet another Deferred because ->then() returns a SyncPromise which is not handled by webonxy.

Is this "amount of code" expected? Is the approach correct / good? Can I simplify the above?

Note: it's not about reducing LoC, I could do that sparing temporary variables. It's more about the approach, because currently it's:

• Deferred
• ->then() returning a SyncPromise
• contains a closure which returns a Deferred
• to return the SyncPromise I need to wrap it in a Deferred

I guess I would have expected I could return $deferred->then(…) directly without wrapping it again.

Thanks!

[vladar]

Have you tried this:

'resolve' => function (Model $model, $_) {
    $dataloader = $this->getDataLoaderFor('relation1');
    $dataloader->batch($model->relation1_id);

    return new Deferred(function () use ($model, $dataloader) {
        $relation1 = $dataloader->load($model->relation1_id);
        $dataloader2 = $this->getDataLoaderFor('relation2');
        $dataloader2->batch($relation1->relation2_id);

        return new Deferred(function () use ($relation1, $dataloader2) {
            return $dataloader2->load($relation1->relation2_id);
        });
    });
},

I've added a test for this scenario and it seems to be working as expected.

Resolver:

graphql-php/tests/Executor/DeferredFieldsTest.php

Lines 170 to 187 in 46a99a3

	'topStoryAuthor' => [
	'type' => $this->userType,
	'resolve' => function ($category, $args, $context, ResolveInfo $info) {
	$this->paths[] = $info->path;
	return new Deferred(function () use ($category) {
	$this->paths[] = 'deferred-for-category-' . $category['id'] . '-topStoryAuthor1';
	$story = $this->findStoryById($category['topStoryId']);
	return new Deferred(function () use ($category, $story) {
	$this->paths[] = 'deferred-for-category-' . $category['id'] . '-topStoryAuthor2';
	return $this->findUserById($story['authorId']);
	});
	});
	},
	],
	],

Test:

graphql-php/tests/Executor/DeferredFieldsTest.php

Lines 556 to 637 in 46a99a3

	public function testDeferredChaining()
	{
	$schema = new Schema([
	'query' => $this->queryType,
	]);
	$query = Parser::parse('
	{
	categories {
	name
	topStory {
	title
	author {
	name
	}
	}
	topStoryAuthor {
	name
	}
	}
	}
	');
	$author1 = ['name' => 'John'/*, 'bestFriend' => ['name' => 'Dirk']*/];
	$author2 = ['name' => 'Jane'/*, 'bestFriend' => ['name' => 'Joe']*/];
	$author3 = ['name' => 'Joe'/*, 'bestFriend' => ['name' => 'Jane']*/];
	$author4 = ['name' => 'Dirk'/*, 'bestFriend' => ['name' => 'John']*/];
	$story1 = ['title' => 'Story #8', 'author' => $author1];
	$story2 = ['title' => 'Story #3', 'author' => $author3];
	$story3 = ['title' => 'Story #9', 'author' => $author2];
	$result = Executor::execute($schema, $query);
	$expected = [
	'data' => [
	'categories' => [
	['name' => 'Category #1', 'topStory' => $story1, 'topStoryAuthor' => $author1],
	['name' => 'Category #2', 'topStory' => $story2, 'topStoryAuthor' => $author3],
	['name' => 'Category #3', 'topStory' => $story3, 'topStoryAuthor' => $author2],
	],
	],
	];
	self::assertEquals($expected, $result->toArray());
	$expectedPaths = [
	['categories'],
	['categories', 0, 'name'],
	['categories', 0, 'topStory'],
	['categories', 0, 'topStoryAuthor'],
	['categories', 1, 'name'],
	['categories', 1, 'topStory'],
	['categories', 1, 'topStoryAuthor'],
	['categories', 2, 'name'],
	['categories', 2, 'topStory'],
	['categories', 2, 'topStoryAuthor'],
	'deferred-for-category-1-topStory',
	'deferred-for-category-1-topStoryAuthor1',
	'deferred-for-category-2-topStory',
	'deferred-for-category-2-topStoryAuthor1',
	'deferred-for-category-3-topStory',
	'deferred-for-category-3-topStoryAuthor1',
	'deferred-for-category-1-topStoryAuthor2',
	'deferred-for-category-2-topStoryAuthor2',
	'deferred-for-category-3-topStoryAuthor2',
	['categories', 0, 'topStory', 'title'],
	['categories', 0, 'topStory', 'author'],
	['categories', 1, 'topStory', 'title'],
	['categories', 1, 'topStory', 'author'],
	['categories', 2, 'topStory', 'title'],
	['categories', 2, 'topStory', 'author'],
	['categories', 0, 'topStoryAuthor', 'name'],
	['categories', 1, 'topStoryAuthor', 'name'],
	['categories', 2, 'topStoryAuthor', 'name'],
	'deferred-for-story-8-author',
	'deferred-for-story-3-author',
	'deferred-for-story-9-author',
	['categories', 0, 'topStory', 'author', 'name'],
	['categories', 1, 'topStory', 'author', 'name'],
	['categories', 2, 'topStory', 'author', 'name'],
	];
	self::assertEquals($expectedPaths, $this->paths);
	}

[mfn]

🤔

Well, bummer on my side. I thought I created an example where I would need 3 nested deferrds. But I failed, because I realized as I saw your example, yes, I can correctly reduce it to 2 levels. Ok #fail 😄

But…

… I'll try once more to distill my actual question better 🤞

The way I understand it: every dataloader depending on the outcome of another dataloader, needs to be wrapped in a deferred.

My initial example has 2 dataloaders, so correctly you nested it twice.

So when I've n dataloaders, the code will be come:

• dataloader 1
• deferred
  • dataloader 2
  • deferred
    • dataloader 3
    • deferred
      • dataloader n
      • deferred

AKA callback hell.

What I tried to come up with, using ->then(), was to un-nest this:

• dataloader 1
• deferred
• then
  • dataloader 2
• then
  • dataloader 3
• then
  • dataloader n

I think I can actually do that, with the only restriction:

• then returns a SyncPromise
• to make the GraphQL resolver accept this, I've to wrap it in a deffered

I.e., same as above but ends with:
- …

• then
  • dataloader n
• deferred returning SyncPromise

Could I better express what I meant?

[vladar]

Edited. You could try something like this:

'resolve' => function (Model $model, $_) {
    $dataloader = $this->getDataLoaderFor('relation1');
    $dataloader->batch($model->relation1_id);

    $startPromise = new SyncPromise();
    $finalPromise = $startPromise
        ->then(function($model) {
            $relation1 = $dataloader->load($model->relation1_id);
            $dataloader2 = $this->getDataLoaderFor('relation2');
            $dataloader2->batch($relation1->relation2_id);
            return $relation1;
        })
        ->then(function($relation1) {
            return $dataloader2->load($relation1->relation2_id);
        });

    return new Deferred(function () use ($model, $startPromise, $finalPromise) {
        $startPromise->resolve($model);
        return $finalPromise;
    });
},

But even if this technique works you should re-check if it actually produces a meaningful sequence of calls to enable batching.

I guess we could improve the API to enable Deferreds chaining. I need to think a bit on how to achieve that.

[vladar]

Can you try PR #581 which should address this?