diff --git a/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientException.java b/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientException.java index 9cf7aace1..a12779c67 100644 --- a/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientException.java +++ b/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientException.java @@ -1,12 +1,23 @@ package org.patinanetwork.codebloom.common.leetcode; +import lombok.Getter; + +@Getter public class LeetcodeClientException extends RuntimeException { + private final boolean notFound; + public LeetcodeClientException(final String message) { + this(message, false); + } + + public LeetcodeClientException(final String message, final boolean notFound) { super(message); + this.notFound = notFound; } public LeetcodeClientException(final String message, final Throwable e) { super(message, e); + this.notFound = false; } } diff --git a/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientImpl.java b/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientImpl.java index db125de88..4cbc61164 100644 --- a/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientImpl.java +++ b/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientImpl.java @@ -159,6 +159,14 @@ public LeetcodeQuestion findQuestionBySlug(final String slug) { JsonNode node = mapper.readTree(body); JsonNode questionNode = node.path("data").path("question"); + if (!questionNode.isObject() + && node.hasNonNull("errors") + && !node.path("errors").isEmpty()) { + throw new LeetcodeClientException("LeetCode returned GraphQL errors for slug " + slug); + } + if (node.path("data").isObject() && node.path("data").has("question") && questionNode.isNull()) { + throw new LeetcodeClientException("LeetCode returned no question for slug " + slug, true); + } if (!questionNode.isObject()) { throw new LeetcodeClientException("LeetCode returned no question data"); } @@ -205,6 +213,12 @@ public LeetcodeQuestion findQuestionBySlug(final String slug) { .acceptanceRate(acRate) .topics(tags) .build(); + } catch (LeetcodeClientException e) { + if (e.isNotFound()) { + throw e; + } + errorCounter().increment(); + throw new LeetcodeClientException("Error fetching the API", e); } catch (InterruptedException e) { errorCounter().increment(); Thread.currentThread().interrupt(); diff --git a/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestion.java b/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestion.java index 8ac7d4eae..e65ebd4f3 100644 --- a/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestion.java +++ b/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestion.java @@ -9,6 +9,7 @@ import org.patinanetwork.codebloom.common.db.repos.question.QuestionRepository; import org.patinanetwork.codebloom.common.db.repos.question.topic.QuestionTopicRepository; import org.patinanetwork.codebloom.common.leetcode.LeetcodeClient; +import org.patinanetwork.codebloom.common.leetcode.LeetcodeClientException; import org.patinanetwork.codebloom.common.leetcode.models.LeetcodeQuestion; import org.patinanetwork.codebloom.common.leetcode.throttled.ThrottledLeetcodeClient; import org.springframework.context.annotation.Profile; @@ -44,12 +45,23 @@ void attachTagsToExistingQuestions() { } for (var question : questions) { - log.info("Updating question with id of {}", question.getId()); + log.info("Updating question with id of {} and slug {}", question.getId(), question.getQuestionSlug()); LeetcodeQuestion leetcodeQuestion; try { leetcodeQuestion = leetcodeClient.findQuestionBySlug(question.getQuestionSlug()); } catch (Exception e) { - log.error("LeetcodeClient threw an exception", e); + if (e instanceof LeetcodeClientException clientException && clientException.isNotFound()) { + log.info( + "Skipping topic lookup for question id {} and slug {} because it was not found", + question.getId(), + question.getQuestionSlug()); + } else { + log.error( + "LeetcodeClient threw an exception for question id {} and slug {}", + question.getId(), + question.getQuestionSlug(), + e); + } continue; } @@ -64,6 +76,11 @@ void attachTagsToExistingQuestions() { questionTopicRepository.createQuestionTopic(newQuestionTopic); } + log.info( + "Attached {} topics to question id {} and slug {}", + leetcodeQuestion.getTopics().size(), + question.getId(), + question.getQuestionSlug()); } log.info("This task is complete."); diff --git a/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessService.java b/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessService.java index 762d8d8bd..0aba00e7d 100644 --- a/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessService.java +++ b/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessService.java @@ -33,7 +33,7 @@ @Profile("!ci | thread") public class LeetcodeQuestionProcessService { - private static final ReentrantLock LOCK = new ReentrantLock(); + private final ReentrantLock lock = new ReentrantLock(); private static final int MAX_JOBS_PER_RUN = 10; private static final long REQUESTS_OVER_TIME = 1L; @@ -89,7 +89,7 @@ private List claimBatch(final int maxSize) { @Scheduled(initialDelay = 0, fixedDelay = 30, timeUnit = TimeUnit.MINUTES) @Async public CompletableFuture drainQueue() { - if (!LOCK.tryLock()) { + if (!lock.tryLock()) { log.info("thread attempted to drain queue, but queue is already being drained."); return CompletableFuture.completedFuture(Empty.of()); } @@ -119,7 +119,7 @@ public CompletableFuture drainQueue() { } } } finally { - LOCK.unlock(); + lock.unlock(); } return CompletableFuture.completedFuture(Empty.of()); } diff --git a/src/test/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientTest.java b/src/test/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientTest.java index 6f491f35e..08cfec1ab 100644 --- a/src/test/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientTest.java +++ b/src/test/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientTest.java @@ -50,6 +50,50 @@ void setup() { when(leetcodeAuthStealer.getCsrf()).thenReturn(null); } + @Test + void explicitNullQuestionIsNotFound() throws Exception { + when(httpResponse.statusCode()).thenReturn(200); + when(httpResponse.body()).thenReturn("{\"data\":{\"question\":null}}"); + when(httpClient.send(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))) + .thenReturn(httpResponse); + + var error = assertThrows(LeetcodeClientException.class, () -> leetcodeClient.findQuestionBySlug("old-slug")); + assertTrue(error.isNotFound()); + assertNull(meterRegistry.find("leetcode.client.exception").counter()); + } + + @ParameterizedTest + @ValueSource( + strings = { + "{\"data\":{\"question\":null},\"errors\":[{\"message\":\"Unauthorized\"}]}", + "{\"data\":{}}", + "{\"data\":null}", + "invalid json" + }) + void unsuccessfulOrMalformedResponseIsNotNotFound(String body) throws Exception { + when(httpResponse.statusCode()).thenReturn(200); + when(httpResponse.body()).thenReturn(body); + when(httpClient.send(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))) + .thenReturn(httpResponse); + + var error = assertThrows(LeetcodeClientException.class, () -> leetcodeClient.findQuestionBySlug("example")); + assertFalse(error.isNotFound()); + assertEquals( + 1.0, meterRegistry.get("leetcode.client.exception").counter().count()); + } + + @ParameterizedTest + @ValueSource(ints = {403, 404, 429, 503}) + void httpFailureDoesNotPermanentlyExcludeQuestion(int status) throws Exception { + when(httpResponse.statusCode()).thenReturn(status); + when(httpResponse.body()).thenReturn("{\"data\":{\"question\":null}}"); + when(httpClient.send(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))) + .thenReturn(httpResponse); + + var error = assertThrows(LeetcodeClientException.class, () -> leetcodeClient.findQuestionBySlug("example")); + assertFalse(error.isNotFound()); + } + @Test void testFindQuestionBySlug() throws Exception { String responseJson = """ diff --git a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java index ad5d385d9..46eff5e89 100644 --- a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java +++ b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java @@ -17,6 +17,7 @@ import org.patinanetwork.codebloom.common.db.models.question.Question; import org.patinanetwork.codebloom.common.db.repos.question.QuestionRepository; import org.patinanetwork.codebloom.common.db.repos.question.topic.QuestionTopicRepository; +import org.patinanetwork.codebloom.common.leetcode.LeetcodeClientException; import org.patinanetwork.codebloom.common.leetcode.throttled.ThrottledLeetcodeClient; import org.patinanetwork.codebloom.common.time.StandardizedLocalDateTime; import org.slf4j.LoggerFactory; @@ -68,8 +69,64 @@ function hello() { .thenThrow(new RuntimeException("Expected!")); attachTagsToExistingQuestion.attachTagsToExistingQuestions(); + attachTagsToExistingQuestion.attachTagsToExistingQuestions(); + verify(leetcodeClient, times(2)).findQuestionBySlug(mockQuestion.getQuestionSlug()); + verifyNoInteractions(questionTopicRepository); assertTrue(logWatcher.list.stream() .anyMatch(log -> log.getLevel().equals(Level.ERROR) - && log.getFormattedMessage().contains("LeetcodeClient threw an exception"))); + && log.getFormattedMessage().contains("LeetcodeClient threw an exception") + && log.getFormattedMessage().contains(mockQuestion.getId()) + && log.getFormattedMessage().contains(mockQuestion.getQuestionSlug()))); + } + + @Test + void notFoundQuestionIsSkippedAndOtherQuestionsContinue() { + var missing = Question.builder().id("missing").questionSlug("old-slug").build(); + var valid = Question.builder().id("valid").questionSlug("valid-slug").build(); + when(questionRepository.getAllQuestionsWithNoTopics()).thenReturn(List.of(missing, valid)); + when(leetcodeClient.findQuestionBySlug("old-slug")) + .thenThrow(new LeetcodeClientException("Question not found", true)); + when(leetcodeClient.findQuestionBySlug("valid-slug")) + .thenReturn(org.patinanetwork.codebloom.common.leetcode.models.LeetcodeQuestion.builder() + .topics(List.of()) + .build()); + + attachTagsToExistingQuestion.attachTagsToExistingQuestions(); + attachTagsToExistingQuestion.attachTagsToExistingQuestions(); + + verify(leetcodeClient, times(2)).findQuestionBySlug("old-slug"); + verify(leetcodeClient, times(2)).findQuestionBySlug("valid-slug"); + verifyNoInteractions(questionTopicRepository); + assertTrue(logWatcher.list.stream() + .anyMatch(log -> log.getLevel().equals(Level.INFO) + && log.getFormattedMessage().contains("Skipping topic lookup for question id missing"))); + assertFalse(logWatcher.list.stream().anyMatch(log -> log.getLevel().equals(Level.ERROR))); + } + + @Test + void attachesTopicsAndContinuesPastFailedQuestion() { + var failed = Question.builder().id("failed").questionSlug("old-slug").build(); + var valid = Question.builder() + .id("valid") + .questionSlug("classes-with-at-least-5-students") + .build(); + when(questionRepository.getAllQuestionsWithNoTopics()).thenReturn(List.of(failed, valid)); + when(leetcodeClient.findQuestionBySlug("old-slug")).thenThrow(new RuntimeException("Missing")); + when(leetcodeClient.findQuestionBySlug(valid.getQuestionSlug())) + .thenReturn(org.patinanetwork.codebloom.common.leetcode.models.LeetcodeQuestion.builder() + .topics(List.of(org.patinanetwork.codebloom.common.leetcode.models.LeetcodeTopicTag.builder() + .name("Database") + .slug("database") + .build())) + .build()); + + attachTagsToExistingQuestion.attachTagsToExistingQuestions(); + + verify(questionTopicRepository) + .createQuestionTopic( + argThat(topic -> topic.getQuestionId().orElseThrow().equals("valid") + && topic.getTopicSlug().equals("database"))); + assertTrue(logWatcher.list.stream().anyMatch(log -> log.getFormattedMessage() + .contains("Attached 1 topics to question id valid and slug classes-with-at-least-5-students"))); } }