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

OAI Refactoring #1094

Merged
merged 59 commits into from
Oct 20, 2023
Merged

OAI Refactoring #1094

merged 59 commits into from
Oct 20, 2023

Conversation

haogatyp
Copy link
Contributor

No description provided.

@j3nsch j3nsch marked this pull request as draft August 17, 2023 15:23
$formatOptions = [];
if (isset($config->oai->format->$metadataPrefix)) {
$formatOptions = $config->oai->format->$metadataPrefix->toArray();
unset($options['class']);
Copy link
Member

Choose a reason for hiding this comment

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

Ich denke class sollte einfach drin bleiben. Dann spart man sich auch die ganze Sonderbehandlung in getFormatClassName. Die Funktion kann dann wegfallen.

$server = new $serverClass();
}

$options = $this->getFormatOptions($metaDataPrefix);
Copy link
Member

Choose a reason for hiding this comment

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

Die Klasse sollte einfach aus den Options kommen. Keine Sonderbehandlung mit getFormatClassName.

Copy link
Member

Choose a reason for hiding this comment

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

Wenn man möchte kann man dann hier class immer noch entfernen, bevor man setOptions an dem Server-Objekt aufruft.

@@ -43,13 +43,6 @@

<!-- add include here for each new metadata format -->
Copy link
Member

Choose a reason for hiding this comment

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

Das ist ja die alte Aufforderung hier manuell Includes hinzuzufügen. Klar funktioniert das, als Marker, aber das sollte so geändert werden, dass es ganz klar ein "technischer" Marker ist der automatisch ersetzt wird.

oai.format.default.hasFilesVisibleInOai = 0

; OPUS 4 XML (only for admins)
oai.format.copy_xml.class = Oai_Model_Prefix_CopyXml_CopyXmlServer
Copy link
Member

Choose a reason for hiding this comment

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

Sieht gut aus. Das ist die Idee, aber lass mal bitte die CopyXmlServer Klasse weg. Für dieses eine Format machen wir das ohne Klasse.

Wir haben hier auch noch ein weiteres Problem mit der Klasse. WIr hatten ja vereinbart prefixLabel als "Flag" zu verwenden, ob das Format bei ListMetadataFormats auftauchen soll oder nicht. Gibt es einen Eintrag in der Konfiguration, der aber leer ist, wird es nicht gelistet. Wenn man versucht das mit der Klasse zu machen wird es schwierig, weil man nicht so einfach zwischen NULL weil nicht gesetzt und NULL explizit gesetzt unterscheiden kann.

Copy link
Member

Choose a reason for hiding this comment

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

Vermutlich werden wir doch noch eine Option für die "Sichtbarkeit" brauchen und eine Option für die Access-Kontrolle, vielleicht "adminOnly". Das kann später ausgebaut werden, wenn es UseCases für eine feinere Kontrolle über ACLs gibt.

Copy link
Member

Choose a reason for hiding this comment

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

Wir könnten jetzt auch erst einmal sagen, Formate, die nicht sichtbar sind, können nur von Admins abgerufen werden und uns damit die zweite Option erst einmal sparen.

tests/Bootstrap.php Outdated Show resolved Hide resolved
@j3nsch j3nsch marked this pull request as ready for review October 20, 2023 13:40
@j3nsch j3nsch merged commit c676f59 into v4.8.1 Oct 20, 2023
4 checks passed
@j3nsch j3nsch deleted the oai-refactoring branch October 20, 2023 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants