Institusion token support added - #8
Closed
matasarei wants to merge 74 commits into
Closed
Conversation
matasarei
commented
May 14, 2023
matasarei
commented
Contributor
- Institusion token support added;
- Updated GuzzleHttp to the latest version;
- Docs and changelog updated.
security: Update guzzle to ^7.15.2 to resolve SSRF vulnerability
docs: Update README and CHANGELOG to follow best practices
Today's suite, CI matrix and PHP 8 fixes all landed after the 1.4.1 entry and were unrecorded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
poser.pugx.org rendered 'No Release' for the version badge and '-' for the PHP requirement, while Packagist itself serves v1.4.1 and php >=7.2 for this package - its cache is stale. shields.io returns the correct values, so all three move there. CI goes first and is pinned to ?branch=master so it reports the default branch rather than the last run to finish. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
array_key_exists(0, $this->data['affiliation-current']['affiliation']) dereferences two keys before it checks anything, so a profile without affiliation-current raised two warnings and then a TypeError: array_key_exists(): Argument #2 ($array) must be of type array, null given. The sparse-data pass missed this one because the line mentions array_key_exists and was scanned as already guarded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
…eas() Both checked only the outer key and then read a sub-key from it, so a payload carrying 'affiliation-history' or 'subject-areas' without its inner list raised a warning and then a TypeError: array_map(): Argument #2 ($array) must be of type array, null given. isset() walks the whole path, so moving the check onto the full expression covers both levels. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
prepareAffiliationData() and prepareSubjectArea() read @id, @href, @abbrev, @code and $ without checking them, so an affiliation or subject area missing any of those raised 'Undefined array key' on PHP 8 and handed back a DTO with silently null fields. prepareSubjectArea() has the same defect as the reviewed helper and is fixed in the same commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
The method declares ': bool' but returned $this->data['@status'] raw, so a document entry without that key raised 'Undefined array key' and then TypeError: Return value must be of type bool, null returned. It now returns false when the key is absent. The declared type is kept and present values coerce exactly as before, so no caller changes - but note the coercion means 'found' and 'NOT_FOUND' both return true, which is a separate defect this commit deliberately does not change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
assertEquals("found", $results[0]->getStatus()) passed only through loose
comparison - the ': bool' declaration means the method returns true, not
the string. The assertion now states that contract, with a comment
recording that 'found' and 'NOT_FOUND' are indistinguishable today.
Adds coverage for the missing-@status path, constructing CitationCount
directly because retrieve() uses @status to tell a single document from
a list and so never produces one without it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
The libxml warnings this branch removed were caught only by the PHP <= 8.0 jobs, because PHPUnit 8.5/9.x promote warnings to errors on their own. On PHPUnit 10/11 the same regression would go green. failOnWarning is accepted by 8.5, 9.6, 10.5 and 11.5 - verified on all four - so it validates across the whole require-dev range. failOnDeprecation is deliberately not set: it would also fire on deprecations raised by vendor code on a future PHP. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
The example carried a 32-character hex string shaped exactly like a live Elsevier API key, under a comment telling readers to replace it. Whether or not it was ever valid, it should not be in the repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
The pre-parse libxml_clear_errors() wiped the global libxml error buffer, which belongs to whoever called us - a consumer accumulating errors with libxml_use_internal_errors(true) lost them just by making a request. There is no API to push errors back, so instead of clearing we record the buffer depth first and take our error only if the count grew. The buffer is cleared afterwards only when the caller had internal errors off, in which case it was never theirs to keep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
Two gaps, both reachable: An empty response body with an XML content type makes simplexml_load_string() return false without recording a libxml error, so the 'Unknown XML parsing error' fallback is not defensive-only - it is the path master crashed on. Verified reachable on PHP 7.2 and 8.4 alike. Nothing asserted that a caller's own libxml errors survive a request. The new test fails against the previous implementation and passes now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
It is development metadata like the rest of the export-ignore list and has no meaning inside a consumer's vendor/ directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
Breaking. PHP 7.2 and 7.3 reached end of life in November 2020 and
December 2021, and nothing depends on them here:
- Packagist reports 0 downloads and 0 dependents; the package was only
registered there today, so there is no installed base to break.
- Every other PHP project in the organisation already requires >=7.4
(registries, studbase, eportfolio, ldap-php, diia-php all ^7.4;
strike-plagiarism-php ^7.4 || ^8.0).
Keeping the floor was not free: it forced require-dev to span PHPUnit 8.5
to 11, which is why phpunit.xml.dist carries no coverage configuration,
and it split the dependency graph across two Guzzle majors.
phpunit/phpunit ^8.5 is dropped from require-dev with it - 8.5 was only
ever selected on 7.2/7.3; PHP 7.4 resolves to 9.6.
The guzzle constraint is deliberately left as ^7.15.2 || ^8.0.1. Dropping
guzzle 7 is not required by the PHP bump and would cause conflicts for
consumers whose projects already pin ^7.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
fix: run clean on every supported PHP version and turn CI green
composer.json declares guzzlehttp/guzzle ^7.15.2 || ^8.0.1, but guzzle 8 requires only ^7.4 || ^8.0, so it wins on every PHP version left in the matrix. Dropping the 7.2 and 7.3 jobs therefore removed the only legs that ever installed guzzle 7.15.x, leaving half the declared constraint unexercised. Adds one --prefer-lowest leg on PHP 7.4, which resolves guzzle to 7.15.2. The install step becomes composer update: no composer.lock is committed, so install was already resolving fresh, and --prefer-lowest needs update. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
'>=7.4' lets composer install this on PHP 9 when it lands - a compatibility claim nothing has tested, and one the package cannot honour in advance. '^7.4 || ^8.0' says what CI actually covers. Verified: still installs on 8.5, and now refuses PHP 9 with 'Root composer.json requires php ^7.4 || ^8.0'. README's requirements line is updated to match, since '>= 7.4' would now overclaim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
The file's header says the format is based on Keep a Changelog, which orders sections Added, Changed, Deprecated, Removed, Fixed, Security. 'Changed' was sitting above 'Added'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
'No consumer of this package targets them' was true, but it asks the reader to take an unverifiable claim on trust. The concrete constraints - the PHPUnit 8.5-to-11 require-dev span and the two Guzzle majors - say the same thing and are visible in the repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
build: require PHP 7.4, dropping 7.2 and 7.3
Minor, not major. The PHP floor moved from 7.2 to 7.4, but no public API changed and composer excludes versions whose platform requirement is unmet rather than failing - a project on PHP 7.3 keeps resolving to 1.4.1. The earlier 'requires a major version bump' note was reasoning from convention instead of from what actually breaks. Adds a Removed section for the files that left the distributed package, which is the other change that is more than a patch. Also corrects the 1.4.0 date: the tag points at a commit dated 2023-05-14, not 2023-06-14. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
These methods end with an if-block and fall off the end when it does not match, returning null implicitly. The value is unchanged - this is legibility only, and matches the shape AbstractItem::getBibrecord already uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
Packagist uses these for search and for the Issues/Source/Docs links on the package page; without them the listing has none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
1.4.2 was cut on the 1.4.x maintenance branch, so master's changelog never mentioned a released version that exists on Packagist. Also adds the link definitions Keep a Changelog expects, so the version headings resolve to compare views, plus an empty Unreleased section for the next change. All eight links verified to return 200. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
docs/ was generated in May 2023 and never refreshed, but it is published on GitHub Pages (master branch, /docs) and the README sends users there as the official API documentation. It still showed each(), which was removed today because it is fatal on PHP 8, and none of this year's changes. Regenerated with ApiGen 7.0.0-alpha.6, the successor to the 'ApiGen dev-master' that produced the original. Every existing filename is preserved, so no published deep link breaks; the 9 new files are the extra Guzzle 8 and PSR-17 interfaces now referenced. README records the exact command, including the --workers 1 that the PHAR requires, so the next person does not have to work it out from a footer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
docs/ is generated output published on GitHub Pages, and it had gone three years stale without anything noticing - it was still showing each(), removed in PHP 8. This regenerates it and fails if the result differs from what is committed. Scoped by path rather than run on every push: the generated output also depends on the resolved dependency versions - guzzle 7 and guzzle 8 produce 32 differing files - so an unconditional check would go red whenever a dependency published, unrelated to any change here. Verified both directions locally: untouched source produces no diff, and a one-line docblock change in src/ is detected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
A byte-exact drift gate does not work here. ApiGen lists implementing classes in filesystem order without sorting them, so identical source produces different output on macOS and Linux - 8 files differ, all of them interface pages, differing only in the order of their 'implemented by' lists. The first version of this job failed CI for exactly that reason while passing locally. Making CI the only generator removes the problem rather than working around it: one platform, one ordering. On any change to src/ or composer.json, the workflow regenerates docs/ and commits the result, so what GitHub Pages serves always matches master. docs/** is deliberately not a trigger, so the commit cannot retrigger the workflow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RuoLdar57twsLspTGjNCpb
ci: regenerate docs/ automatically on every source change
Contributor
Author
|
Closing this — with thanks for the original library, which we have been building on since 2023.
Apologies for the noise on this thread: the PR was opened from our fork's The institution-token support this PR originally added is in the fork if anyone wants it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.