Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions classes/local/client.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@ class client {
/** @var $httpclient \GuzzleHttp\Client */
protected $httpclient;

/** @var mixed Guzzle handler override used only under PHPUnit. */
protected static $testhandler = null;

/**
* Inject a Guzzle handler (e.g. a MockHandler stack) for unit testing. Has no effect
* outside PHPUnit. Pass null to reset.
*
* @param mixed $handler
*/
public static function set_test_handler($handler) {
static::$testhandler = $handler;
}

/**
* Construct a guzzle client setup with basic authentication and appropriate
* options and headers set.
Expand All @@ -62,6 +75,9 @@ public static function get_instance($headers = []) {
]
];
$config['headers'] = array_merge($config['headers'], $headers);
if (defined('PHPUNIT_TEST') && PHPUNIT_TEST && !is_null(static::$testhandler)) {
$config['handler'] = static::$testhandler;
}
$client = new static();
$client->httpclient = new \GuzzleHttp\Client($config);
return $client;
Expand Down
32 changes: 29 additions & 3 deletions classes/local/job/contacts_job.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,34 @@ public function run() {
$uri->setPagingTop(250);
$uri->setResourcePath($jobpersistent->get('endpoint'));
$uri->addExpand('Registration/Contact');
$filter = "Contact/LastModifiedDateTime gt datetime('". $jobpersistent->get('lastsourcetimemodified') ."')";
$lastsourcetimemodified = $jobpersistent->get('lastsourcetimemodified');
// Page using a (LastModifiedDateTime, ContactID) keyset cursor. The ContactID
// tiebreaker matches the other sync jobs and stops the cursor stalling when more
// than one page of contacts share the same LastModifiedDateTime.
$filter = "(Contact/LastModifiedDateTime gt datetime('" . $lastsourcetimemodified . "'))";
if ($jobpersistent->get('lastsourceid')) {
$filter .= " OR (Contact/LastModifiedDateTime eq datetime('" . $lastsourcetimemodified . "')";
$filter .= " AND Contact/ContactID gt " . $jobpersistent->get('lastsourceid') . ")";
}
$uri->setFilterBy($filter);
$uri->setOrderBy('Contact/LastModifiedDateTime ASC');
$uri->setOrderBy('Contact/LastModifiedDateTime ASC,Contact/ContactID ASC');
$request = new Request('GET', $uri->output(true));
$response = client::get_instance()->send_request($request);
$collection = response_processor::process($response);
if ($collection->count() > 0) {
$cursortimebefore = $jobpersistent->get('lastsourcetimemodified');
$cursoridbefore = $jobpersistent->get('lastsourceid');
foreach ($collection as $resource) {
try {
// No need to process cancelled registrations.
// No need to process cancelled registrations, but still advance the
// paging cursor so a page made up entirely of cancelled registrations
// does not stall pagination and re-request the same page forever.
if ($resource->Status == RegistrationStatus::CANCELLED) {
$cancelledcontact = $resource->getContact();
if (!empty($cancelledcontact) && !empty($cancelledcontact->LastModifiedDateTime)) {
$jobpersistent->set('lastsourceid', $cancelledcontact->ContactID);
$jobpersistent->set('lastsourcetimemodified', $cancelledcontact->LastModifiedDateTime);
}
continue;
}
$contactresource = $resource->getContact();
Expand Down Expand Up @@ -218,6 +235,15 @@ public function run() {
}
// See if need to get another page of records.
$hasnext = (bool) $collection->hasNext();
// Safety net: if Arlo reports more pages but our paging cursor did not move
// while processing this page, stop instead of re-requesting the identical page
// forever (prevents runaway polling of the Arlo API).
if ($hasnext
&& $jobpersistent->get('lastsourcetimemodified') === $cursortimebefore
&& $jobpersistent->get('lastsourceid') == $cursoridbefore) {
$this->add_error(get_string('pagingnoprogress', 'enrol_arlo'));
$hasnext = false;
}
}
}
return true;
Expand Down
1 change: 1 addition & 0 deletions lang/en/enrol_arlo.php
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@
$string['contactresourcemissing'] = 'Contact resource missing from Registration.';
$string['contactrecordmissing'] = 'Contact record missing.';
$string['noassociateduser'] = 'No associated Moodle user account.';
$string['pagingnoprogress'] = 'Paging halted: the sync cursor did not advance while more pages were reported. Stopped to avoid re-requesting the same page.';
$string['unsuccessfulenrolment'] = 'Unsuccessful enrolment';
$string['unsuccessfulenrolments'] = 'Unsuccessful enrolments';
$string['unsuccessfulenrolmentscount'] = 'Unsuccessful enrolments: {$a}';
Expand Down
177 changes: 177 additions & 0 deletions tests/contacts_job_test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* Tests for contacts_job pagination.
*
* Regression coverage for the Batalas incident: a page of registrations that are all
* Cancelled used to leave the paging cursor (lastsourcetimemodified) unchanged while the
* Arlo response still advertised a "next" page, so contacts_job::run() re-requested the
* identical page forever, flooding the Arlo API.
*
* @package enrol_arlo
* @category phpunit
* @copyright 2026 Arlo
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/

defined('MOODLE_INTERNAL') || die();

use enrol_arlo\local\client;
use enrol_arlo\local\job\contacts_job;
use enrol_arlo\local\persistent\job_persistent;
use GuzzleHttp\Handler\MockHandler;
use GuzzleHttp\HandlerStack;
use GuzzleHttp\Middleware;
use GuzzleHttp\Psr7\Response;

class contacts_job_test extends advanced_testcase {

/** @var enrol_arlo_generator $plugingenerator handle to plugin generator. */
protected $plugingenerator;

/** @var array captured Guzzle request/response history for the current test. */
protected $history = [];

/**
* Test setup.
*/
public function setUp(): void {
global $CFG;
parent::setUp();
require_once($CFG->dirroot . '/enrol/arlo/lib.php');
$this->resetAfterTest();
$this->plugingenerator = $this->getDataGenerator()->get_plugin_generator('enrol_arlo');
$this->plugingenerator->enable_plugin();
$this->plugingenerator->setup_plugin();
}

/**
* Reset the injected HTTP handler so static state does not leak between tests.
*/
public function tearDown(): void {
client::set_test_handler(null);
$this->history = [];
parent::tearDown();
}

/**
* Build a course with an Arlo Event enrolment instance.
*
* @return stdClass the enrolment instance record.
*/
protected function create_arlo_instance() {
$course = $this->getDataGenerator()->create_course();
$template = $this->plugingenerator->create_event_template();
$event = $this->plugingenerator->create_event($template);
return $this->plugingenerator->create_event_enrolment_instance($course, $event);
}

/**
* Fetch the auto-registered contacts job for an enrolment instance.
*
* @param int $instanceid
* @return job_persistent
*/
protected function get_contacts_job($instanceid) {
return job_persistent::get_record(
['area' => 'enrolment', 'type' => 'contacts', 'instanceid' => $instanceid]
);
}

/**
* Queue a sequence of fixture files as Arlo XML responses and route the client through them.
*
* @param array $fixtures fixture filenames under tests/fixtures/
*/
protected function mock_arlo_responses(array $fixtures) {
global $CFG;
$responses = [];
foreach ($fixtures as $fixture) {
$xml = file_get_contents($CFG->dirroot . '/enrol/arlo/tests/fixtures/' . $fixture);
$responses[] = new Response(200, ['Content-Type' => 'application/xml'], $xml);
}
$stack = HandlerStack::create(new MockHandler($responses));
$this->history = [];
$stack->push(Middleware::history($this->history));
client::set_test_handler($stack);
}

/**
* A full page of Cancelled registrations must still advance the cursor and stop paging,
* rather than re-requesting the same page forever.
*/
public function test_all_cancelled_page_advances_cursor_and_stops_paging(): void {
$instance = $this->create_arlo_instance();
$job = $this->get_contacts_job($instance->id);

// Sanity: the cursor starts at the epoch default that the flood was stuck on.
$this->assertSame('1970-01-01T00:00:00Z', $job->get('lastsourcetimemodified'));
$this->assertEquals(0, $job->get('lastsourceid'));

// Page 1: 3 Cancelled registrations + a "next" link. Page 2: empty (terminates).
$this->mock_arlo_responses([
'registrations_cancelled_page.xml',
'registrations_empty_page.xml',
]);

$result = (new contacts_job($job))->run();
$this->assertTrue($result);

// Exactly two requests: page 1, then page 2 - NOT an unbounded re-request loop.
$this->assertCount(2, $this->history,
'contacts_job should page once and stop; more requests means the cursor stalled.');

// The cursor advanced past the cancelled page to the last contact on it.
$job->read();
$this->assertSame('2025-07-17T15:39:56.2075043Z', $job->get('lastsourcetimemodified'));
$this->assertEquals(2003, $job->get('lastsourceid'));

// The second request used the advanced keyset cursor (incl. the ContactID tiebreaker).
$secondfilter = urldecode((string) $this->history[1]['request']->getUri());
$this->assertStringContainsString("gt datetime('2025-07-17T15:39:56.2075043Z')", $secondfilter);
$this->assertStringContainsString('Contact/ContactID gt 2003', $secondfilter);
}

/**
* If the cursor genuinely cannot advance (e.g. contacts with no LastModifiedDateTime) while
* the API still reports more pages, the safety net must stop paging instead of looping.
*/
public function test_paging_halts_when_cursor_cannot_advance(): void {
$instance = $this->create_arlo_instance();
$job = $this->get_contacts_job($instance->id);

// Queue the same non-advancing page twice: with the guard only ONE is ever requested.
$this->mock_arlo_responses([
'registrations_cancelled_no_modified_page.xml',
'registrations_cancelled_no_modified_page.xml',
]);

$contactsjob = new contacts_job($job);
$contactsjob->run();

// The guard stopped after a single request rather than re-fetching the identical page.
$this->assertCount(1, $this->history,
'A non-advancing page with a next link must not be requested more than once.');

// Cursor untouched, and the no-progress reason was recorded.
$job->read();
$this->assertSame('1970-01-01T00:00:00Z', $job->get('lastsourcetimemodified'));
$this->assertEquals(0, $job->get('lastsourceid'));
$this->assertTrue($contactsjob->has_errors());
$this->assertContains(get_string('pagingnoprogress', 'enrol_arlo'), $contactsjob->get_errors());
}
}
41 changes: 41 additions & 0 deletions tests/fixtures/registrations_cancelled_no_modified_page.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<Registrations>
<Link rel="http://schemas.arlo.co/api/2012/02/auth/Registration" type="application/xml" title="Registration" href="https://example.arlo.co/api/2012-02-01/auth/resources/registrations/1101/">
<Registration>
<RegistrationID>1101</RegistrationID>
<UniqueIdentifier>00000000-0000-0000-0000-000000001101</UniqueIdentifier>
<Status>Cancelled</Status>
<CreatedDateTime>2023-02-01T09:00:00.0000000Z</CreatedDateTime>
<LastModifiedDateTime>2023-02-10T09:00:00.0000000Z</LastModifiedDateTime>
<Link rel="http://schemas.arlo.co/api/2012/02/auth/related/Contact" type="application/xml" title="Contact" href="https://example.arlo.co/api/2012-02-01/auth/resources/contacts/2101/">
<Contact>
<ContactID>2101</ContactID>
<UniqueIdentifier>00000000-0000-0000-0000-000000002101</UniqueIdentifier>
<FirstName>Test</FirstName>
<LastName>Nolmdone</LastName>
<Email>nolmd1@example.com</Email>
<Status>Active</Status>
</Contact>
</Link>
</Registration>
</Link>
<Link rel="http://schemas.arlo.co/api/2012/02/auth/Registration" type="application/xml" title="Registration" href="https://example.arlo.co/api/2012-02-01/auth/resources/registrations/1102/">
<Registration>
<RegistrationID>1102</RegistrationID>
<UniqueIdentifier>00000000-0000-0000-0000-000000001102</UniqueIdentifier>
<Status>Cancelled</Status>
<CreatedDateTime>2023-03-01T09:00:00.0000000Z</CreatedDateTime>
<LastModifiedDateTime>2023-03-10T09:00:00.0000000Z</LastModifiedDateTime>
<Link rel="http://schemas.arlo.co/api/2012/02/auth/related/Contact" type="application/xml" title="Contact" href="https://example.arlo.co/api/2012-02-01/auth/resources/contacts/2102/">
<Contact>
<ContactID>2102</ContactID>
<UniqueIdentifier>00000000-0000-0000-0000-000000002102</UniqueIdentifier>
<FirstName>Test</FirstName>
<LastName>Nolmdtwo</LastName>
<Email>nolmd2@example.com</Email>
<Status>Active</Status>
</Contact>
</Link>
</Registration>
</Link>
<Link rel="next" type="application/xml" href="https://example.arlo.co/api/2012-02-01/auth/resources/onlineactivities/36/registrations/?top=250&amp;expand=Registration,Registration/Contact"/>
</Registrations>
72 changes: 72 additions & 0 deletions tests/fixtures/registrations_cancelled_page.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<Registrations>
<Link rel="http://schemas.arlo.co/api/2012/02/auth/Registration" type="application/xml" title="Registration" href="https://example.arlo.co/api/2012-02-01/auth/resources/registrations/1001/">
<Registration>
<RegistrationID>1001</RegistrationID>
<UniqueIdentifier>00000000-0000-0000-0000-000000001001</UniqueIdentifier>
<Status>Cancelled</Status>
<CreatedDateTime>2023-02-01T09:00:00.0000000Z</CreatedDateTime>
<LastModifiedDateTime>2023-02-10T09:00:00.0000000Z</LastModifiedDateTime>
<Link rel="self" type="application/xml" href="https://example.arlo.co/api/2012-02-01/auth/resources/registrations/1001/"/>
<Link rel="http://schemas.arlo.co/api/2012/02/auth/related/Contact" type="application/xml" title="Contact" href="https://example.arlo.co/api/2012-02-01/auth/resources/contacts/2001/">
<Contact>
<ContactID>2001</ContactID>
<UniqueIdentifier>00000000-0000-0000-0000-000000002001</UniqueIdentifier>
<FirstName>Test</FirstName>
<LastName>Learnerone</LastName>
<Email>learner1@example.com</Email>
<Status>Active</Status>
<CreatedDateTime>2022-11-01T10:00:00.0000000Z</CreatedDateTime>
<LastModifiedDateTime>2023-03-07T13:11:09.9601334Z</LastModifiedDateTime>
<Link rel="self" type="application/xml" href="https://example.arlo.co/api/2012-02-01/auth/resources/contacts/2001/"/>
</Contact>
</Link>
</Registration>
</Link>
<Link rel="http://schemas.arlo.co/api/2012/02/auth/Registration" type="application/xml" title="Registration" href="https://example.arlo.co/api/2012-02-01/auth/resources/registrations/1002/">
<Registration>
<RegistrationID>1002</RegistrationID>
<UniqueIdentifier>00000000-0000-0000-0000-000000001002</UniqueIdentifier>
<Status>Cancelled</Status>
<CreatedDateTime>2023-04-01T09:00:00.0000000Z</CreatedDateTime>
<LastModifiedDateTime>2023-04-10T09:00:00.0000000Z</LastModifiedDateTime>
<Link rel="self" type="application/xml" href="https://example.arlo.co/api/2012-02-01/auth/resources/registrations/1002/"/>
<Link rel="http://schemas.arlo.co/api/2012/02/auth/related/Contact" type="application/xml" title="Contact" href="https://example.arlo.co/api/2012-02-01/auth/resources/contacts/2002/">
<Contact>
<ContactID>2002</ContactID>
<UniqueIdentifier>00000000-0000-0000-0000-000000002002</UniqueIdentifier>
<FirstName>Test</FirstName>
<LastName>Learnertwo</LastName>
<Email>learner2@example.com</Email>
<Status>Active</Status>
<CreatedDateTime>2022-12-01T10:00:00.0000000Z</CreatedDateTime>
<LastModifiedDateTime>2024-01-15T08:30:00.0000000Z</LastModifiedDateTime>
<Link rel="self" type="application/xml" href="https://example.arlo.co/api/2012-02-01/auth/resources/contacts/2002/"/>
</Contact>
</Link>
</Registration>
</Link>
<Link rel="http://schemas.arlo.co/api/2012/02/auth/Registration" type="application/xml" title="Registration" href="https://example.arlo.co/api/2012-02-01/auth/resources/registrations/1003/">
<Registration>
<RegistrationID>1003</RegistrationID>
<UniqueIdentifier>00000000-0000-0000-0000-000000001003</UniqueIdentifier>
<Status>Cancelled</Status>
<CreatedDateTime>2025-06-01T09:00:00.0000000Z</CreatedDateTime>
<LastModifiedDateTime>2025-06-10T09:00:00.0000000Z</LastModifiedDateTime>
<Link rel="self" type="application/xml" href="https://example.arlo.co/api/2012-02-01/auth/resources/registrations/1003/"/>
<Link rel="http://schemas.arlo.co/api/2012/02/auth/related/Contact" type="application/xml" title="Contact" href="https://example.arlo.co/api/2012-02-01/auth/resources/contacts/2003/">
<Contact>
<ContactID>2003</ContactID>
<UniqueIdentifier>00000000-0000-0000-0000-000000002003</UniqueIdentifier>
<FirstName>Test</FirstName>
<LastName>Learnerthree</LastName>
<Email>learner3@example.com</Email>
<Status>Active</Status>
<CreatedDateTime>2023-01-01T10:00:00.0000000Z</CreatedDateTime>
<LastModifiedDateTime>2025-07-17T15:39:56.2075043Z</LastModifiedDateTime>
<Link rel="self" type="application/xml" href="https://example.arlo.co/api/2012-02-01/auth/resources/contacts/2003/"/>
</Contact>
</Link>
</Registration>
</Link>
<Link rel="next" type="application/xml" href="https://example.arlo.co/api/2012-02-01/auth/resources/onlineactivities/36/registrations/?top=250&amp;expand=Registration,Registration/Contact"/>
</Registrations>
3 changes: 3 additions & 0 deletions tests/fixtures/registrations_empty_page.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<Registrations>
<Link rel="self" type="application/xml" href="https://example.arlo.co/api/2012-02-01/auth/resources/onlineactivities/36/registrations/"/>
</Registrations>
Loading