From a30d80c0b5ff3b58a08ac452844dea4cc08d21b2 Mon Sep 17 00:00:00 2001 From: Nancy Huang <205217630+naanci@users.noreply.github.com> Date: Sun, 4 Oct 2026 20:30:25 -0400 Subject: [PATCH 1/9] TAN-989: migrate updated leetcode question in prod database, improve logging by adding question ID and slug --- .../V0082__Correct_renamed_question_596.SQL | 18 +++++++++++ .../AttachTagsToExistingQuestion.java | 13 ++++++-- .../AttachTagsToExistingQuestionTest.java | 32 ++++++++++++++++++- 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 db/migration/V0082__Correct_renamed_question_596.SQL diff --git a/db/migration/V0082__Correct_renamed_question_596.SQL b/db/migration/V0082__Correct_renamed_question_596.SQL new file mode 100644 index 000000000..826037a08 --- /dev/null +++ b/db/migration/V0082__Correct_renamed_question_596.SQL @@ -0,0 +1,18 @@ +DO $$ +BEGIN + CASE current_database() + WHEN 'codebloom-prod' THEN + UPDATE + "Question" + SET + "questionTitle" = 'Classes With at Least 5 Students', + "questionSlug" = 'classes-with-at-least-5-students', + "questionLink" = 'https://leetcode.com/problems/classes-with-at-least-5-students' + WHERE + id = '7ea283ec-2e03-4753-b8f8-406b3a68cbe1' + AND "questionNumber" = 596 + AND "questionSlug" = 'classes-more-than-5-students'; + ELSE + RAISE NOTICE 'Skipping prod only migration: Current database is %', current_database(); + END CASE; +END $$; 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..ed1053836 100644 --- a/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestion.java +++ b/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestion.java @@ -44,12 +44,16 @@ 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); + log.error( + "LeetcodeClient threw an exception for question id {} and slug {}", + question.getId(), + question.getQuestionSlug(), + e); continue; } @@ -64,6 +68,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/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java index ad5d385d9..623e12602 100644 --- a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java +++ b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java @@ -68,8 +68,38 @@ function hello() { .thenThrow(new RuntimeException("Expected!")); attachTagsToExistingQuestion.attachTagsToExistingQuestions(); + 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 correctedSlugAttachesTopicsAndContinuesPastFailedQuestion() { + var failed = Question.builder().id("failed").questionSlug("old-slug").build(); + var corrected = Question.builder() + .id("corrected") + .questionSlug("classes-with-at-least-5-students") + .build(); + when(questionRepository.getAllQuestionsWithNoTopics()).thenReturn(List.of(failed, corrected)); + when(leetcodeClient.findQuestionBySlug("old-slug")).thenThrow(new RuntimeException("Missing")); + when(leetcodeClient.findQuestionBySlug(corrected.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("corrected") + && topic.getTopicSlug().equals("database"))); + assertTrue(logWatcher.list.stream().anyMatch(log -> log.getFormattedMessage() + .contains("Attached 1 topics to question id corrected and slug classes-with-at-least-5-students"))); } } From eb140205b0aaea7fc3fd9bb0a600f4f73cd1b5b6 Mon Sep 17 00:00:00 2001 From: Nancy Huang <205217630+naanci@users.noreply.github.com> Date: Mon, 5 Oct 2026 01:31:46 -0400 Subject: [PATCH 2/9] TAN-989: skip topic lookup for obsolete submission preserve historical metadata --- ...2__Skip_topic_lookup_for_obsolete_question.SQL} | 9 +++++---- .../db/repos/question/QuestionSqlRepository.java | 3 ++- .../leetcode/AttachTagsToExistingQuestionTest.java | 14 +++++++------- 3 files changed, 14 insertions(+), 12 deletions(-) rename db/migration/{V0082__Correct_renamed_question_596.SQL => V0082__Skip_topic_lookup_for_obsolete_question.SQL} (51%) diff --git a/db/migration/V0082__Correct_renamed_question_596.SQL b/db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL similarity index 51% rename from db/migration/V0082__Correct_renamed_question_596.SQL rename to db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL index 826037a08..3d2454b1d 100644 --- a/db/migration/V0082__Correct_renamed_question_596.SQL +++ b/db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL @@ -1,3 +1,6 @@ +ALTER TABLE "Question" +ADD COLUMN "skipTopicLookup" BOOLEAN NOT NULL DEFAULT FALSE; + DO $$ BEGIN CASE current_database() @@ -5,14 +8,12 @@ BEGIN UPDATE "Question" SET - "questionTitle" = 'Classes With at Least 5 Students', - "questionSlug" = 'classes-with-at-least-5-students', - "questionLink" = 'https://leetcode.com/problems/classes-with-at-least-5-students' + "skipTopicLookup" = TRUE WHERE id = '7ea283ec-2e03-4753-b8f8-406b3a68cbe1' AND "questionNumber" = 596 AND "questionSlug" = 'classes-more-than-5-students'; ELSE - RAISE NOTICE 'Skipping prod only migration: Current database is %', current_database(); + RAISE NOTICE 'Skipping prod only update: Current database is %', current_database(); END CASE; END $$; diff --git a/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java b/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java index 3c3a7e494..3c6e3424d 100644 --- a/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java +++ b/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java @@ -487,7 +487,8 @@ public List getAllQuestionsWithNoTopics() { q."submissionId" FROM "Question" q - WHERE NOT EXISTS ( + WHERE NOT q."skipTopicLookup" + AND NOT EXISTS ( SELECT 1 FROM "QuestionTopic" qt WHERE qt."questionId" = q.id 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 623e12602..b19a55960 100644 --- a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java +++ b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java @@ -77,15 +77,15 @@ function hello() { } @Test - void correctedSlugAttachesTopicsAndContinuesPastFailedQuestion() { + void attachesTopicsAndContinuesPastFailedQuestion() { var failed = Question.builder().id("failed").questionSlug("old-slug").build(); - var corrected = Question.builder() - .id("corrected") + var valid = Question.builder() + .id("valid") .questionSlug("classes-with-at-least-5-students") .build(); - when(questionRepository.getAllQuestionsWithNoTopics()).thenReturn(List.of(failed, corrected)); + when(questionRepository.getAllQuestionsWithNoTopics()).thenReturn(List.of(failed, valid)); when(leetcodeClient.findQuestionBySlug("old-slug")).thenThrow(new RuntimeException("Missing")); - when(leetcodeClient.findQuestionBySlug(corrected.getQuestionSlug())) + 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") @@ -97,9 +97,9 @@ void correctedSlugAttachesTopicsAndContinuesPastFailedQuestion() { verify(questionTopicRepository) .createQuestionTopic( - argThat(topic -> topic.getQuestionId().orElseThrow().equals("corrected") + 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 corrected and slug classes-with-at-least-5-students"))); + .contains("Attached 1 topics to question id valid and slug classes-with-at-least-5-students"))); } } From 746b2228bd6a4e92668a04b205f6344813151821 Mon Sep 17 00:00:00 2001 From: Nancy Huang <205217630+naanci@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:19:21 -0400 Subject: [PATCH 3/9] TAN-989: continue topic lookup exlclusion for unavailable questions from LC --- ...kip_topic_lookup_for_obsolete_question.SQL | 17 ------- .../db/repos/question/QuestionRepository.java | 3 ++ .../repos/question/QuestionSqlRepository.java | 8 ++++ .../common/leetcode/LeetcodeClientImpl.java | 12 +++++ .../LeetcodeQuestionNotFoundException.java | 8 ++++ .../AttachTagsToExistingQuestion.java | 17 +++++++ .../question/QuestionRepositoryTest.java | 29 ++++++++++++ .../common/leetcode/LeetcodeClientTest.java | 28 +++++++++++ .../AttachTagsToExistingQuestionTest.java | 46 +++++++++++++++++++ 9 files changed, 151 insertions(+), 17 deletions(-) create mode 100644 src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeQuestionNotFoundException.java diff --git a/db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL b/db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL index 3d2454b1d..b33fb7bea 100644 --- a/db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL +++ b/db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL @@ -1,19 +1,2 @@ ALTER TABLE "Question" ADD COLUMN "skipTopicLookup" BOOLEAN NOT NULL DEFAULT FALSE; - -DO $$ -BEGIN - CASE current_database() - WHEN 'codebloom-prod' THEN - UPDATE - "Question" - SET - "skipTopicLookup" = TRUE - WHERE - id = '7ea283ec-2e03-4753-b8f8-406b3a68cbe1' - AND "questionNumber" = 596 - AND "questionSlug" = 'classes-more-than-5-students'; - ELSE - RAISE NOTICE 'Skipping prod only update: Current database is %', current_database(); - END CASE; -END $$; diff --git a/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepository.java b/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepository.java index 4fbd77e61..b7311312e 100644 --- a/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepository.java +++ b/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepository.java @@ -82,6 +82,9 @@ ArrayList getQuestionsByUserId( */ List getAllQuestionsWithNoTopics(); + /** Permanently excludes this submission from topic lookup without changing its historical metadata. */ + void skipTopicLookup(String questionId); + /** * @note - Returns all incomplete questions with user information, ordered by most recently submitted. Incomplete * questions are those missing either a runtime, memory, code, or language. diff --git a/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java b/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java index 3c6e3424d..b5e8b3364 100644 --- a/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java +++ b/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java @@ -464,6 +464,14 @@ AND NOT EXISTS (SELECT 1 FROM "QuestionBank" qb return new ArrayList<>(questions); } + @Override + public void skipTopicLookup(final String questionId) { + jdbcClient + .sql("UPDATE \"Question\" SET \"skipTopicLookup\" = TRUE WHERE id = :id") + .param("id", UUID.fromString(questionId)) + .update(); + } + @Override public List getAllQuestionsWithNoTopics() { String sql = """ 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..8d3d28340 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,15 @@ 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); + } + // Treat an explicit null question without GraphQL errors as unavailable. + if (node.path("data").isObject() && node.path("data").has("question") && questionNode.isNull()) { + throw new LeetcodeQuestionNotFoundException(slug); + } if (!questionNode.isObject()) { throw new LeetcodeClientException("LeetCode returned no question data"); } @@ -205,6 +214,9 @@ public LeetcodeQuestion findQuestionBySlug(final String slug) { .acceptanceRate(acRate) .topics(tags) .build(); + } catch (LeetcodeQuestionNotFoundException e) { + errorCounter().increment(); + throw e; } catch (InterruptedException e) { errorCounter().increment(); Thread.currentThread().interrupt(); diff --git a/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeQuestionNotFoundException.java b/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeQuestionNotFoundException.java new file mode 100644 index 000000000..b75e1f4ff --- /dev/null +++ b/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeQuestionNotFoundException.java @@ -0,0 +1,8 @@ +package org.patinanetwork.codebloom.common.leetcode; + +public class LeetcodeQuestionNotFoundException extends LeetcodeClientException { + + public LeetcodeQuestionNotFoundException(final String slug) { + super("LeetCode returned no question for slug " + slug); + } +} 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 ed1053836..ab117d35e 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.LeetcodeQuestionNotFoundException; import org.patinanetwork.codebloom.common.leetcode.models.LeetcodeQuestion; import org.patinanetwork.codebloom.common.leetcode.throttled.ThrottledLeetcodeClient; import org.springframework.context.annotation.Profile; @@ -48,6 +49,22 @@ void attachTagsToExistingQuestions() { LeetcodeQuestion leetcodeQuestion; try { leetcodeQuestion = leetcodeClient.findQuestionBySlug(question.getQuestionSlug()); + } catch (LeetcodeQuestionNotFoundException e) { + try { + questionRepository.skipTopicLookup(question.getId()); + } catch (Exception persistenceError) { + log.error( + "Failed to save topic lookup exclusion for question id {} and slug {}", + question.getId(), + question.getQuestionSlug(), + persistenceError); + continue; + } + log.info( + "Skipping future topic lookups for question id {} and slug {} because it was not found", + question.getId(), + question.getQuestionSlug()); + continue; } catch (Exception e) { log.error( "LeetcodeClient threw an exception for question id {} and slug {}", diff --git a/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java b/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java index c2d0b0a57..19237b66b 100644 --- a/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java +++ b/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java @@ -7,6 +7,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.UUID; import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -23,6 +24,7 @@ import org.patinanetwork.codebloom.common.time.StandardizedOffsetDateTime; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.jdbc.core.simple.JdbcClient; @SpringBootTest @TestInstance(TestInstance.Lifecycle.PER_CLASS) @@ -31,6 +33,10 @@ public class QuestionRepositoryTest extends BaseRepositoryTest { private QuestionRepository questionRepository; + + @Autowired + private JdbcClient jdbcClient; + private Question testQuestion; private String mockSuperUserId = "ed3bfe18-e42a-467f-b4fa-07e8da4d2555"; @@ -206,6 +212,29 @@ void testGetQuestionsWithNoTopics() { assertTrue(questions.size() > 0); } + @Test + @Order(8) + void skippedTopicLookupPreservesSubmissionAndExcludesFutureRuns() { + var before = questionRepository.getQuestionById(testQuestion.getId()).orElseThrow(); + assertTrue(questionRepository.getAllQuestionsWithNoTopics().stream() + .anyMatch(question -> question.getId().equals(testQuestion.getId()))); + try { + questionRepository.skipTopicLookup(testQuestion.getId()); + for (int run = 0; run < 2; run++) { + assertFalse(questionRepository.getAllQuestionsWithNoTopics().stream() + .anyMatch(question -> question.getId().equals(testQuestion.getId()))); + } + assertEquals( + before, + questionRepository.getQuestionById(testQuestion.getId()).orElseThrow()); + } finally { + jdbcClient + .sql("UPDATE \"Question\" SET \"skipTopicLookup\" = FALSE WHERE id = :id") + .param("id", UUID.fromString(testQuestion.getId())) + .update(); + } + } + @Test @Order(9) void testGetAllIncompleteQuestionsWithUser() { 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..5a85be127 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,34 @@ 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); + + assertThrows(LeetcodeQuestionNotFoundException.class, () -> leetcodeClient.findQuestionBySlug("old-slug")); + } + + @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 instanceof LeetcodeQuestionNotFoundException); + } + @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 b19a55960..ce9753b47 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.LeetcodeQuestionNotFoundException; import org.patinanetwork.codebloom.common.leetcode.throttled.ThrottledLeetcodeClient; import org.patinanetwork.codebloom.common.time.StandardizedLocalDateTime; import org.slf4j.LoggerFactory; @@ -69,6 +70,7 @@ function hello() { attachTagsToExistingQuestion.attachTagsToExistingQuestions(); verifyNoInteractions(questionTopicRepository); + verify(questionRepository, never()).skipTopicLookup(anyString()); assertTrue(logWatcher.list.stream() .anyMatch(log -> log.getLevel().equals(Level.ERROR) && log.getFormattedMessage().contains("LeetcodeClient threw an exception") @@ -76,6 +78,50 @@ function hello() { && log.getFormattedMessage().contains(mockQuestion.getQuestionSlug()))); } + @Test + void notFoundQuestionIsExcludedAndOtherQuestionsContinue() { + 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), List.of(valid)); + when(leetcodeClient.findQuestionBySlug("old-slug")) + .thenThrow(new LeetcodeQuestionNotFoundException("old-slug")); + when(leetcodeClient.findQuestionBySlug("valid-slug")) + .thenReturn(org.patinanetwork.codebloom.common.leetcode.models.LeetcodeQuestion.builder() + .topics(List.of()) + .build()); + + attachTagsToExistingQuestion.attachTagsToExistingQuestions(); + attachTagsToExistingQuestion.attachTagsToExistingQuestions(); + + verify(questionRepository).skipTopicLookup("missing"); + verify(leetcodeClient).findQuestionBySlug("old-slug"); + verify(leetcodeClient, times(2)).findQuestionBySlug("valid-slug"); + verifyNoInteractions(questionTopicRepository); + } + + @Test + void failedExclusionWriteDoesNotStopOtherLookups() { + 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 LeetcodeQuestionNotFoundException("old-slug")); + doThrow(new RuntimeException("Database unavailable")) + .when(questionRepository) + .skipTopicLookup("missing"); + when(leetcodeClient.findQuestionBySlug("valid-slug")) + .thenReturn(org.patinanetwork.codebloom.common.leetcode.models.LeetcodeQuestion.builder() + .topics(List.of()) + .build()); + + attachTagsToExistingQuestion.attachTagsToExistingQuestions(); + + verify(leetcodeClient).findQuestionBySlug("valid-slug"); + assertTrue(logWatcher.list.stream() + .anyMatch(log -> log.getLevel().equals(Level.ERROR) + && log.getFormattedMessage().contains("Failed to save topic lookup exclusion"))); + } + @Test void attachesTopicsAndContinuesPastFailedQuestion() { var failed = Question.builder().id("failed").questionSlug("old-slug").build(); From 1cf2b40ec6b00ffc22dfe85deef6ca50d843843c Mon Sep 17 00:00:00 2001 From: Nancy Huang <205217630+naanci@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:53:17 -0400 Subject: [PATCH 4/9] TAN-989: clean up and refind code for errors, exceptions, and tests --- .../leetcode/LeetcodeClientException.java | 11 ++++ .../common/leetcode/LeetcodeClientImpl.java | 10 +-- .../LeetcodeQuestionNotFoundException.java | 8 --- .../AttachTagsToExistingQuestion.java | 39 +++++------ .../question/QuestionRepositoryTest.java | 28 -------- .../QuestionTopicLookupIntegrationTest.java | 66 +++++++++++++++++++ .../common/leetcode/LeetcodeClientTest.java | 17 ++++- .../AttachTagsToExistingQuestionTest.java | 6 +- 8 files changed, 121 insertions(+), 64 deletions(-) delete mode 100644 src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeQuestionNotFoundException.java create mode 100644 src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionTopicLookupIntegrationTest.java 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 8d3d28340..a20330409 100644 --- a/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientImpl.java +++ b/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientImpl.java @@ -164,9 +164,8 @@ public LeetcodeQuestion findQuestionBySlug(final String slug) { && !node.path("errors").isEmpty()) { throw new LeetcodeClientException("LeetCode returned GraphQL errors for slug " + slug); } - // Treat an explicit null question without GraphQL errors as unavailable. if (node.path("data").isObject() && node.path("data").has("question") && questionNode.isNull()) { - throw new LeetcodeQuestionNotFoundException(slug); + throw new LeetcodeClientException("LeetCode returned no question for slug " + slug, true); } if (!questionNode.isObject()) { throw new LeetcodeClientException("LeetCode returned no question data"); @@ -214,9 +213,12 @@ public LeetcodeQuestion findQuestionBySlug(final String slug) { .acceptanceRate(acRate) .topics(tags) .build(); - } catch (LeetcodeQuestionNotFoundException e) { + } catch (LeetcodeClientException e) { errorCounter().increment(); - throw e; + if (e.isNotFound()) { + throw e; + } + 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/common/leetcode/LeetcodeQuestionNotFoundException.java b/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeQuestionNotFoundException.java deleted file mode 100644 index b75e1f4ff..000000000 --- a/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeQuestionNotFoundException.java +++ /dev/null @@ -1,8 +0,0 @@ -package org.patinanetwork.codebloom.common.leetcode; - -public class LeetcodeQuestionNotFoundException extends LeetcodeClientException { - - public LeetcodeQuestionNotFoundException(final String slug) { - super("LeetCode returned no question for slug " + slug); - } -} 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 ab117d35e..8d1272663 100644 --- a/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestion.java +++ b/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestion.java @@ -9,7 +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.LeetcodeQuestionNotFoundException; +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; @@ -49,28 +49,29 @@ void attachTagsToExistingQuestions() { LeetcodeQuestion leetcodeQuestion; try { leetcodeQuestion = leetcodeClient.findQuestionBySlug(question.getQuestionSlug()); - } catch (LeetcodeQuestionNotFoundException e) { - try { - questionRepository.skipTopicLookup(question.getId()); - } catch (Exception persistenceError) { + } catch (Exception e) { + if (e instanceof LeetcodeClientException clientException && clientException.isNotFound()) { + try { + questionRepository.skipTopicLookup(question.getId()); + } catch (Exception persistenceError) { + log.error( + "Failed to save topic lookup exclusion for question id {} and slug {}", + question.getId(), + question.getQuestionSlug(), + persistenceError); + continue; + } + log.info( + "Skipping future topic lookups for question id {} and slug {} because it was not found", + question.getId(), + question.getQuestionSlug()); + } else { log.error( - "Failed to save topic lookup exclusion for question id {} and slug {}", + "LeetcodeClient threw an exception for question id {} and slug {}", question.getId(), question.getQuestionSlug(), - persistenceError); - continue; + e); } - log.info( - "Skipping future topic lookups for question id {} and slug {} because it was not found", - question.getId(), - question.getQuestionSlug()); - continue; - } catch (Exception e) { - log.error( - "LeetcodeClient threw an exception for question id {} and slug {}", - question.getId(), - question.getQuestionSlug(), - e); continue; } diff --git a/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java b/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java index 19237b66b..680afabd1 100644 --- a/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java +++ b/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java @@ -7,7 +7,6 @@ import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.UUID; import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -24,7 +23,6 @@ import org.patinanetwork.codebloom.common.time.StandardizedOffsetDateTime; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.jdbc.core.simple.JdbcClient; @SpringBootTest @TestInstance(TestInstance.Lifecycle.PER_CLASS) @@ -34,9 +32,6 @@ public class QuestionRepositoryTest extends BaseRepositoryTest { private QuestionRepository questionRepository; - @Autowired - private JdbcClient jdbcClient; - private Question testQuestion; private String mockSuperUserId = "ed3bfe18-e42a-467f-b4fa-07e8da4d2555"; @@ -212,29 +207,6 @@ void testGetQuestionsWithNoTopics() { assertTrue(questions.size() > 0); } - @Test - @Order(8) - void skippedTopicLookupPreservesSubmissionAndExcludesFutureRuns() { - var before = questionRepository.getQuestionById(testQuestion.getId()).orElseThrow(); - assertTrue(questionRepository.getAllQuestionsWithNoTopics().stream() - .anyMatch(question -> question.getId().equals(testQuestion.getId()))); - try { - questionRepository.skipTopicLookup(testQuestion.getId()); - for (int run = 0; run < 2; run++) { - assertFalse(questionRepository.getAllQuestionsWithNoTopics().stream() - .anyMatch(question -> question.getId().equals(testQuestion.getId()))); - } - assertEquals( - before, - questionRepository.getQuestionById(testQuestion.getId()).orElseThrow()); - } finally { - jdbcClient - .sql("UPDATE \"Question\" SET \"skipTopicLookup\" = FALSE WHERE id = :id") - .param("id", UUID.fromString(testQuestion.getId())) - .update(); - } - } - @Test @Order(9) void testGetAllIncompleteQuestionsWithUser() { diff --git a/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionTopicLookupIntegrationTest.java b/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionTopicLookupIntegrationTest.java new file mode 100644 index 000000000..3d8f4c3f3 --- /dev/null +++ b/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionTopicLookupIntegrationTest.java @@ -0,0 +1,66 @@ +package org.patinanetwork.codebloom.common.db.repos.question; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.UUID; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; +import org.patinanetwork.codebloom.common.db.repos.question.topic.QuestionTopicRepository; +import org.patinanetwork.codebloom.common.db.repos.question.topic.service.QuestionTopicService; +import org.springframework.jdbc.core.simple.JdbcClient; +import org.springframework.jdbc.datasource.SingleConnectionDataSource; + +@EnabledIfEnvironmentVariable(named = "TOPIC_LOOKUP_TEST_URL", matches = ".+") +public class QuestionTopicLookupIntegrationTest { + + @Test + void exclusionPersistsWithoutChangingHistoricalMetadata() throws Exception { + var ds = new SingleConnectionDataSource(System.getenv("TOPIC_LOOKUP_TEST_URL"), true); + try { + var jdbc = JdbcClient.create(ds); + jdbc.sql(""" + CREATE TEMP TABLE "Question" ( + id uuid PRIMARY KEY, "userId" uuid, "questionSlug" text, + "questionDifficulty" text DEFAULT 'Easy', "questionNumber" smallint, + "questionLink" text, "pointsAwarded" integer, "questionTitle" text, + description text, "acceptanceRate" real DEFAULT 0, + "createdAt" timestamptz DEFAULT NOW(), "submittedAt" timestamptz DEFAULT NOW(), + runtime text, memory text, code text, language text, "submissionId" text + ) + """).update(); + jdbc.sql("CREATE TEMP TABLE \"QuestionTopic\" (\"questionId\" uuid)") + .update(); + jdbc.sql(Files.readString(Path.of("db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL"))) + .update(); + var missingId = UUID.randomUUID(); + var validId = UUID.randomUUID(); + for (var id : new UUID[] {missingId, validId}) { + jdbc.sql(""" + INSERT INTO "Question" (id, "questionNumber", "questionSlug", "questionTitle", + "questionLink", description, code) + VALUES (:id, 596, 'original-slug', 'Original title', 'original link', + 'original description', 'original code') + """).param("id", id).update(); + } + var repository = new QuestionSqlRepository( + ds, jdbc, mock(QuestionTopicRepository.class), mock(QuestionTopicService.class)); + var before = repository.getQuestionById(missingId.toString()).orElseThrow(); + assertEquals(2, repository.getAllQuestionsWithNoTopics().size()); + + repository.skipTopicLookup(missingId.toString()); + repository.skipTopicLookup(missingId.toString()); + for (int run = 0; run < 2; run++) { + var eligible = repository.getAllQuestionsWithNoTopics(); + assertEquals(1, eligible.size()); + assertEquals(validId.toString(), eligible.getFirst().getId()); + } + assertEquals( + before, repository.getQuestionById(missingId.toString()).orElseThrow()); + } finally { + ds.destroy(); + } + } +} 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 5a85be127..2d6ac8800 100644 --- a/src/test/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientTest.java +++ b/src/test/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientTest.java @@ -57,7 +57,8 @@ void explicitNullQuestionIsNotFound() throws Exception { when(httpClient.send(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))) .thenReturn(httpResponse); - assertThrows(LeetcodeQuestionNotFoundException.class, () -> leetcodeClient.findQuestionBySlug("old-slug")); + var error = assertThrows(LeetcodeClientException.class, () -> leetcodeClient.findQuestionBySlug("old-slug")); + assertTrue(error.isNotFound()); } @ParameterizedTest @@ -75,7 +76,19 @@ void unsuccessfulOrMalformedResponseIsNotNotFound(String body) throws Exception .thenReturn(httpResponse); var error = assertThrows(LeetcodeClientException.class, () -> leetcodeClient.findQuestionBySlug("example")); - assertFalse(error instanceof LeetcodeQuestionNotFoundException); + assertFalse(error.isNotFound()); + } + + @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 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 ce9753b47..576a6d0c8 100644 --- a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java +++ b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java @@ -17,7 +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.LeetcodeQuestionNotFoundException; +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; @@ -84,7 +84,7 @@ void notFoundQuestionIsExcludedAndOtherQuestionsContinue() { var valid = Question.builder().id("valid").questionSlug("valid-slug").build(); when(questionRepository.getAllQuestionsWithNoTopics()).thenReturn(List.of(missing, valid), List.of(valid)); when(leetcodeClient.findQuestionBySlug("old-slug")) - .thenThrow(new LeetcodeQuestionNotFoundException("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()) @@ -105,7 +105,7 @@ void failedExclusionWriteDoesNotStopOtherLookups() { 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 LeetcodeQuestionNotFoundException("old-slug")); + .thenThrow(new LeetcodeClientException("Question not found", true)); doThrow(new RuntimeException("Database unavailable")) .when(questionRepository) .skipTopicLookup("missing"); From f5c93abe413733bd56dd929372dd3221c0f4a754 Mon Sep 17 00:00:00 2001 From: Nancy Huang <205217630+naanci@users.noreply.github.com> Date: Mon, 5 Oct 2026 19:39:51 -0400 Subject: [PATCH 5/9] TAN-989: simplify client exception + remove db flag --- ...kip_topic_lookup_for_obsolete_question.SQL | 2 - .../db/repos/question/QuestionRepository.java | 3 - .../repos/question/QuestionSqlRepository.java | 11 +--- .../AttachTagsToExistingQuestion.java | 12 +--- .../QuestionTopicLookupIntegrationTest.java | 66 ------------------- .../AttachTagsToExistingQuestionTest.java | 33 ++-------- 6 files changed, 8 insertions(+), 119 deletions(-) delete mode 100644 db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL delete mode 100644 src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionTopicLookupIntegrationTest.java diff --git a/db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL b/db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL deleted file mode 100644 index b33fb7bea..000000000 --- a/db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL +++ /dev/null @@ -1,2 +0,0 @@ -ALTER TABLE "Question" -ADD COLUMN "skipTopicLookup" BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepository.java b/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepository.java index b7311312e..4fbd77e61 100644 --- a/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepository.java +++ b/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepository.java @@ -82,9 +82,6 @@ ArrayList getQuestionsByUserId( */ List getAllQuestionsWithNoTopics(); - /** Permanently excludes this submission from topic lookup without changing its historical metadata. */ - void skipTopicLookup(String questionId); - /** * @note - Returns all incomplete questions with user information, ordered by most recently submitted. Incomplete * questions are those missing either a runtime, memory, code, or language. diff --git a/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java b/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java index b5e8b3364..3c3a7e494 100644 --- a/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java +++ b/src/main/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionSqlRepository.java @@ -464,14 +464,6 @@ AND NOT EXISTS (SELECT 1 FROM "QuestionBank" qb return new ArrayList<>(questions); } - @Override - public void skipTopicLookup(final String questionId) { - jdbcClient - .sql("UPDATE \"Question\" SET \"skipTopicLookup\" = TRUE WHERE id = :id") - .param("id", UUID.fromString(questionId)) - .update(); - } - @Override public List getAllQuestionsWithNoTopics() { String sql = """ @@ -495,8 +487,7 @@ public List getAllQuestionsWithNoTopics() { q."submissionId" FROM "Question" q - WHERE NOT q."skipTopicLookup" - AND NOT EXISTS ( + WHERE NOT EXISTS ( SELECT 1 FROM "QuestionTopic" qt WHERE qt."questionId" = q.id 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 8d1272663..e65ebd4f3 100644 --- a/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestion.java +++ b/src/main/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestion.java @@ -51,18 +51,8 @@ void attachTagsToExistingQuestions() { leetcodeQuestion = leetcodeClient.findQuestionBySlug(question.getQuestionSlug()); } catch (Exception e) { if (e instanceof LeetcodeClientException clientException && clientException.isNotFound()) { - try { - questionRepository.skipTopicLookup(question.getId()); - } catch (Exception persistenceError) { - log.error( - "Failed to save topic lookup exclusion for question id {} and slug {}", - question.getId(), - question.getQuestionSlug(), - persistenceError); - continue; - } log.info( - "Skipping future topic lookups for question id {} and slug {} because it was not found", + "Skipping topic lookup for question id {} and slug {} because it was not found", question.getId(), question.getQuestionSlug()); } else { diff --git a/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionTopicLookupIntegrationTest.java b/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionTopicLookupIntegrationTest.java deleted file mode 100644 index 3d8f4c3f3..000000000 --- a/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionTopicLookupIntegrationTest.java +++ /dev/null @@ -1,66 +0,0 @@ -package org.patinanetwork.codebloom.common.db.repos.question; - -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.*; - -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.UUID; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; -import org.patinanetwork.codebloom.common.db.repos.question.topic.QuestionTopicRepository; -import org.patinanetwork.codebloom.common.db.repos.question.topic.service.QuestionTopicService; -import org.springframework.jdbc.core.simple.JdbcClient; -import org.springframework.jdbc.datasource.SingleConnectionDataSource; - -@EnabledIfEnvironmentVariable(named = "TOPIC_LOOKUP_TEST_URL", matches = ".+") -public class QuestionTopicLookupIntegrationTest { - - @Test - void exclusionPersistsWithoutChangingHistoricalMetadata() throws Exception { - var ds = new SingleConnectionDataSource(System.getenv("TOPIC_LOOKUP_TEST_URL"), true); - try { - var jdbc = JdbcClient.create(ds); - jdbc.sql(""" - CREATE TEMP TABLE "Question" ( - id uuid PRIMARY KEY, "userId" uuid, "questionSlug" text, - "questionDifficulty" text DEFAULT 'Easy', "questionNumber" smallint, - "questionLink" text, "pointsAwarded" integer, "questionTitle" text, - description text, "acceptanceRate" real DEFAULT 0, - "createdAt" timestamptz DEFAULT NOW(), "submittedAt" timestamptz DEFAULT NOW(), - runtime text, memory text, code text, language text, "submissionId" text - ) - """).update(); - jdbc.sql("CREATE TEMP TABLE \"QuestionTopic\" (\"questionId\" uuid)") - .update(); - jdbc.sql(Files.readString(Path.of("db/migration/V0082__Skip_topic_lookup_for_obsolete_question.SQL"))) - .update(); - var missingId = UUID.randomUUID(); - var validId = UUID.randomUUID(); - for (var id : new UUID[] {missingId, validId}) { - jdbc.sql(""" - INSERT INTO "Question" (id, "questionNumber", "questionSlug", "questionTitle", - "questionLink", description, code) - VALUES (:id, 596, 'original-slug', 'Original title', 'original link', - 'original description', 'original code') - """).param("id", id).update(); - } - var repository = new QuestionSqlRepository( - ds, jdbc, mock(QuestionTopicRepository.class), mock(QuestionTopicService.class)); - var before = repository.getQuestionById(missingId.toString()).orElseThrow(); - assertEquals(2, repository.getAllQuestionsWithNoTopics().size()); - - repository.skipTopicLookup(missingId.toString()); - repository.skipTopicLookup(missingId.toString()); - for (int run = 0; run < 2; run++) { - var eligible = repository.getAllQuestionsWithNoTopics(); - assertEquals(1, eligible.size()); - assertEquals(validId.toString(), eligible.getFirst().getId()); - } - assertEquals( - before, repository.getQuestionById(missingId.toString()).orElseThrow()); - } finally { - ds.destroy(); - } - } -} 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 576a6d0c8..f9fcd6b75 100644 --- a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java +++ b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java @@ -70,7 +70,6 @@ function hello() { attachTagsToExistingQuestion.attachTagsToExistingQuestions(); verifyNoInteractions(questionTopicRepository); - verify(questionRepository, never()).skipTopicLookup(anyString()); assertTrue(logWatcher.list.stream() .anyMatch(log -> log.getLevel().equals(Level.ERROR) && log.getFormattedMessage().contains("LeetcodeClient threw an exception") @@ -79,10 +78,10 @@ function hello() { } @Test - void notFoundQuestionIsExcludedAndOtherQuestionsContinue() { + 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), List.of(valid)); + 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")) @@ -93,33 +92,13 @@ void notFoundQuestionIsExcludedAndOtherQuestionsContinue() { attachTagsToExistingQuestion.attachTagsToExistingQuestions(); attachTagsToExistingQuestion.attachTagsToExistingQuestions(); - verify(questionRepository).skipTopicLookup("missing"); - verify(leetcodeClient).findQuestionBySlug("old-slug"); + verify(leetcodeClient, times(2)).findQuestionBySlug("old-slug"); verify(leetcodeClient, times(2)).findQuestionBySlug("valid-slug"); verifyNoInteractions(questionTopicRepository); - } - - @Test - void failedExclusionWriteDoesNotStopOtherLookups() { - 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)); - doThrow(new RuntimeException("Database unavailable")) - .when(questionRepository) - .skipTopicLookup("missing"); - when(leetcodeClient.findQuestionBySlug("valid-slug")) - .thenReturn(org.patinanetwork.codebloom.common.leetcode.models.LeetcodeQuestion.builder() - .topics(List.of()) - .build()); - - attachTagsToExistingQuestion.attachTagsToExistingQuestions(); - - verify(leetcodeClient).findQuestionBySlug("valid-slug"); assertTrue(logWatcher.list.stream() - .anyMatch(log -> log.getLevel().equals(Level.ERROR) - && log.getFormattedMessage().contains("Failed to save topic lookup exclusion"))); + .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 From 95971deecdd8faf6582568e498754f6122fbe5a7 Mon Sep 17 00:00:00 2001 From: Nancy Huang <205217630+naanci@users.noreply.github.com> Date: Mon, 5 Oct 2026 20:19:55 -0400 Subject: [PATCH 6/9] TAN-989: isolate queue lock per service (test fix) --- .../LeetcodeQuestionProcessService.java | 6 ++-- .../LeetcodeQuestionProcessServiceTest.java | 2 +- ...eetcodeQuestionProcessServiceUnitTest.java | 30 +++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) 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/scheduled/leetcode/LeetcodeQuestionProcessServiceTest.java b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceTest.java index 44217bcb0..8cf756ec2 100644 --- a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceTest.java +++ b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceTest.java @@ -177,7 +177,7 @@ void jobStatusTransitionValid() { @Test void drainQueueValid() { - service.drainQueue(); + service.drainQueue().join(); } // TODO: (TAN-32) re-enable diff --git a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceUnitTest.java b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceUnitTest.java index 9438ad0e8..9fb46c983 100644 --- a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceUnitTest.java +++ b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceUnitTest.java @@ -5,6 +5,8 @@ import java.util.List; import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -65,6 +67,34 @@ private void runQueue() { .join(); } + @Test + void independentServiceCanDrainWhileAnotherInstanceIsRunning() { + var otherJobs = mock(JobRepository.class); + when(otherJobs.findIncompleteJobs(10)).thenReturn(List.of()); + var otherService = new LeetcodeQuestionProcessService(otherJobs, client, questions, bank); + when(jobs.findIncompleteJobs(10)).thenAnswer(invocation -> { + CompletableFuture.runAsync(() -> otherService.drainQueue().join()).get(5, TimeUnit.SECONDS); + return List.of(); + }); + + runQueue(); + + verify(otherJobs).findIncompleteJobs(10); + } + + @Test + void sameServiceSkipsConcurrentDrain() { + var service = new LeetcodeQuestionProcessService(jobs, client, questions, bank); + when(jobs.findIncompleteJobs(10)).thenAnswer(invocation -> { + CompletableFuture.runAsync(() -> service.drainQueue().join()).get(5, TimeUnit.SECONDS); + return List.of(); + }); + + service.drainQueue().join(); + + verify(jobs).findIncompleteJobs(10); + } + @ParameterizedTest @NullAndEmptySource @ValueSource(strings = {" ", "\t\n"}) From d55111a63d76c7c6d9c48f4824dc5cd33cbbb83c Mon Sep 17 00:00:00 2001 From: Nancy Huang <205217630+naanci@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:32:49 -0400 Subject: [PATCH 7/9] TAN-989: resolve QueueLock priority test --- .../common/utils/lock/QueueLockTest.java | 92 ++++++++++--------- 1 file changed, 49 insertions(+), 43 deletions(-) diff --git a/src/test/java/org/patinanetwork/codebloom/common/utils/lock/QueueLockTest.java b/src/test/java/org/patinanetwork/codebloom/common/utils/lock/QueueLockTest.java index e7c0fbbb2..0ae118ae0 100644 --- a/src/test/java/org/patinanetwork/codebloom/common/utils/lock/QueueLockTest.java +++ b/src/test/java/org/patinanetwork/codebloom/common/utils/lock/QueueLockTest.java @@ -4,13 +4,11 @@ import static org.mockito.Mockito.*; import io.github.bucket4j.BlockingBucket; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -75,59 +73,67 @@ void acquireShouldAppendToEndOfQueue() throws InterruptedException { @Test @Timeout(value = 5, unit = TimeUnit.SECONDS) - void acquireFastShouldAppendToStartOfQueue() throws InterruptedException { - CountDownLatch tickerCallLatch = new CountDownLatch(1); - CountDownLatch releaseTicker = new CountDownLatch(1); + void acquireFastShouldAppendToStartOfQueue() throws Exception { + var tickerStarted = new CountDownLatch(1); + var releaseTicker = new CountDownLatch(1); + var releaseNormal = new CountDownLatch(1); + var normalSelected = new CountDownLatch(1); + var calls = new AtomicInteger(); doAnswer(invocation -> { - tickerCallLatch.countDown(); - releaseTicker.await(); + int call = calls.incrementAndGet(); + if (call == 1) { + tickerStarted.countDown(); + releaseTicker.await(); + } else if (call == 3) { + normalSelected.countDown(); + releaseNormal.await(); + } return null; }) .when(bucket) .consume(1); - executor.submit(() -> { - try { + try { + var initial = executor.submit(() -> { queueLock.acquire(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - }); - assertTrue(tickerCallLatch.await(2, TimeUnit.SECONDS)); - - List orderedCalls = Collections.synchronizedList(new ArrayList<>()); - CountDownLatch doneLatch = new CountDownLatch(2); + return null; + }); + assertTrue(tickerStarted.await(2, TimeUnit.SECONDS)); - executor.submit(() -> { - try { + var normal = executor.submit(() -> { queueLock.acquire(); - orderedCalls.add("T2"); - doneLatch.countDown(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - }); + return null; + }); + awaitQueueSize(1); - Thread.sleep(100); - - executor.submit(() -> { - try { + var fast = executor.submit(() -> { queueLock.acquireFast(); - orderedCalls.add("T3"); - doneLatch.countDown(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - }); - - Thread.sleep(100); - - releaseTicker.countDown(); + return null; + }); + awaitQueueSize(2); + + releaseTicker.countDown(); + fast.get(2, TimeUnit.SECONDS); + assertTrue(normalSelected.await(2, TimeUnit.SECONDS)); + assertFalse(normal.isDone(), "Normal request must wait until the fast request is released"); + + releaseNormal.countDown(); + normal.get(2, TimeUnit.SECONDS); + initial.get(2, TimeUnit.SECONDS); + verify(bucket, times(3)).consume(1); + } finally { + releaseTicker.countDown(); + releaseNormal.countDown(); + } + } - assertTrue(doneLatch.await(2, TimeUnit.SECONDS)); - assertEquals("T3", orderedCalls.get(0)); - assertEquals("T2", orderedCalls.get(1)); + private void awaitQueueSize(int expected) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(2); + while (queueLock.queue.size() != expected && System.nanoTime() < deadline) { + Thread.sleep(1); + } + assertEquals(expected, queueLock.queue.size(), "Requests did not enter the queue in time"); } @Test From 3787531ad6b3dc2b43a26c302f35e864602618dc Mon Sep 17 00:00:00 2001 From: Nancy Huang <205217630+naanci@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:48:07 -0400 Subject: [PATCH 8/9] TAN-989: exclude confirmed missing questions from client error metrics and some cleanup --- .../codebloom/common/leetcode/LeetcodeClientImpl.java | 2 +- .../common/db/repos/question/QuestionRepositoryTest.java | 1 - .../codebloom/common/leetcode/LeetcodeClientTest.java | 3 +++ .../scheduled/leetcode/AttachTagsToExistingQuestionTest.java | 2 ++ 4 files changed, 6 insertions(+), 2 deletions(-) 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 a20330409..4cbc61164 100644 --- a/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientImpl.java +++ b/src/main/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientImpl.java @@ -214,10 +214,10 @@ public LeetcodeQuestion findQuestionBySlug(final String slug) { .topics(tags) .build(); } catch (LeetcodeClientException e) { - errorCounter().increment(); if (e.isNotFound()) { throw e; } + errorCounter().increment(); throw new LeetcodeClientException("Error fetching the API", e); } catch (InterruptedException e) { errorCounter().increment(); diff --git a/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java b/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java index 680afabd1..c2d0b0a57 100644 --- a/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java +++ b/src/test/java/org/patinanetwork/codebloom/common/db/repos/question/QuestionRepositoryTest.java @@ -31,7 +31,6 @@ public class QuestionRepositoryTest extends BaseRepositoryTest { private QuestionRepository questionRepository; - private Question testQuestion; private String mockSuperUserId = "ed3bfe18-e42a-467f-b4fa-07e8da4d2555"; 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 2d6ac8800..08cfec1ab 100644 --- a/src/test/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientTest.java +++ b/src/test/java/org/patinanetwork/codebloom/common/leetcode/LeetcodeClientTest.java @@ -59,6 +59,7 @@ void explicitNullQuestionIsNotFound() throws Exception { var error = assertThrows(LeetcodeClientException.class, () -> leetcodeClient.findQuestionBySlug("old-slug")); assertTrue(error.isNotFound()); + assertNull(meterRegistry.find("leetcode.client.exception").counter()); } @ParameterizedTest @@ -77,6 +78,8 @@ void unsuccessfulOrMalformedResponseIsNotNotFound(String body) throws Exception var error = assertThrows(LeetcodeClientException.class, () -> leetcodeClient.findQuestionBySlug("example")); assertFalse(error.isNotFound()); + assertEquals( + 1.0, meterRegistry.get("leetcode.client.exception").counter().count()); } @ParameterizedTest 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 f9fcd6b75..46eff5e89 100644 --- a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java +++ b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/AttachTagsToExistingQuestionTest.java @@ -69,6 +69,8 @@ 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) From 9c2649d92946dc200b5606a386879f126e9fa146 Mon Sep 17 00:00:00 2001 From: Nancy Huang <205217630+naanci@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:17:17 -0400 Subject: [PATCH 9/9] TAN-989: remove flaky and queue lock --- .../common/utils/lock/QueueLockTest.java | 92 +++++++++---------- .../LeetcodeQuestionProcessServiceTest.java | 2 +- ...eetcodeQuestionProcessServiceUnitTest.java | 30 ------ 3 files changed, 44 insertions(+), 80 deletions(-) diff --git a/src/test/java/org/patinanetwork/codebloom/common/utils/lock/QueueLockTest.java b/src/test/java/org/patinanetwork/codebloom/common/utils/lock/QueueLockTest.java index 0ae118ae0..e7c0fbbb2 100644 --- a/src/test/java/org/patinanetwork/codebloom/common/utils/lock/QueueLockTest.java +++ b/src/test/java/org/patinanetwork/codebloom/common/utils/lock/QueueLockTest.java @@ -4,11 +4,13 @@ import static org.mockito.Mockito.*; import io.github.bucket4j.BlockingBucket; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -73,67 +75,59 @@ void acquireShouldAppendToEndOfQueue() throws InterruptedException { @Test @Timeout(value = 5, unit = TimeUnit.SECONDS) - void acquireFastShouldAppendToStartOfQueue() throws Exception { - var tickerStarted = new CountDownLatch(1); - var releaseTicker = new CountDownLatch(1); - var releaseNormal = new CountDownLatch(1); - var normalSelected = new CountDownLatch(1); - var calls = new AtomicInteger(); + void acquireFastShouldAppendToStartOfQueue() throws InterruptedException { + CountDownLatch tickerCallLatch = new CountDownLatch(1); + CountDownLatch releaseTicker = new CountDownLatch(1); doAnswer(invocation -> { - int call = calls.incrementAndGet(); - if (call == 1) { - tickerStarted.countDown(); - releaseTicker.await(); - } else if (call == 3) { - normalSelected.countDown(); - releaseNormal.await(); - } + tickerCallLatch.countDown(); + releaseTicker.await(); return null; }) .when(bucket) .consume(1); - try { - var initial = executor.submit(() -> { + executor.submit(() -> { + try { queueLock.acquire(); - return null; - }); - assertTrue(tickerStarted.await(2, TimeUnit.SECONDS)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + assertTrue(tickerCallLatch.await(2, TimeUnit.SECONDS)); + + List orderedCalls = Collections.synchronizedList(new ArrayList<>()); + CountDownLatch doneLatch = new CountDownLatch(2); - var normal = executor.submit(() -> { + executor.submit(() -> { + try { queueLock.acquire(); - return null; - }); - awaitQueueSize(1); + orderedCalls.add("T2"); + doneLatch.countDown(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); - var fast = executor.submit(() -> { + Thread.sleep(100); + + executor.submit(() -> { + try { queueLock.acquireFast(); - return null; - }); - awaitQueueSize(2); - - releaseTicker.countDown(); - fast.get(2, TimeUnit.SECONDS); - assertTrue(normalSelected.await(2, TimeUnit.SECONDS)); - assertFalse(normal.isDone(), "Normal request must wait until the fast request is released"); - - releaseNormal.countDown(); - normal.get(2, TimeUnit.SECONDS); - initial.get(2, TimeUnit.SECONDS); - verify(bucket, times(3)).consume(1); - } finally { - releaseTicker.countDown(); - releaseNormal.countDown(); - } - } + orderedCalls.add("T3"); + doneLatch.countDown(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + + Thread.sleep(100); + + releaseTicker.countDown(); - private void awaitQueueSize(int expected) throws InterruptedException { - long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(2); - while (queueLock.queue.size() != expected && System.nanoTime() < deadline) { - Thread.sleep(1); - } - assertEquals(expected, queueLock.queue.size(), "Requests did not enter the queue in time"); + assertTrue(doneLatch.await(2, TimeUnit.SECONDS)); + assertEquals("T3", orderedCalls.get(0)); + assertEquals("T2", orderedCalls.get(1)); } @Test diff --git a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceTest.java b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceTest.java index 8cf756ec2..44217bcb0 100644 --- a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceTest.java +++ b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceTest.java @@ -177,7 +177,7 @@ void jobStatusTransitionValid() { @Test void drainQueueValid() { - service.drainQueue().join(); + service.drainQueue(); } // TODO: (TAN-32) re-enable diff --git a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceUnitTest.java b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceUnitTest.java index 9fb46c983..9438ad0e8 100644 --- a/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceUnitTest.java +++ b/src/test/java/org/patinanetwork/codebloom/scheduled/leetcode/LeetcodeQuestionProcessServiceUnitTest.java @@ -5,8 +5,6 @@ import java.util.List; import java.util.Optional; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -67,34 +65,6 @@ private void runQueue() { .join(); } - @Test - void independentServiceCanDrainWhileAnotherInstanceIsRunning() { - var otherJobs = mock(JobRepository.class); - when(otherJobs.findIncompleteJobs(10)).thenReturn(List.of()); - var otherService = new LeetcodeQuestionProcessService(otherJobs, client, questions, bank); - when(jobs.findIncompleteJobs(10)).thenAnswer(invocation -> { - CompletableFuture.runAsync(() -> otherService.drainQueue().join()).get(5, TimeUnit.SECONDS); - return List.of(); - }); - - runQueue(); - - verify(otherJobs).findIncompleteJobs(10); - } - - @Test - void sameServiceSkipsConcurrentDrain() { - var service = new LeetcodeQuestionProcessService(jobs, client, questions, bank); - when(jobs.findIncompleteJobs(10)).thenAnswer(invocation -> { - CompletableFuture.runAsync(() -> service.drainQueue().join()).get(5, TimeUnit.SECONDS); - return List.of(); - }); - - service.drainQueue().join(); - - verify(jobs).findIncompleteJobs(10); - } - @ParameterizedTest @NullAndEmptySource @ValueSource(strings = {" ", "\t\n"})