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

Split OAI server attachNonDeleted function #4160

Merged
merged 8 commits into from
Dec 18, 2024
42 changes: 28 additions & 14 deletions module/VuFind/src/VuFind/OAI/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
use VuFind\Db\Service\ChangeTrackerServiceInterface;
use VuFind\Db\Service\OaiResumptionServiceInterface;
use VuFind\Exception\RecordMissing as RecordMissingException;
use VuFind\RecordDriver\AbstractBase as AbstractRecordDriver;
use VuFind\SimpleXML;
use VuFindApi\Formatter\RecordFormatter;

Expand Down Expand Up @@ -444,24 +445,20 @@ protected function attachNonDeleted(
$headerOnly = false,
$set = ''
) {
// Get the XML (and display an error if it is unsupported):
if ($format === false) {
$xml = ''; // no metadata if in header-only mode!
} elseif ('oai_vufind_json' === $format && $this->supportsVuFindMetadata()) {
$xml = $this->getVuFindMetadata($record); // special case
} else {
$xml = $record
->getXML($format, $this->baseHostURL, $this->recordLinkerHelper);
if ($xml === false) {
return false;
demiankatz marked this conversation as resolved.
Show resolved Hide resolved
}
// If no format was requested, report success without doing anything:
return true;
demiankatz marked this conversation as resolved.
Show resolved Hide resolved
}

$xml = $this->getRecordAsXML($record, $format);

// Headers should be returned only if the metadata format matching
// the supplied metadataPrefix is available.
// If RecordDriver returns nothing, skip this record.
if (empty($xml)) {
return true;
// If returned XML is empty, return true to simply skip this record.
// If returned XML is false, an error was encountered during the process
// of generating the XML file.
if (!$xml) {
return $xml !== false;
}

// Check for sets:
Expand Down Expand Up @@ -492,14 +489,31 @@ protected function attachNonDeleted(
);

// Inject metadata if necessary:
if (!$headerOnly && !empty($xml)) {
if (!$headerOnly) {
$metadata = $recXml->addChild('metadata');
SimpleXML::appendElement($metadata, $xml);
}

return true;
}

/**
* Get record as a metadata presentation
*
* @param AbstractRecordDriver $record A record driver object
* @param string $format Metadata format to obtain
*
* @return string|false String or false if an error occured
*/
protected function getRecordAsXML(AbstractRecordDriver $record, string $format): string|false
{
if ('oai_vufind_json' === $format && $this->supportsVuFindMetadata()) {
// Return empty string instead of false for BC.
return $this->getVuFindMetadata($record) ?: '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the commitment to backward compatibility here, but I actually think in this instance, it was a bug that we didn't handle false return values from getVuFindMetadata(). I'd suggest allowing the false return, and also fixing the return type annotation and comment on getVuFindMetadata() to reflect the possibility that an error can occur.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, well, from an improvement to a bugfix, will adjust :)

}
return $record->getXML($format, $this->baseHostURL, $this->recordLinkerHelper);
}

/**
* Respond to a GetRecord request.
*
Expand Down
Loading