Skip to content

Commit

Permalink
MDL-74691 enrol_lti: allow omission of lineitems property in ags claim
Browse files Browse the repository at this point in the history
The claim is valid with either or both of the URLs, so long as there's
at least one.
  • Loading branch information
snake committed Jun 16, 2022
1 parent 4a9fa42 commit 006b322
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 28 deletions.
20 changes: 13 additions & 7 deletions enrol/lti/classes/local/ltiadvantage/entity/ags_info.php
Expand Up @@ -38,7 +38,7 @@ class ags_info {
/** @var string Scope for posting scores.*/
private const SCOPES_SCORES_POST = 'https://purl.imsglobal.org/spec/lti-ags/scope/score';

/** @var \moodle_url The service URL used to get/put lineitems*/
/** @var \moodle_url|null The service URL used to get/put lineitems, if supported*/
private $lineitemsurl;

/** @var \moodle_url|null The lineitemurl, which is only present when a single lineitem is supported.*/
Expand All @@ -56,11 +56,17 @@ class ags_info {
/**
* The ags_info constructor.
*
* @param \moodle_url $lineitemsurl The service URL used to get/put lineitems.
* @param \moodle_url|null $lineitemsurl The service URL used to get/put lineitems, if supported.
* @param \moodle_url|null $lineitemurl The lineitemurl, which is only present when a single lineitem is supported.
* @param array $scopes The array of supported scopes for this service instance.
*/
private function __construct(\moodle_url $lineitemsurl, ?\moodle_url $lineitemurl, array $scopes) {
private function __construct(?\moodle_url $lineitemsurl, ?\moodle_url $lineitemurl, array $scopes) {

// Platforms may support just lineitemurl, just lineitemsurl or both. At least one of the two is required.
if (is_null($lineitemsurl) && is_null($lineitemurl)) {
throw new \coding_exception("Missing lineitem or lineitems URL");
}

$this->lineitemsurl = $lineitemsurl;
$this->lineitemurl = $lineitemurl;
$this->validate_scopes($scopes);
Expand All @@ -69,12 +75,12 @@ private function __construct(\moodle_url $lineitemsurl, ?\moodle_url $lineitemur
/**
* Factory method to create a new ags_info instance.
*
* @param \moodle_url $lineitemsurl The service URL used to get/put lineitems.
* @param \moodle_url|null $lineitemsurl The service URL used to get/put lineitems, if supported.
* @param \moodle_url|null $lineitemurl The lineitemurl, which is only present when a single lineitem is supported.
* @param array $scopes The array of supported scopes for this service instance.
* @return ags_info the object instance.
*/
public static function create(\moodle_url $lineitemsurl, ?\moodle_url $lineitemurl = null,
public static function create(?\moodle_url $lineitemsurl = null, ?\moodle_url $lineitemurl = null,
array $scopes = []): ags_info {
return new self($lineitemsurl, $lineitemurl, $scopes);
}
Expand Down Expand Up @@ -110,11 +116,11 @@ private function validate_scopes(array $scopes): void {
}

/**
* Get the url for querying line items.
* Get the url for querying line items, if supported.
*
* @return \moodle_url the url.
*/
public function get_lineitemsurl(): \moodle_url {
public function get_lineitemsurl(): ?\moodle_url {
return $this->lineitemsurl;
}

Expand Down
4 changes: 2 additions & 2 deletions enrol/lti/classes/local/ltiadvantage/entity/resource_link.php
Expand Up @@ -163,11 +163,11 @@ public function set_resourceid(int $resourceid): void {
/**
* Add grade service information to this resource_link instance.
*
* @param \moodle_url $lineitemsurl the service URL for get/put of line items.
* @param \moodle_url|null $lineitemsurl the service URL for get/put of line items, if supported.
* @param \moodle_url|null $lineitemurl the service URL if only a single line item is present in the platform.
* @param string[] $scopes the string array of grade service scopes which may be used by the service.
*/
public function add_grade_service(\moodle_url $lineitemsurl, ?\moodle_url $lineitemurl = null, array $scopes = []) {
public function add_grade_service(?\moodle_url $lineitemsurl = null, ?\moodle_url $lineitemurl = null, array $scopes = []) {
$this->gradeservice = ags_info::create($lineitemsurl, $lineitemurl, $scopes);
}

Expand Down
Expand Up @@ -50,7 +50,7 @@ private function resource_link_from_record(\stdClass $record): resource_link {
$record->id
);

if ($record->lineitemsservice) {
if ($record->lineitemsservice || $record->lineitemservice) {
$scopes = [];
if ($record->lineitemscope) {
$lineitemscopes = json_decode($record->lineitemscope);
Expand All @@ -65,7 +65,7 @@ private function resource_link_from_record(\stdClass $record): resource_link {
$scopes[] = $record->scorescope;
}
$resourcelink->add_grade_service(
new \moodle_url($record->lineitemsservice),
$record->lineitemsservice ? new \moodle_url($record->lineitemsservice) : null,
$record->lineitemservice ? new \moodle_url($record->lineitemservice) : null,
$scopes
);
Expand Down Expand Up @@ -112,7 +112,7 @@ private function record_from_resource_link(resource_link $resourcelink): \stdCla
'ltideploymentid' => $resourcelink->get_deploymentid(),
'resourceid' => $resourcelink->get_resourceid(),
'lticontextid' => $resourcelink->get_contextid(),
'lineitemsservice' => $gradeservice ? $gradeservice->get_lineitemsurl()->out(false) : null,
'lineitemsservice' => null,
'lineitemservice' => null,
'lineitemscope' => null,
'resultscope' => $gradeservice ? $gradeservice->get_resultscope() : null,
Expand All @@ -121,7 +121,10 @@ private function record_from_resource_link(resource_link $resourcelink): \stdCla
'nrpsserviceversions' => $nrpservice ? json_encode($nrpservice->get_service_versions()) : null
];

if ($gradeservice && ($lineitemurl = $gradeservice->get_lineitemurl())) {
if ($gradeservice && ($lineitemsurl = $gradeservice->get_lineitemsurl())) {
$record['lineitemsservice'] = $lineitemsurl->out(false);
}
if ($gradeservice && ($lineitemurl = $gradeservice->get_lineitemurl())) {
$record['lineitemservice'] = $lineitemurl->out(false);
}
if ($gradeservice && ($lineitemscopes = $gradeservice->get_lineitemscope())) {
Expand Down
Expand Up @@ -163,12 +163,12 @@ private function resource_link_from_launchdata(\stdClass $launchdata, \stdClass
$context ? $context->get_id() : null
);
}
// AGS. If the lineitemsurl is missing, it means the tool has no access to the endpoint.
// Add the AGS configuration for the resource link.
// See: http://www.imsglobal.org/spec/lti-ags/v2p0#assignment-and-grade-service-claim.
if ($launchdata->ags && $launchdata->ags['lineitems']) {
if ($launchdata->ags && (!empty($launchdata->ags['lineitems']) || !empty($launchdata->ags['lineitem']))) {
$resourcelink->add_grade_service(
new \moodle_url($launchdata->ags['lineitems']),
isset($launchdata->ags['lineitem']) ? new \moodle_url($launchdata->ags['lineitem']) : null,
!empty($launchdata->ags['lineitems']) ? new \moodle_url($launchdata->ags['lineitems']) : null,
!empty($launchdata->ags['lineitem']) ? new \moodle_url($launchdata->ags['lineitem']) : null,
$launchdata->ags['scope']
);
}
Expand Down
29 changes: 29 additions & 0 deletions enrol/lti/tests/local/ltiadvantage/entity/ags_info_test.php
Expand Up @@ -286,6 +286,35 @@ public function instantiation_data_provider(): array {
'exceptionmessage' => "Scope must be a string value"
]
],
'Claim contains a single lineitem URL only with valid scopes' => [
'args' => [
'lineitemsurl' => null,
'lineitemurl' => new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'),
'scopes' => [
'https://purl.imsglobal.org/spec/lti-ags/scope/score'
]
],
'expectations' => [
'valid' => true,
'lineitemscope' => null,
'scorescope' => 'https://purl.imsglobal.org/spec/lti-ags/scope/score',
'resultscope' => null
]
],
'Claim contains no lineitems URL or lineitem URL' => [
'args' => [
'lineitemsurl' => null,
'lineitemurl' => null,
'scopes' => [
'https://purl.imsglobal.org/spec/lti-ags/scope/score'
]
],
'expectations' => [
'valid' => false,
'exception' => \coding_exception::class,
'exceptionmessage' => "Missing lineitem or lineitems URL"
]
],
];
}
}
95 changes: 84 additions & 11 deletions enrol/lti/tests/local/ltiadvantage/entity/resource_link_test.php
Expand Up @@ -115,23 +115,96 @@ public function instantiation_data_provider(): array {
/**
* Test confirming that a grade service instance can be added to the object instance.
*
* @param array $args the array of method arguments
* @param array $expected the array of expectations
* @dataProvider add_grade_service_provider
* @covers ::add_grade_service
*/
public function test_add_grade_service() {
public function test_add_grade_service(array $args, array $expected) {
$reslink = resource_link::create('res-link-id-123', 24, 44);
$this->assertNull($reslink->get_grade_service());
$reslink->add_grade_service(
new \moodle_url('https://platform.example.org/10/lineitems'),
new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'),
['https://purl.imsglobal.org/spec/lti-ags/scope/lineitem']
);

if (!$expected['valid']) {
$this->expectException($expected['exception']);
}
$reslink->add_grade_service(...array_values($args));
$gradeservice = $reslink->get_grade_service();
$this->assertInstanceOf(ags_info::class, $gradeservice);
$this->assertEquals(new \moodle_url('https://platform.example.org/10/lineitems'),
$gradeservice->get_lineitemsurl());
$this->assertEquals(new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'),
$gradeservice->get_lineitemurl());
$this->assertEquals(['https://purl.imsglobal.org/spec/lti-ags/scope/lineitem'], $gradeservice->get_scopes());
$this->assertEquals($args['lineitemsurl'], $gradeservice->get_lineitemsurl());
$this->assertEquals($args['lineitemurl'], $gradeservice->get_lineitemurl());
$this->assertEquals($args['scope'], $gradeservice->get_scopes());
}

/**
* Data provider for testing the add_grade_service method.
*
* @return array the array of test case data.
*/
public function add_grade_service_provider(): array {
return [
'Valid, both URLs, some scopes' => [
'args' => [
'lineitemsurl' => new \moodle_url('https://platform.example.org/10/lineitems'),
'lineitemurl' => new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'),
'scope' => [
'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem',
'https://purl.imsglobal.org/spec/lti-ags/scope/score'
]
],
'expected' => [
'valid' => true,
]
],
'Valid, only coupled line item URL, some scopes' => [
'args' => [
'lineitemsurl' => null,
'lineitemurl' => new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'),
'scope' => [
'https://purl.imsglobal.org/spec/lti-ags/scope/score'
]
],
'expected' => [
'valid' => true,
]
],
'Valid, only decoupled line items URL, some scopes' => [
'args' => [
'lineitemsurl' => new \moodle_url('https://platform.example.org/10/lineitems'),
'lineitemurl' => null,
'scope' => [
'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem',
'https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly',
'https://purl.imsglobal.org/spec/lti-ags/scope/score',
]
],
'expected' => [
'valid' => true,
]
],
'Valid, URLs without any scopes' => [
'args' => [
'lineitemsurl' => new \moodle_url('https://platform.example.org/10/lineitems'),
'lineitemurl' => new \moodle_url('https://platform.example.org/10/lineitems/4/lineitem'),
'scope' => []
],
'expected' => [
'valid' => true,
]
],
'Invalid, missing both URLs' => [
'args' => [
'lineitemsurl' => null,
'lineitemurl' => null,
'scope' => [
'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem'
]
],
'expected' => [
'valid' => false,
'exception' => \coding_exception::class
]
],
];
}

/**
Expand Down

0 comments on commit 006b322

Please sign in to comment.