Stability fixes, Laravel compatibility updates, EPO OPS improvements & initial implementation of USPTO ODP#180
Stability fixes, Laravel compatibility updates, EPO OPS improvements & initial implementation of USPTO ODP#180srdco wants to merge 18 commits into
Conversation
…r-from-ops Fix OPS family import when applicant/inventor arrays are empty
…r-from-ops Introduce FamilyDataService and USPTOService to enrich OPS family data and harden parsing
…r-from-ops-05ghel Introduce FamilyDataService and USPTO enrichment; refactor OPS usage and auth routes
…r-from-ops-ns4v6y Add FamilyDataService and USPTO ODP enrichment for US family imports
…r-from-ops-bs1lnq Default USPTO ODP integration to built-in endpoints (no endpoint env required)
…oller Replace AuthenticatesUsers trait with explicit login/logout implementation using `login` field
…on-on-entity-attach Prevent duplicate matter-actor pivot inserts (avoid uqactor_role violation)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new FamilyDataService to orchestrate patent family retrieval, integrating USPTO ODP as an enrichment and fallback source for US applications alongside the existing EPO OPS service. It also removes the dependency on the laravel/ui package by manually implementing authentication logic and explicitly defining routes. Key feedback includes addressing a security vulnerability regarding missing rate limiting in the new login implementation, handling potential API response format inconsistencies in OPSService, and adding timeouts to external HTTP requests in USPTOService. Additionally, a race condition was identified in the ActorPivotController duplicate check.
| public function login(Request $request) | ||
| { | ||
| $this->validateLogin($request); | ||
|
|
||
| if (! Auth::attempt($this->credentials($request), $request->has('remember'))) { | ||
| return $this->sendFailedLoginResponse($request); | ||
| } |
There was a problem hiding this comment.
The manual implementation of the login logic lacks rate limiting, which was previously provided by the AuthenticatesUsers trait (via the ThrottlesLogins trait it uses). This leaves the login endpoint vulnerable to brute-force attacks. Since you are avoiding framework traits for stability, consider implementing rate limiting manually using Laravel's RateLimiter facade within the login method to maintain security parity.
| continue; | ||
| } | ||
|
|
||
| $response = Http::withHeaders($headers)->acceptJson()->get($url); |
There was a problem hiding this comment.
External HTTP requests should have a defined timeout to prevent the application from hanging if the USPTO API is unresponsive or slow. This should also be applied to the search requests on lines 116 and 136.
$response = Http::timeout(10)->withHeaders($headers)->acceptJson()->get($url);| $inventors = collect($member[0]['exchange-document']['bibliographic-data']['parties']['inventors']['inventor'] ?? []) | ||
| ->where('@data-format', 'original') | ||
| ->values() | ||
| ->pluck('inventor-name.name.$') | ||
| ->filter() | ||
| ->values() | ||
| ->all(); |
There was a problem hiding this comment.
The EPO OPS API may return a single object instead of an array when there is only one inventor. In such cases, collect() will create a collection of the object's properties (keys and values), and the subsequent where() filter will fail to find the data because it expects a list of objects. Normalizing the data to a list using Arr::isList ensures the collection logic works correctly for both single and multiple inventors.
$inventorData = $member[0]['exchange-document']['bibliographic-data']['parties']['inventors']['inventor'] ?? [];
$inventors = collect(Arr::isList($inventorData) ? $inventorData : [$inventorData])
->where('@data-format', 'original')
->values()
->pluck('inventor-name.name.$')
->filter()
->values()
->all();| $applicants = collect($member[0]['exchange-document']['bibliographic-data']['parties']['applicants']['applicant'] ?? []) | ||
| ->where('@data-format', 'original') | ||
| ->values() | ||
| ->pluck('applicant-name.name.$') | ||
| ->filter() | ||
| ->values() | ||
| ->all(); |
There was a problem hiding this comment.
Similar to the inventors collection, the applicants data can also be returned as a single object by the API. It should be normalized to a list before filtering to avoid issues with single-applicant patents.
$applicantData = $member[0]['exchange-document']['bibliographic-data']['parties']['applicants']['applicant'] ?? [];
$applicants = collect(Arr::isList($applicantData) ? $applicantData : [$applicantData])
->where('@data-format', 'original')
->values()
->pluck('applicant-name.name.$')
->filter()
->values()
->all();| $existingLink = ActorPivot::where('matter_id', $request->matter_id) | ||
| ->where('role', $request->role) | ||
| ->where('actor_id', $request->actor_id) | ||
| ->first(); | ||
|
|
||
| if ($existingLink) { | ||
| return $existingLink; | ||
| } |
There was a problem hiding this comment.
This check introduces a "look-before-you-leap" pattern which is susceptible to race conditions. While it reduces the likelihood of duplicate entries, it doesn't prevent them if two requests occur simultaneously. To truly handle unique constraint violations gracefully as mentioned in the PR description, consider wrapping the eventual insertion logic in a try-catch block for UniqueConstraintViolationException or using atomic operations like firstOrCreate if the subsequent logic allows.
|
Well, thanks for the contribution! Can you also update the "New features" section of the root README.md, where the new features are summarized? Maybe also a short section in the Wiki on how to use the new features? Finally, did you consider the Gemini proposals? |
|
Not yet - I'm trying to find some time to address those comments which
looked like good suggestions.
|
2 participants
Title
Description
This PR consolidates multiple fixes and enhancements focused on system stability, framework compatibility, and improved USPTO ODP support and documentation.
Key changes:
Data handling & integrity
Framework & authentication updates
Auth::routesmacro with explicit route definitionsDatabase compatibility
USPTO ODP improvements
General maintenance
Overall, this PR improves robustness in edge cases, improves compatibility with newer Laravel/PHP environments, and streamlines matter creation through EPO & USPTO.