KTL-4174 Create a job to collect index requests from GH issues#306
Conversation
cebe075 to
8e094b1
Compare
| * Updated only when a polling run completes without any server errors | ||
| */ | ||
| @Column(name = "user_request_check_timestamp") | ||
| var userRequestCheckTimestamp: Instant |
There was a problem hiding this comment.
It is better to have a field called "log_type" or something similar, and to have two separate records in the DB for the Maven index update and the user request check.
| val since = mavenCentralLogRepository.retrieveUserRequestCheckTimestamp() | ||
| val runStartedAt = Instant.now() | ||
|
|
||
| // Get batch of issues from GitHub |
There was a problem hiding this comment.
Please don't leave excessive comments. Code should be self-documenting. Comment should be only in non-obvious places and should answer question "why", not "how".
Fix in other places as well please.
There was a problem hiding this comment.
I removed some comments, please let me know if you find any more of them that can be deleted
| @@ -0,0 +1,336 @@ | |||
| package io.klibs.app.indexing | |||
There was a problem hiding this comment.
move the service to io.klibs.app.service.impl
| } | ||
| } | ||
|
|
||
| internal data class ParsedRequest( |
There was a problem hiding this comment.
Let's move these data calsses to a separate files in io.klibs.app.dto package.
There was a problem hiding this comment.
I moved ParsedRequest to io.klibs.integration.maven.dto, but I left ProcessedRequestInfo in io.klibs.app.dto. Please let me know, if that's ok
| } | ||
| } | ||
|
|
||
| internal data class ParsedRequest( |
There was a problem hiding this comment.
It also could be named as MavenArtifactDto and lay in the io.klibs.integration.maven.dto
| * Updates the timestamp in the database only if all issues were processed | ||
| * without server-side errors. | ||
| */ | ||
| fun checkUserRequests() { |
There was a problem hiding this comment.
try to divide such big methods, the best if it fits one screen without scrolling.
| return | ||
| } | ||
|
|
||
| val issuesToProcess = issuesBatch.issues.mapNotNull { issue -> |
There was a problem hiding this comment.
Could be shortened to val issuesToProcess = issuesBatch.issues.associateWith { issue -> convertToValidMavenArtifact(issue) } if the whole body moves to a separate method
| * | ||
| * Returns null if the request is valid, or an error message if it is not | ||
| */ | ||
| internal fun validateRequest(parsed: ParsedRequest): String? { |
There was a problem hiding this comment.
This method could be extracted to a seperate MavenArtifactUtils class
| @@ -17,22 +17,22 @@ import org.springframework.transaction.annotation.Transactional | |||
| import org.springframework.web.server.ResponseStatusException | |||
|
|
|||
| @Service | |||
| class RequestIndexingService( | |||
| class UserRequestIndexingService( | |||
There was a problem hiding this comment.
Please move this service to io.klibs.app.service.impl package as well
7c16089 to
d51c715
Compare
8e094b1 to
c4033c7
Compare
d51c715 to
650588b
Compare
RequestIndexingServiceorScraperType.MANUAL_REQUEST) were inconsistent and could be confused with distinct parts of the project, likeIndexingRequestRepository. These occurrences were renamed touser requestsnow.io.klibs.integration.maven.search.impl.BaseMavenSearchClient#getRemoteFileUrlnow also processes untrusted data directly provided by users. That's why anURLBuilderwas added to it.UserRequestCheckService.findDuplicateIssueNumberfilters out duplicate requests within the current batch;UserRequestIndexingService.isAlreadyIndexedOrQueuedchecks if request is not already saved in the database.