add well known endpoints. #89
Conversation
WalkthroughThe changes in this pull request introduce several enhancements to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (8)
core/src/commonMain/kotlin/work/socialhub/kbsky/ATProtocolConfig.kt (1)
23-26: ApproveupdateAuthUriwith a suggestionThe new
updateAuthUriproperty is a good addition, providing control over AuthURI updates. However, the KDoc comment could be more informative.Consider expanding the KDoc comment to provide more context on when and why a user might want to set this to
false. For example:/** * Controls whether to update AuthURI when retrieving Meta resources. * Set to false if you want to use a fixed AuthURI regardless of Meta resource information. * This can be useful in scenarios where you're working with a custom or non-standard setup. */ var updateAuthUri: Boolean = truecore/src/commonMain/kotlin/work/socialhub/kbsky/api/entity/meta/WellKnownOAuthProtectedResourceResponse.kt (1)
1-23: Summary: Consider implementing all suggested changes for improved robustness.Overall, the
WellKnownOAuthProtectedResourceResponseclass is well-structured, but there are several areas for improvement:
- Consider using a
data classinstead of a regular class.- Revise the property declarations, especially for
List<String>types, to either use nullable types or provide default values.- Consider the nullability of
Stringproperties based on the API's behavior.Implementing these changes will make the class more robust and better aligned with Kotlin best practices. After making these changes, ensure that the class still correctly represents the API response structure.
core/src/commonMain/kotlin/work/socialhub/kbsky/api/meta/MetaResource.kt (3)
9-14: LGTM with a minor suggestion: Consider generalizing the URL in the comment.The method
wellKnownOAuthProtectedResourceis well-defined and properly documented. However, the URL in the KDoc comment is specific to a particular host (oyster.us-east.host.bsky.network). Consider making it more generic to reflect that this interface might be used with different hosts.You could update the comment to something like:
/** * Get OAuth Protected Resource * Endpoint: /.well-known/oauth-protected-resource */This way, the comment remains accurate regardless of the specific host being used.
17-22: LGTM with a minor suggestion: Consider generalizing the URL in the comment.The method
wellKnownOAuthAuthorizationServeris well-defined and properly documented. However, as with the previous method, the URL in the KDoc comment is specific to a particular host (bsky.social). Consider making it more generic for better flexibility.You could update the comment to:
/** * Get OAuth authorization server * Endpoint: /.well-known/oauth-authorization-server */This change would make the interface more adaptable to different environments or hosts.
1-23: Overall, great job on implementing the MetaResource interface!The
MetaResourceinterface is well-structured and follows Kotlin best practices. It provides clear, focused methods for accessing OAuth-related endpoints. The code is clean, readable, and properly documented.To further improve the interface:
- Consider generalizing the URLs in the KDoc comments as suggested earlier.
- You might want to add some additional documentation about how this interface fits into the larger API structure, if applicable.
If you'd like any assistance in implementing these suggestions or have any questions about OAuth implementation details, feel free to ask!
core/src/jvmTest/kotlin/work/socialhub/kbsky/meta/MetaTest.kt (1)
1-28: Overall: Good start, but needs improvements in test methods.The
MetaTestclass provides a good foundation for testing the meta-related functionality. However, there are some areas for improvement:
- Both test methods lack assertions, which are crucial for automated testing.
- Console output is used instead of proper test validation.
- The second test method name could be more descriptive.
Next steps:
- Implement the suggested improvements for both test methods.
- Consider adding more test cases to cover different scenarios or edge cases.
- Ensure that the
AbstractTestclass provides all necessary setup for these tests.These changes will significantly improve the reliability and maintainability of your test suite.
If you need any help implementing these improvements or want to discuss the test strategy further, please let me know.
core/src/jvmTest/kotlin/work/socialhub/kbsky/com/atproto/server/CreateSessionTest.kt (1)
31-32: LGTM! Consider enhancing the debug output.The addition of printing the service endpoint aligns well with the PR objective of adding well-known endpoints. It enhances the test by providing more information about the created session without altering existing functionality.
Consider adding a descriptive label to the printed service endpoint for improved readability:
-println(response.data.didDoc?.asDIDDetails?.service?.get(0)?.serviceEndpoint) +println("Service Endpoint: ${response.data.didDoc?.asDIDDetails?.service?.get(0)?.serviceEndpoint}")This change would make the debug output more self-explanatory.
core/src/commonMain/kotlin/work/socialhub/kbsky/internal/meta/_MetaResource.kt (1)
1-12: Organize imports and remove unused onesSome imports may not be necessary or could be organized for better readability.
Consider reviewing the imports to ensure only necessary ones are included and they are properly grouped.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- core/src/commonMain/kotlin/work/socialhub/kbsky/ATProtocol.kt (1 hunks)
- core/src/commonMain/kotlin/work/socialhub/kbsky/ATProtocolConfig.kt (1 hunks)
- core/src/commonMain/kotlin/work/socialhub/kbsky/api/entity/meta/WellKnownOAuthAuthorizationServer.kt (1 hunks)
- core/src/commonMain/kotlin/work/socialhub/kbsky/api/entity/meta/WellKnownOAuthProtectedResourceResponse.kt (1 hunks)
- core/src/commonMain/kotlin/work/socialhub/kbsky/api/meta/MetaResource.kt (1 hunks)
- core/src/commonMain/kotlin/work/socialhub/kbsky/internal/_ATProtocol.kt (3 hunks)
- core/src/commonMain/kotlin/work/socialhub/kbsky/internal/meta/_MetaResource.kt (1 hunks)
- core/src/jvmTest/kotlin/work/socialhub/kbsky/com/atproto/server/CreateSessionTest.kt (1 hunks)
- core/src/jvmTest/kotlin/work/socialhub/kbsky/meta/MetaTest.kt (1 hunks)
🧰 Additional context used
📓 Learnings (2)
core/src/commonMain/kotlin/work/socialhub/kbsky/ATProtocolConfig.kt (1)
Learnt from: uakihir0 PR: uakihir0/kbsky#67 File: core/src/commonMain/kotlin/work/socialhub/kbsky/internal/_ATProtocol.kt:12-18 Timestamp: 2024-07-04T10:09:32.037Z Learning: The `_ATProtocol` class now uses `ATProtocolConfig` in its constructor instead of a URI string. Ensure all instances of `_ATProtocol` are updated accordingly.core/src/commonMain/kotlin/work/socialhub/kbsky/internal/_ATProtocol.kt (1)
Learnt from: uakihir0 PR: uakihir0/kbsky#67 File: core/src/commonMain/kotlin/work/socialhub/kbsky/internal/_ATProtocol.kt:12-18 Timestamp: 2024-07-04T10:09:32.037Z Learning: The `_ATProtocol` class now uses `ATProtocolConfig` in its constructor instead of a URI string. Ensure all instances of `_ATProtocol` are updated accordingly.
🔇 Additional comments (16)
core/src/commonMain/kotlin/work/socialhub/kbsky/ATProtocol.kt (3)
6-6: LGTM: Import statement for MetaResourceThe import statement for
MetaResourceis correctly added and follows the existing import pattern for other resources in the file.
Line range hint
1-13: Overall assessment: Well-structured addition of meta endpointThe changes in this file are concise and well-structured. The addition of the
meta()method to theATProtocolinterface, along with the corresponding import, effectively expands the API to include metadata resources. This aligns perfectly with the PR objective of adding well-known endpoints.A few points to consider:
- Ensure that the
MetaResourceclass is properly implemented and tested.- Update any existing documentation to reflect this new capability.
- If there are any client implementations of
ATProtocol, make sure they are updated to include this new method.Great job on maintaining consistency with the existing codebase structure!
13-13: LGTM: New meta() method addedThe
meta()method is a good addition to theATProtocolinterface. It follows the same pattern as other resource methods and aligns with the PR objective of adding well-known endpoints.To ensure consistency across the codebase, let's verify the usage and implementation of this new method:
✅ Verification successful
Verification Successful:
meta()method is properly implemented and used within the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of the new meta() method # Check for the implementation of meta() method echo "Checking for meta() method implementation:" rg --type kotlin "override fun meta\(\).*MetaResource" -g '!ATProtocol.kt' # Check for usage of meta() method echo "Checking for meta() method usage:" rg --type kotlin "\.meta\(\)" -g '!ATProtocol.kt'Length of output: 464
core/src/commonMain/kotlin/work/socialhub/kbsky/ATProtocolConfig.kt (2)
12-15: LGTM: Addition ofauthUripropertyThe new
authUriproperty is a good addition, allowing separate configuration of the authorization server URI. The default value and KDoc comment are appropriate.
Line range hint
1-27: Verify impact on dependent classesThe additions to
ATProtocolConfiglook good and enhance its flexibility. However, given that_ATProtocolnow usesATProtocolConfigin its constructor (as per the retrieved learning), we should verify that these changes don't introduce any inconsistencies or breaking changes in_ATProtocolor other dependent classes.Let's run a script to check for potential impacts:
This script will help us identify any potential areas that might need updates due to these changes.
✅ Verification successful
Impact on dependent classes verified
The changes to
ATProtocolConfighave been reviewed, and no inconsistencies or breaking changes have been found in_ATProtocolor other dependent classes.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of ATProtocolConfig and potential impacts of the new properties # Search for ATProtocolConfig usage echo "Searching for ATProtocolConfig usage:" rg --type kotlin "ATProtocolConfig" -C 3 # Search for authUri references echo "\nSearching for authUri references:" rg --type kotlin "authUri" -C 3 # Search for updateAuthUri references echo "\nSearching for updateAuthUri references:" rg --type kotlin "updateAuthUri" -C 3 # Search for _ATProtocol class echo "\nSearching for _ATProtocol class definition:" rg --type kotlin "class _ATProtocol" -C 5Length of output: 20614
core/src/commonMain/kotlin/work/socialhub/kbsky/api/entity/meta/WellKnownOAuthProtectedResourceResponse.kt (1)
1-4: LGTM: Package declaration and imports are correct.The package declaration follows the standard Kotlin naming convention, and the necessary imports for serialization are correctly specified.
core/src/commonMain/kotlin/work/socialhub/kbsky/api/meta/MetaResource.kt (2)
1-5: LGTM: Package declaration and imports are correct.The package declaration and imports are well-structured and relevant to the interface's functionality. Good job on following Kotlin naming conventions and avoiding unused imports.
7-7: LGTM: Interface declaration is appropriate.The
MetaResourceinterface is well-named and follows Kotlin naming conventions. Its public visibility is suitable for an API resource interface.core/src/jvmTest/kotlin/work/socialhub/kbsky/meta/MetaTest.kt (2)
1-7: LGTM: Package declaration and imports are appropriate.The package declaration and imports are correct and relevant for the test class. All necessary imports are present, and there are no unused imports.
9-10: LGTM: Class declaration is correct.The
MetaTestclass is properly declared and extendsAbstractTest, which is likely providing common setup or utility methods for tests.core/src/commonMain/kotlin/work/socialhub/kbsky/internal/_ATProtocol.kt (4)
8-8: LGTM: New imports added correctly.The new import statements for
MetaResourceand_MetaResourceare correctly added and follow the existing import pattern in the file. These imports are necessary for the newmetaproperty andmeta()method implementation.Also applies to: 12-12
21-21: LGTM: Newmetaproperty added correctly.The new protected
metaproperty of typeMetaResourceis correctly added and initialized. It follows the pattern of other resource properties in the class and uses theconfigparameter, which aligns with the learning from a previous PR about_ATProtocolusingATProtocolConfig.
38-41: LGTM: Newmeta()method implemented correctly.The new
meta()method is correctly implemented:
- It's properly marked with the
overridekeyword.- It returns the
metaproperty as expected.- The Javadoc comment using
{@inheritDoc}is consistent with other methods in the class.- The implementation follows the pattern of other resource getter methods in the class.
Line range hint
1-41: Summary: Changes align with PR objectives and enhance_ATProtocolfunctionality.The changes in this file successfully add support for well-known endpoints related to meta resources:
- New imports for
MetaResourceand_MetaResourceare added.- A new protected
metaproperty is introduced, initialized with_MetaResource(config).- A new
meta()method is implemented, providing access to themetaproperty.These changes enhance the functionality of the
_ATProtocolclass by integrating meta-related capabilities, aligning well with the PR objectives of adding well-known endpoints. The implementation follows existing patterns in the codebase and maintains consistency with other similar resources.core/src/commonMain/kotlin/work/socialhub/kbsky/internal/meta/_MetaResource.kt (1)
27-31: Consider thread safety when updatingconfig.authUriModifying
config.authUrimight lead to concurrency issues ifconfigis accessed from multiple threads. Ensure that the configuration object is safely mutable or consider making it immutable.Verify if
configis accessed concurrently elsewhere. If so, consider synchronizing access or using thread-safe structures.core/src/commonMain/kotlin/work/socialhub/kbsky/api/entity/meta/WellKnownOAuthAuthorizationServer.kt (1)
6-7: Class definition and serialization setup look goodThe use of the
@Serializableannotation and class declaration are correct.
| @SerialName("authorization_servers") | ||
| lateinit var authorizationServers: List<String> |
There was a problem hiding this comment.
Revise the authorizationServers property declaration.
The authorizationServers property is declared as lateinit var with a List<String> type. This can cause issues because List is an interface and can't be initialized later with lateinit.
Consider one of these alternatives:
- If the field is optional:
@SerialName("authorization_servers")
var authorizationServers: List<String>? = null- If the field should always have a value (even if empty):
@SerialName("authorization_servers")
var authorizationServers: List<String> = emptyList()Choose the option that best fits the API's behavior.
| @SerialName("scopes_supported") | ||
| lateinit var scopesSupported: List<String> |
There was a problem hiding this comment.
Revise the scopesSupported property declaration.
The scopesSupported property has the same issue as authorizationServers: it's declared as lateinit var with a List<String> type.
Consider one of these alternatives:
- If the field is optional:
@SerialName("scopes_supported")
var scopesSupported: List<String>? = null- If the field should always have a value (even if empty):
@SerialName("scopes_supported")
var scopesSupported: List<String> = emptyList()Choose the option that best fits the API's behavior.
| @SerialName("bearer_methods_supported") | ||
| lateinit var bearerMethodsSupported: List<String> |
There was a problem hiding this comment.
Revise the bearerMethodsSupported property declaration.
The bearerMethodsSupported property has the same issue as the previous List<String> properties: it's declared as lateinit var with a List<String> type.
Consider one of these alternatives:
- If the field is optional:
@SerialName("bearer_methods_supported")
var bearerMethodsSupported: List<String>? = null- If the field should always have a value (even if empty):
@SerialName("bearer_methods_supported")
var bearerMethodsSupported: List<String> = emptyList()Choose the option that best fits the API's behavior.
| @SerialName("resource") | ||
| lateinit var resource: String |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a nullable type for resource if it's optional.
The resource property is currently declared as a non-null String with lateinit. If this field is optional in the API response, consider using a nullable type instead.
Here's a suggested change if the field is optional:
@SerialName("resource")
-lateinit var resource: String
+var resource: String? = nullIf the field is always required, the current implementation is fine.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @SerialName("resource") | |
| lateinit var resource: String | |
| @SerialName("resource") | |
| var resource: String? = null |
| @SerialName("resource_documentation") | ||
| lateinit var resourceDocumentation: String |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a nullable type for resourceDocumentation if it's optional.
The resourceDocumentation property is currently declared as a non-null String with lateinit. If this field is optional in the API response, consider using a nullable type instead.
Here's a suggested change if the field is optional:
@SerialName("resource_documentation")
-lateinit var resourceDocumentation: String
+var resourceDocumentation: String? = nullIf the field is always required, the current implementation is fine.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @SerialName("resource_documentation") | |
| lateinit var resourceDocumentation: String | |
| @SerialName("resource_documentation") | |
| var resourceDocumentation: String? = null |
| override fun wellKnownOAuthAuthorizationServer() | ||
| : Response<WellKnownOAuthAuthorizationServer> { | ||
| return proceed { | ||
| runBlocking { | ||
| HttpRequest() | ||
| .url("${config.pdsUri}.well-known/oauth-authorization-server") | ||
| .accept(MediaType.JSON) | ||
| .get() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to avoid blocking calls with runBlocking
Similar to the previous method, using runBlocking can block the thread. Refactor the method to be suspend and remove runBlocking to improve performance and scalability.
Apply this diff:
- override fun wellKnownOAuthAuthorizationServer()
+ override suspend fun wellKnownOAuthAuthorizationServer()
: Response<WellKnownOAuthAuthorizationServer> {
return proceed {
- runBlocking {
HttpRequest()
.url("${config.pdsUri}.well-known/oauth-authorization-server")
.accept(MediaType.JSON)
.get()
- }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override fun wellKnownOAuthAuthorizationServer() | |
| : Response<WellKnownOAuthAuthorizationServer> { | |
| return proceed { | |
| runBlocking { | |
| HttpRequest() | |
| .url("${config.pdsUri}.well-known/oauth-authorization-server") | |
| .accept(MediaType.JSON) | |
| .get() | |
| } | |
| } | |
| } | |
| override suspend fun wellKnownOAuthAuthorizationServer() | |
| : Response<WellKnownOAuthAuthorizationServer> { | |
| return proceed { | |
| HttpRequest() | |
| .url("${config.pdsUri}.well-known/oauth-authorization-server") | |
| .accept(MediaType.JSON) | |
| .get() | |
| } | |
| } |
| var authUri = it.data.authorizationServers[0] | ||
| if (!authUri.endsWith("/")) authUri += "/" | ||
| config.authUri = authUri |
There was a problem hiding this comment.
Handle potential null or empty authorizationServers list
Accessing authorizationServers[0] without checking if the list is null or empty can result in a NullPointerException or IndexOutOfBoundsException. Ensure that the list contains elements before accessing the first item.
Apply this diff to add a safety check:
if (config.updateAuthUri) {
- var authUri = it.data.authorizationServers[0]
- if (!authUri.endsWith("/")) authUri += "/"
- config.authUri = authUri
+ val servers = it.data.authorizationServers
+ if (!servers.isNullOrEmpty()) {
+ var authUri = servers[0]
+ if (!authUri.endsWith("/")) authUri += "/"
+ config.authUri = authUri
+ config.authUri = authUri
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var authUri = it.data.authorizationServers[0] | |
| if (!authUri.endsWith("/")) authUri += "/" | |
| config.authUri = authUri | |
| val servers = it.data.authorizationServers | |
| if (!servers.isNullOrEmpty()) { | |
| var authUri = servers[0] | |
| if (!authUri.endsWith("/")) authUri += "/" | |
| config.authUri = authUri | |
| } |
| override fun wellKnownOAuthProtectedResource() | ||
| : Response<WellKnownOAuthProtectedResourceResponse> { | ||
| return proceed<WellKnownOAuthProtectedResourceResponse> { | ||
| runBlocking { | ||
| HttpRequest() | ||
| .url("${config.pdsUri}.well-known/oauth-protected-resource") | ||
| .accept(MediaType.JSON) | ||
| .get() | ||
| } | ||
| }.also { | ||
| if (config.updateAuthUri) { | ||
| var authUri = it.data.authorizationServers[0] | ||
| if (!authUri.endsWith("/")) authUri += "/" | ||
| config.authUri = authUri | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to avoid blocking calls with runBlocking
Using runBlocking in non-suspending functions can lead to thread blocking and potential performance issues. Consider marking the function as suspend and removing runBlocking to utilize Kotlin's coroutines effectively.
Apply this diff to refactor the method:
- override fun wellKnownOAuthProtectedResource()
+ override suspend fun wellKnownOAuthProtectedResource()
: Response<WellKnownOAuthProtectedResourceResponse> {
return proceed<WellKnownOAuthProtectedResourceResponse> {
- runBlocking {
HttpRequest()
.url("${config.pdsUri}.well-known/oauth-protected-resource")
.accept(MediaType.JSON)
.get()
- }
}.also {
if (config.updateAuthUri) {
var authUri = it.data.authorizationServers[0]
if (!authUri.endsWith("/")) authUri += "/"
config.authUri = authUri
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override fun wellKnownOAuthProtectedResource() | |
| : Response<WellKnownOAuthProtectedResourceResponse> { | |
| return proceed<WellKnownOAuthProtectedResourceResponse> { | |
| runBlocking { | |
| HttpRequest() | |
| .url("${config.pdsUri}.well-known/oauth-protected-resource") | |
| .accept(MediaType.JSON) | |
| .get() | |
| } | |
| }.also { | |
| if (config.updateAuthUri) { | |
| var authUri = it.data.authorizationServers[0] | |
| if (!authUri.endsWith("/")) authUri += "/" | |
| config.authUri = authUri | |
| } | |
| } | |
| } | |
| override suspend fun wellKnownOAuthProtectedResource() | |
| : Response<WellKnownOAuthProtectedResourceResponse> { | |
| return proceed<WellKnownOAuthProtectedResourceResponse> { | |
| HttpRequest() | |
| .url("${config.pdsUri}.well-known/oauth-protected-resource") | |
| .accept(MediaType.JSON) | |
| .get() | |
| }.also { | |
| if (config.updateAuthUri) { | |
| var authUri = it.data.authorizationServers[0] | |
| if (!authUri.endsWith("/")) authUri += "/" | |
| config.authUri = authUri | |
| } | |
| } | |
| } |
| HttpRequest() | ||
| .url("${config.pdsUri}.well-known/oauth-authorization-server") | ||
| .accept(MediaType.JSON) | ||
| .get() |
There was a problem hiding this comment.
Improve error handling for HTTP requests
Currently, there is no error handling for the HTTP requests made using HttpRequest().get(). Consider adding exception handling to manage network errors or unexpected responses gracefully.
Apply this diff to add basic error handling:
HttpRequest()
.url("${config.pdsUri}.well-known/oauth-authorization-server")
.accept(MediaType.JSON)
- .get()
+ .get().also { response ->
+ if (!response.isSuccessful) {
+ // Handle error scenario
+ throw Exception("Failed to fetch OAuth authorization server information")
+ }
+ }Committable suggestion was skipped due to low confidence.
| class WellKnownOAuthAuthorizationServer { | ||
|
|
||
| @SerialName("issuer") | ||
| lateinit var issuer: String |
There was a problem hiding this comment.
Avoid using lateinit var for properties in serializable classes
Using lateinit var for properties in serializable classes can lead to runtime exceptions if the properties are accessed before initialization. It's safer to use nullable types or provide default values for the properties to avoid potential UninitializedPropertyAccessExceptions during deserialization.
Apply this diff to replace lateinit var with nullable properties:
- lateinit var issuer: String
+ var issuer: String? = null
- lateinit var scopesSupported: List<String>
+ var scopesSupported: List<String>? = null
// Repeat similar changes for other propertiesAlso applies to: 13-13, 16-16, 19-19, 22-22, 25-25, 28-28, 31-31, 34-34, 40-40, 43-43, 46-46, 58-58, 61-61, 64-64, 67-67, 70-70, 73-73, 76-76, 79-79, 85-85
|
うーんこれ、ATProtocol に含めずに、別のクラスで定義したほうがいいだろうか？ |
|
auth パッケージを切り直して作り直すのでここはクローズ |
Summary by CodeRabbit
New Features
meta()function in theATProtocolinterface for accessing metadata resources.ATProtocolConfigfor managing authorization server URIs.Bug Fixes
Tests
MetaTestclass with tests for OAuth protected resources and authorization servers.