-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: rework client configuration to make DX better #38
base: main
Are you sure you want to change the base?
Conversation
BREAKING CHANGE: Simplified the API by moving client metadata from the request to the constructor in GoogleSafeBrowsing. This change streamlines the threatMatches.find method usage by reducing redundancy, making it easier to initialize and use the API client in different contexts. Updated relevant documentation and tests to reflect these modifications.
Reviewer's Guide by SourceryThe PR refactors the GoogleSafeBrowsing client configuration by moving client metadata from the request payload to the constructor. This simplifies the API usage by reducing parameter redundancy and making the client initialization more straightforward. The implementation involves modifying the class constructor, updating the findThreatMatches method signature, and removing the now-unnecessary FindThreatMatchesRequest interface. Updated class diagram for GoogleSafeBrowsingclassDiagram
class GoogleSafeBrowsing {
- String apiKey
- ClientInfo client
- String endpoint
+ GoogleSafeBrowsing(String apiKey, ClientInfo client, String endpoint)
+ findThreatMatches(ThreatInfo threatInfo) Promise<FindThreatMatchesResponse>
}
class ClientInfo {
+ String clientId
+ String clientVersion
}
class ThreatInfo {
+ List<String> threatTypes
+ List<String> platformTypes
+ List<String> threatEntryTypes
+ List<ThreatEntry> threatEntries
}
class ThreatEntry {
+ String url
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @hckhanh - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
||
Finds the threat entries that match the Safe Browsing lists. | ||
|
||
```ts | ||
import { GoogleSafeBrowsing } from '@hckhanh/google-safe-browsing' | ||
|
||
const client = new GoogleSafeBrowsing('apiKey') | ||
const client = new GoogleSafeBrowsing('apiKey', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Consider documenting the purpose and requirements for clientId and clientVersion configuration parameters
Adding a brief explanation of these configuration parameters would help developers understand how to properly set up the client.
// Initialize Google Safe Browsing client with API key and required identifiers
// clientId: Unique identifier for your application instance
// clientVersion: Current version of your application
const client = new GoogleSafeBrowsing('apiKey', {
README.md
Outdated
threatEntryTypes: ['URL'], | ||
threatEntries: [ | ||
{ url: 'http://malware.testing.google.test/testing/malware/' }, | ||
], | ||
}) | ||
|
||
const hasRisk = result.matches !== undefined && result.matches.length > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Consider documenting the response structure and possible values
It would be helpful to include documentation about the structure of the response object and what developers can expect in different scenarios.
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
This update introduces a new method to detect threats from URLs using the Google Safe Browsing API. It includes enhanced error handling with more descriptive error messages and updates to the README.md with usage examples. Additionally, tests have been added to confirm the correct detection of malicious links.
Quality Gate passedIssues Measures |
BREAKING CHANGE: Simplified the API by moving client metadata from the request to the constructor in GoogleSafeBrowsing. This change streamlines the threatMatches.find method usage by reducing redundancy, making it easier to initialize and use the API client in different contexts. Updated relevant documentation and tests to reflect these modifications.
Summary by Sourcery
Rework the GoogleSafeBrowsing client configuration by moving client metadata from the request to the constructor, simplifying the API usage and improving developer experience. Update documentation and tests to align with these changes.
New Features:
Enhancements:
Documentation:
Tests: