From 4d9c4e8448564891722b83f31cb4e522ee3d5417 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Thu, 7 Nov 2024 15:26:11 +0100 Subject: [PATCH 01/14] [kbss-cvut/termit-ui#553] Add language to File. --- .../cvut/kbss/termit/model/resource/File.java | 31 +++++++++---------- .../context/DescriptorFactoryTest.java | 4 +-- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/model/resource/File.java b/src/main/java/cz/cvut/kbss/termit/model/resource/File.java index 26b45f940..c16d62a2a 100644 --- a/src/main/java/cz/cvut/kbss/termit/model/resource/File.java +++ b/src/main/java/cz/cvut/kbss/termit/model/resource/File.java @@ -21,16 +21,16 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import cz.cvut.kbss.jopa.model.annotations.FetchType; import cz.cvut.kbss.jopa.model.annotations.Inferred; +import cz.cvut.kbss.jopa.model.annotations.OWLAnnotationProperty; import cz.cvut.kbss.jopa.model.annotations.OWLClass; import cz.cvut.kbss.jopa.model.annotations.OWLObjectProperty; import cz.cvut.kbss.jopa.model.annotations.Types; +import cz.cvut.kbss.jopa.vocabulary.DC; import cz.cvut.kbss.jsonld.annotation.JsonLdAttributeOrder; -import cz.cvut.kbss.termit.exception.TermItException; import cz.cvut.kbss.termit.model.util.SupportsStorage; import cz.cvut.kbss.termit.service.IdentifierResolver; import cz.cvut.kbss.termit.util.Vocabulary; -import java.lang.reflect.Field; import java.util.Objects; import java.util.Set; @@ -43,6 +43,9 @@ public class File extends Resource implements SupportsStorage { @OWLObjectProperty(iri = Vocabulary.s_p_je_casti_dokumentu, fetch = FetchType.EAGER) private Document document; + @OWLAnnotationProperty(iri = DC.Terms.LANGUAGE, simpleLiteral = true) + private String language; + @Types private Set types; @@ -54,6 +57,14 @@ public void setDocument(Document document) { this.document = document; } + public String getLanguage() { + return language; + } + + public void setLanguage(String language) { + this.language = language; + } + public Set getTypes() { return types; } @@ -73,15 +84,11 @@ public boolean equals(Object o) { return Objects.equals(getUri(), file.getUri()); } - @Override - public int hashCode() { - return Objects.hash(getUri()); - } - @Override public String toString() { return "File{" + - super.toString() + (document != null ? "document=<" + document.getUri() + ">" : "") + '}'; + super.toString() + (language != null ? "@" + language : "") + + (document != null ? "document=<" + document.getUri() + ">" : "") + '}'; } /** @@ -109,12 +116,4 @@ public String getDirectoryName() { return IdentifierResolver.normalizeToAscii(labelPart) + '_' + getUri().hashCode(); } } - - public static Field getDocumentField() { - try { - return File.class.getDeclaredField("document"); - } catch (NoSuchFieldException e) { - throw new TermItException("Fatal error! Unable to retrieve \"document\" field.", e); - } - } } diff --git a/src/test/java/cz/cvut/kbss/termit/persistence/context/DescriptorFactoryTest.java b/src/test/java/cz/cvut/kbss/termit/persistence/context/DescriptorFactoryTest.java index c22fc8a49..621c8d823 100644 --- a/src/test/java/cz/cvut/kbss/termit/persistence/context/DescriptorFactoryTest.java +++ b/src/test/java/cz/cvut/kbss/termit/persistence/context/DescriptorFactoryTest.java @@ -98,7 +98,7 @@ void termDescriptorCreatesDescriptorWithExactMatchesContextSetToDefaultToAllowEx } @Test - void fileDescriptorContainsAlsoDescriptorForDocument() { + void fileDescriptorContainsAlsoDescriptorForDocument() throws Exception { final File file = Generator.generateFileWithId("test.html"); final Document doc = Generator.generateDocumentWithId(); doc.addFile(file); @@ -106,7 +106,7 @@ void fileDescriptorContainsAlsoDescriptorForDocument() { doc.setVocabulary(Generator.generateUri()); final Descriptor result = sut.fileDescriptor(doc.getVocabulary()); final FieldSpecification docFieldSpec = mock(FieldSpecification.class); - when(docFieldSpec.getJavaField()).thenReturn(File.getDocumentField()); + when(docFieldSpec.getJavaField()).thenReturn(File.class.getDeclaredField("document")); final Descriptor docDescriptor = result.getAttributeDescriptor(docFieldSpec); assertNotNull(docDescriptor); } From 0a6fdba9c864c09cebb614645294942459c5c86a Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Thu, 7 Nov 2024 16:09:33 +0100 Subject: [PATCH 02/14] [kbss-cvut/termit-ui#553] Set File language when it is not provided on creation. --- .../service/business/ResourceService.java | 10 ++++- .../service/business/ResourceServiceTest.java | 39 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/main/java/cz/cvut/kbss/termit/service/business/ResourceService.java b/src/main/java/cz/cvut/kbss/termit/service/business/ResourceService.java index f8d8f87a3..08aee833d 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/business/ResourceService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/business/ResourceService.java @@ -37,6 +37,7 @@ import cz.cvut.kbss.termit.service.document.html.UnconfirmedTermOccurrenceRemover; import cz.cvut.kbss.termit.service.repository.ChangeRecordService; import cz.cvut.kbss.termit.service.repository.ResourceRepositoryService; +import cz.cvut.kbss.termit.util.Configuration; import cz.cvut.kbss.termit.util.TypeAwareResource; import jakarta.annotation.Nonnull; import org.slf4j.Logger; @@ -80,22 +81,26 @@ public class ResourceService private final ChangeRecordService changeRecordService; + private final Configuration config; + private ApplicationEventPublisher eventPublisher; @Autowired public ResourceService(ResourceRepositoryService repositoryService, DocumentManager documentManager, TextAnalysisService textAnalysisService, VocabularyService vocabularyService, - ChangeRecordService changeRecordService) { + ChangeRecordService changeRecordService, Configuration config) { this.repositoryService = repositoryService; this.documentManager = documentManager; this.textAnalysisService = textAnalysisService; this.vocabularyService = vocabularyService; this.changeRecordService = changeRecordService; + this.config = config; } /** * Ensures that document gets removed during Vocabulary removal */ + @Transactional @EventListener public void onVocabularyRemoval(VocabularyWillBeRemovedEvent event) { vocabularyService.find(event.getVocabularyIri()).ifPresent(vocabulary -> { @@ -239,6 +244,9 @@ public void addFileToDocument(Resource document, File file) { throw new UnsupportedAssetOperationException("Cannot add file to the specified resource " + document); } doc.addFile(file); + if (file.getLanguage() == null) { + file.setLanguage(config.getPersistence().getLanguage()); + } if (doc.getVocabulary() != null) { final Vocabulary vocabulary = vocabularyService.getReference(doc.getVocabulary()); repositoryService.persist(file, vocabulary); diff --git a/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java b/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java index 6119b0f90..2777f42fe 100644 --- a/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java +++ b/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java @@ -35,6 +35,8 @@ import cz.cvut.kbss.termit.service.document.TextAnalysisService; import cz.cvut.kbss.termit.service.repository.ChangeRecordService; import cz.cvut.kbss.termit.service.repository.ResourceRepositoryService; +import cz.cvut.kbss.termit.util.Configuration; +import cz.cvut.kbss.termit.util.Constants; import cz.cvut.kbss.termit.util.TypeAwareByteArrayResource; import cz.cvut.kbss.termit.util.TypeAwareResource; import cz.cvut.kbss.termit.util.Utils; @@ -47,6 +49,7 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; +import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; import org.springframework.http.MediaType; @@ -96,6 +99,9 @@ class ResourceServiceTest { @Mock private ApplicationEventPublisher eventPublisher; + @Spy + private Configuration config = new Configuration(); + @InjectMocks private ResourceService sut; @@ -515,4 +521,37 @@ void getContentWithoutUnconfirmedOccurrencesRemovesUnconfirmedOccurrencesFromFil final org.jsoup.nodes.Document doc = Jsoup.parse(result.getInputStream(), StandardCharsets.UTF_8.name(), ""); assertTrue(doc.select("span[score]").isEmpty()); } + + @Test + void addFileToDocumentSetsFileLanguageToDefaultConfiguredWhenNotProvided() { + config.getPersistence().setLanguage(Constants.DEFAULT_LANGUAGE); + final Vocabulary vocabulary = Generator.generateVocabularyWithId(); + final Document document = Generator.generateDocumentWithId(); + document.setVocabulary(vocabulary.getUri()); + final File file = Generator.generateFileWithId("test.hml"); + when(resourceRepositoryService.exists(document.getUri())).thenReturn(true); + when(resourceRepositoryService.findRequired(document.getUri())).thenReturn(document); + when(vocabularyService.getReference(vocabulary.getUri())).thenReturn(vocabulary); + + sut.addFileToDocument(document, file); + verify(resourceRepositoryService).persist(file, vocabulary); + assertEquals(config.getPersistence().getLanguage(), file.getLanguage()); + } + + @Test + void addFileToDocumentDoesNotModifyLanguageWhenItIsAlreadySet() { + config.getPersistence().setLanguage(Constants.DEFAULT_LANGUAGE); + final Vocabulary vocabulary = Generator.generateVocabularyWithId(); + final Document document = Generator.generateDocumentWithId(); + document.setVocabulary(vocabulary.getUri()); + final File file = Generator.generateFileWithId("test.hml"); + file.setLanguage("cs"); + when(resourceRepositoryService.exists(document.getUri())).thenReturn(true); + when(resourceRepositoryService.findRequired(document.getUri())).thenReturn(document); + when(vocabularyService.getReference(vocabulary.getUri())).thenReturn(vocabulary); + + sut.addFileToDocument(document, file); + verify(resourceRepositoryService).persist(file, vocabulary); + assertEquals("cs", file.getLanguage()); + } } From 8c8a15f63bac28f2ed07b9f05a9c2fbaaee1ae14 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Thu, 7 Nov 2024 16:25:54 +0100 Subject: [PATCH 03/14] [kbss-cvut/termit-ui#553] Use File language (if available) when invoking text analysis. --- .../service/document/TextAnalysisService.java | 2 +- .../document/TextAnalysisServiceTest.java | 96 ++++++++++++------- 2 files changed, 63 insertions(+), 35 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java b/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java index adc9dfdae..12bddd7e4 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java @@ -107,7 +107,7 @@ private TextAnalysisInput createAnalysisInput(File file) { publicUrl.isEmpty() || publicUrl.get().isEmpty() ? config.getRepository().getUrl() : publicUrl.get() ); input.setVocabularyRepository(repositoryUrl); - input.setLanguage(config.getPersistence().getLanguage()); + input.setLanguage(file.getLanguage() != null ? file.getLanguage() : config.getPersistence().getLanguage()); input.setVocabularyRepositoryUserName(config.getRepository().getUsername()); input.setVocabularyRepositoryPassword(config.getRepository().getPassword()); return input; diff --git a/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java b/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java index aa431671e..6bdc27284 100644 --- a/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java +++ b/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java @@ -84,6 +84,7 @@ import static org.mockito.Mockito.when; import static org.springframework.test.web.client.match.MockRestRequestMatchers.content; import static org.springframework.test.web.client.match.MockRestRequestMatchers.header; +import static org.springframework.test.web.client.match.MockRestRequestMatchers.jsonPath; import static org.springframework.test.web.client.match.MockRestRequestMatchers.method; import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; import static org.springframework.test.web.client.response.MockRestResponseCreators.withServerError; @@ -143,14 +144,14 @@ void setUp() throws Exception { doCallRealMethod().when(documentManagerSpy).loadFileContent(any()); doNothing().when(documentManagerSpy).createBackup(any()); this.sut = new TextAnalysisService(restTemplate, config, documentManagerSpy, annotationGeneratorMock, - textAnalysisRecordDao, eventPublisher); + textAnalysisRecordDao, eventPublisher); } @Test void analyzeFileInvokesTextAnalysisServiceWithDocumentContent() { mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) - .andExpect(method(HttpMethod.POST)).andExpect(content().string(containsString(CONTENT))) - .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); + .andExpect(method(HttpMethod.POST)).andExpect(content().string(containsString(CONTENT))) + .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); sut.analyzeFile(file, Collections.singleton(vocabulary.getUri())); mockServer.verify(); } @@ -159,7 +160,8 @@ private void generateFile() throws IOException { final java.io.File dir = Files.createTempDirectory("termit").toFile(); dir.deleteOnExit(); config.getFile().setStorage(dir.getAbsolutePath()); - final java.io.File docDir = new java.io.File(dir.getAbsolutePath() + java.io.File.separator + file.getDirectoryName()); + final java.io.File docDir = new java.io.File( + dir.getAbsolutePath() + java.io.File.separator + file.getDirectoryName()); Files.createDirectory(docDir.toPath()); docDir.deleteOnExit(); final java.io.File content = new java.io.File( @@ -172,9 +174,9 @@ private void generateFile() throws IOException { void analyzeFilePassesRepositoryAndVocabularyContextToService() throws Exception { final TextAnalysisInput input = textAnalysisInput(); mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) - .andExpect(method(HttpMethod.POST)) - .andExpect(content().string(objectMapper.writeValueAsString(input))) - .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); + .andExpect(method(HttpMethod.POST)) + .andExpect(content().string(objectMapper.writeValueAsString(input))) + .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); sut.analyzeFile(file, Collections.singleton(vocabulary.getUri())); mockServer.verify(); } @@ -184,8 +186,8 @@ private TextAnalysisInput textAnalysisInput() { input.setContent(CONTENT); input.addVocabularyContext(vocabulary.getUri()); URI repositoryUrl = URI.create( - config.getRepository().getPublicUrl() - .orElse(config.getRepository().getUrl()) + config.getRepository().getPublicUrl() + .orElse(config.getRepository().getUrl()) ); input.setVocabularyRepository(repositoryUrl); input.setLanguage(config.getPersistence().getLanguage()); @@ -198,11 +200,11 @@ private TextAnalysisInput textAnalysisInput() { void analyzeFilePassesContentTypeAndAcceptHeadersToService() throws Exception { final TextAnalysisInput input = textAnalysisInput(); mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) - .andExpect(method(HttpMethod.POST)) - .andExpect(content().string(objectMapper.writeValueAsString(input))) - .andExpect(header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)) - .andExpect(header(HttpHeaders.ACCEPT, MediaType.APPLICATION_XML_VALUE)) - .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); + .andExpect(method(HttpMethod.POST)) + .andExpect(content().string(objectMapper.writeValueAsString(input))) + .andExpect(header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)) + .andExpect(header(HttpHeaders.ACCEPT, MediaType.APPLICATION_XML_VALUE)) + .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); sut.analyzeFile(file, Collections.singleton(vocabulary.getUri())); mockServer.verify(); } @@ -228,11 +230,11 @@ void analyzeFilePassesRepositoryUsernameAndPasswordToServiceWhenProvided() throw void analyzeFileThrowsWebServiceIntegrationExceptionOnError() throws Exception { final TextAnalysisInput input = textAnalysisInput(); mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) - .andExpect(method(HttpMethod.POST)) - .andExpect(content().string(objectMapper.writeValueAsString(input))) - .andRespond(withServerError()); + .andExpect(method(HttpMethod.POST)) + .andExpect(content().string(objectMapper.writeValueAsString(input))) + .andRespond(withServerError()); assertThrows(WebServiceIntegrationException.class, - () -> sut.analyzeFile(file, Collections.singleton(vocabulary.getUri()))); + () -> sut.analyzeFile(file, Collections.singleton(vocabulary.getUri()))); mockServer.verify(); } @@ -256,7 +258,8 @@ void analyzeFileInvokesAnnotationGeneratorWithResultFromTextAnalysisService() th void analyzeFileThrowsNotFoundExceptionWhenFileCannotBeFound() { file.setLabel("unknown.html"); final NotFoundException result = assertThrows(NotFoundException.class, - () -> sut.analyzeFile(file, Collections.singleton(vocabulary.getUri()))); + () -> sut.analyzeFile(file, Collections.singleton( + vocabulary.getUri()))); assertThat(result.getMessage(), containsString("not found on file system")); } @@ -264,11 +267,12 @@ void analyzeFileThrowsNotFoundExceptionWhenFileCannotBeFound() { void analyzeFileThrowsWebServiceIntegrationExceptionWhenRemoteServiceReturnsEmptyBody() throws Exception { final TextAnalysisInput input = textAnalysisInput(); mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) - .andExpect(method(HttpMethod.POST)) - .andExpect(content().string(objectMapper.writeValueAsString(input))) - .andRespond(withSuccess()); + .andExpect(method(HttpMethod.POST)) + .andExpect(content().string(objectMapper.writeValueAsString(input))) + .andRespond(withSuccess()); final WebServiceIntegrationException result = assertThrows(WebServiceIntegrationException.class, - () -> sut.analyzeFile(file, Collections.singleton(vocabulary.getUri()))); + () -> sut.analyzeFile(file, Collections.singleton( + vocabulary.getUri()))); assertThat(result.getMessage(), containsString("empty response")); mockServer.verify(); } @@ -290,13 +294,13 @@ void analyzeFileCreatesFileBackupBeforeInvokingAnnotationGenerator() throws Exce @Test void analyzeFilePassesRepositoryAndSpecifiedVocabularyContextsToService() throws Exception { final Set vocabs = IntStream.range(0, 5).mapToObj(i -> Generator.generateUri()) - .collect(Collectors.toSet()); + .collect(Collectors.toSet()); final TextAnalysisInput expected = textAnalysisInput(); expected.setVocabularyContexts(vocabs); mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) - .andExpect(method(HttpMethod.POST)) - .andExpect(content().string(objectMapper.writeValueAsString(expected))) - .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); + .andExpect(method(HttpMethod.POST)) + .andExpect(content().string(objectMapper.writeValueAsString(expected))) + .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); sut.analyzeFile(file, vocabs); mockServer.verify(); } @@ -305,9 +309,9 @@ void analyzeFilePassesRepositoryAndSpecifiedVocabularyContextsToService() throws void analyzeFileBacksUpFileContentBeforeSavingNewAnalyzedContent() throws Exception { final TextAnalysisInput input = textAnalysisInput(); mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) - .andExpect(method(HttpMethod.POST)) - .andExpect(content().string(objectMapper.writeValueAsString(input))) - .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); + .andExpect(method(HttpMethod.POST)) + .andExpect(content().string(objectMapper.writeValueAsString(input))) + .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); sut.analyzeFile(file, Collections.singleton(vocabulary.getUri())); mockServer.verify(); final InOrder inOrder = Mockito.inOrder(documentManagerSpy, annotationGeneratorMock); @@ -318,8 +322,8 @@ void analyzeFileBacksUpFileContentBeforeSavingNewAnalyzedContent() throws Except @Test void analyzeFileCreatesTextAnalysisRecord() { mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) - .andExpect(method(HttpMethod.POST)).andExpect(content().string(containsString(CONTENT))) - .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); + .andExpect(method(HttpMethod.POST)).andExpect(content().string(containsString(CONTENT))) + .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); sut.analyzeFile(file, Collections.singleton(vocabulary.getUri())); final ArgumentCaptor captor = ArgumentCaptor.forClass(TextAnalysisRecord.class); verify(textAnalysisRecordDao).persist(captor.capture()); @@ -424,7 +428,8 @@ void analyzeFilePublishesAnalysisFinishedEvent() { .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); sut.analyzeFile(file, Collections.singleton(vocabulary.getUri())); - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(FileTextAnalysisFinishedEvent.class); + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass( + FileTextAnalysisFinishedEvent.class); verify(eventPublisher).publishEvent(eventCaptor.capture()); assertNotNull(eventCaptor.getValue()); assertEquals(file.getUri(), eventCaptor.getValue().getFileUri()); @@ -444,10 +449,33 @@ void analyzeTermDefinitionPublishesAnalysisFinishedEvent() throws JsonProcessing sut.analyzeTermDefinition(term, vocabulary.getUri()); - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(TermDefinitionTextAnalysisFinishedEvent.class); + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass( + TermDefinitionTextAnalysisFinishedEvent.class); verify(eventPublisher).publishEvent(eventCaptor.capture()); assertNotNull(eventCaptor.getValue()); assertEquals(term.getUri(), eventCaptor.getValue().getTermUri()); assertEquals(vocabulary.getUri(), eventCaptor.getValue().getVocabularyIri()); } + + @Test + void analyzeFileSetsFileLanguageInTextAnalysisInvocationInput() { + file.setLanguage("cs"); + mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) + .andExpect(method(HttpMethod.POST)) + .andExpect(jsonPath("$.language").value("cs")) + .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); + sut.analyzeFile(file, Collections.singleton(vocabulary.getUri())); + mockServer.verify(); + } + + @Test + void analyzeFileUsesConfiguredPersistenceLanguageInTextAnalysisInvocationInputWhenFileLanguageIsNotSet() { + file.setLanguage(null); + mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) + .andExpect(method(HttpMethod.POST)) + .andExpect(jsonPath("$.language").value(Environment.LANGUAGE)) + .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); + sut.analyzeFile(file, Collections.singleton(vocabulary.getUri())); + mockServer.verify(); + } } From 4db5145ab18225bf2e1327f9222258980b451766 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Thu, 7 Nov 2024 16:55:06 +0100 Subject: [PATCH 04/14] [kbss-cvut/termit-ui#553] Allow retrieving list of languages used in vocabulary. --- .../termit/persistence/dao/VocabularyDao.java | 162 +++++++++++------- .../termit/rest/VocabularyController.java | 16 ++ .../service/business/VocabularyService.java | 13 +- .../VocabularyRepositoryService.java | 11 ++ .../persistence/dao/VocabularyDaoTest.java | 20 +++ .../termit/rest/VocabularyControllerTest.java | 12 ++ src/test/resources/application.yml | 1 + 7 files changed, 170 insertions(+), 65 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java b/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java index d0cd42ea8..fec898b75 100644 --- a/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java +++ b/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java @@ -218,10 +218,13 @@ public Vocabulary update(Vocabulary entity) { /** * Forcefully removes the specified vocabulary. *

- * This deletes the whole graph of the vocabulary, all terms in the vocabulary's glossary and then removes the vocabulary itself. Extreme caution - * should be exercised when using this method. All relevant data, including documents and files, will be dropped. + * This deletes the whole graph of the vocabulary, all terms in the vocabulary's glossary and then removes the + * vocabulary itself. Extreme caution should be exercised when using this method. All relevant data, including + * documents and files, will be dropped. *

- * Publishes {@link VocabularyWillBeRemovedEvent} before the actual removal to allow other services to clean up related resources (e.g., delete the document). + * Publishes {@link VocabularyWillBeRemovedEvent} before the actual removal to allow other services to clean up + * related resources (e.g., delete the document). + * * @param entity The vocabulary to delete */ @ModifiesData @@ -236,9 +239,9 @@ public void remove(Vocabulary entity) { *

* Forcefully removes the specified vocabulary. *

- * This deletes all terms in the vocabulary's glossary and then removes the vocabulary itself. - * Extreme caution should be exercised when using this method, - * as it does not check for any references or usage and just drops all the relevant data. + * This deletes all terms in the vocabulary's glossary and then removes the vocabulary itself. Extreme caution + * should be exercised when using this method, as it does not check for any references or usage and just drops all + * the relevant data. *

* The document is not removed. */ @@ -248,19 +251,19 @@ public void removeVocabularyKeepDocument(Vocabulary entity) { /** *

- * Does not publish the {@link VocabularyWillBeRemovedEvent}.
- * You should use {@link #remove(Vocabulary)} instead. + * Does not publish the {@link VocabularyWillBeRemovedEvent}.
You should use {@link #remove(Vocabulary)} + * instead. *

* Forcefully removes the specified vocabulary. *

* This deletes all terms in the vocabulary's glossary and then removes the vocabulary itself. Extreme caution * should be exercised when using this method, as it does not check for any references or usage and just drops all * the relevant data. - * @param entity The vocabulary to delete - * @param dropGraph if false, - * executes {@code src/main/resources/query/remove/removeGlossaryTerms.ru} removing terms, - * their relations, model, glossary and vocabulary itself, keeps the document. - * When true, the whole vocabulary graph is dropped. + * + * @param entity The vocabulary to delete + * @param dropGraph if false, executes {@code src/main/resources/query/remove/removeGlossaryTerms.ru} removing + * terms, their relations, model, glossary and vocabulary itself, keeps the document. When true, + * the whole vocabulary graph is dropped. */ private void removeVocabulary(Vocabulary entity, boolean dropGraph) { Objects.requireNonNull(entity); @@ -268,7 +271,7 @@ private void removeVocabulary(Vocabulary entity, boolean dropGraph) { try { final URI vocabularyContext = contextMapper.getVocabularyContext(entity.getUri()); - if(dropGraph) { + if (dropGraph) { // drops whole named graph em.createNativeQuery("DROP GRAPH ?context") .setParameter("context", vocabularyContext) @@ -317,8 +320,8 @@ public Optional findGlossary(URI uri) { } /** - * Checks whether terms from the {@code subjectVocabulary} reference (as parent terms) any terms from the {@code - * targetVocabulary}. + * Checks whether terms from the {@code subjectVocabulary} reference (as parent terms) any terms from the + * {@code targetVocabulary}. * * @param subjectVocabulary Subject vocabulary identifier * @param targetVocabulary Target vocabulary identifier @@ -395,7 +398,7 @@ public List getChangesOfContent(Vocabulary vocabulary) { * Gets content change records of the specified vocabulary. * * @param vocabulary Vocabulary whose content changes to get - * @param pageReq Specification of the size and number of the page to return + * @param pageReq Specification of the size and number of the page to return * @return List of change records, ordered by date in descending order */ public List getDetailedHistoryOfContent(Vocabulary vocabulary, Pageable pageReq) { @@ -403,25 +406,27 @@ public List getDetailedHistoryOfContent(Vocabulary vocabul return createDetailedContentChangesQuery(vocabulary, pageReq).getResultList(); } - private TypedQuery createDetailedContentChangesQuery(Vocabulary vocabulary, Pageable pageReq) { + private TypedQuery createDetailedContentChangesQuery(Vocabulary vocabulary, + Pageable pageReq) { return em.createNativeQuery(""" - SELECT ?record WHERE { - ?term ?inVocabulary ?vocabulary ; - a ?termType . - ?record a ?changeRecord ; - ?relatesTo ?term ; - ?hasTime ?timestamp . - OPTIONAL { ?record ?hasChangedAttribute ?attribute . } - } ORDER BY DESC(?timestamp) ?attribute - """, AbstractChangeRecord.class) + SELECT ?record WHERE { + ?term ?inVocabulary ?vocabulary ; + a ?termType . + ?record a ?changeRecord ; + ?relatesTo ?term ; + ?hasTime ?timestamp . + OPTIONAL { ?record ?hasChangedAttribute ?attribute . } + } ORDER BY DESC(?timestamp) ?attribute + """, AbstractChangeRecord.class) .setParameter("inVocabulary", - URI.create(cz.cvut.kbss.termit.util.Vocabulary.s_p_je_pojmem_ze_slovniku)) + URI.create(cz.cvut.kbss.termit.util.Vocabulary.s_p_je_pojmem_ze_slovniku)) .setParameter("vocabulary", vocabulary) - .setParameter("termType", URI.create(SKOS.CONCEPT)) + .setParameter("termType", URI.create(SKOS.CONCEPT)) .setParameter("changeRecord", URI.create(cz.cvut.kbss.termit.util.Vocabulary.s_c_zmena)) .setParameter("relatesTo", URI.create(cz.cvut.kbss.termit.util.Vocabulary.s_p_ma_zmenenou_entitu)) .setParameter("hasTime", URI.create(cz.cvut.kbss.termit.util.Vocabulary.s_p_ma_datum_a_cas_modifikace)) - .setParameter("hasChangedAttribute", URI.create(cz.cvut.kbss.termit.util.Vocabulary.s_p_ma_zmeneny_atribut)) + .setParameter("hasChangedAttribute", + URI.create(cz.cvut.kbss.termit.util.Vocabulary.s_p_ma_zmeneny_atribut)) .setFirstResult((int) pageReq.getOffset()) .setMaxResults(pageReq.getPageSize()); } @@ -576,16 +581,17 @@ public List getVocabularyRelations(Vocabulary vocabulary, Collect try { return em.createNativeQuery(""" - SELECT DISTINCT ?object ?relation ?subject { - ?object a ?vocabularyType ; - ?relation ?subject . - FILTER(?object != ?subject) . - FILTER(?relation NOT IN (?excluded)) . - } ORDER BY ?object ?relation - """, "RDFStatement") + SELECT DISTINCT ?object ?relation ?subject { + ?object a ?vocabularyType ; + ?relation ?subject . + FILTER(?object != ?subject) . + FILTER(?relation NOT IN (?excluded)) . + } ORDER BY ?object ?relation + """, "RDFStatement") .setParameter("subject", vocabularyUri) - .setParameter("excluded", excludedRelations) - .setParameter("vocabularyType", URI.create(EntityToOwlClassMapper.getOwlClassForEntity(Vocabulary.class))) + .setParameter("excluded", excludedRelations) + .setParameter("vocabularyType", + URI.create(EntityToOwlClassMapper.getOwlClassForEntity(Vocabulary.class))) .getResultList(); } catch (RuntimeException e) { throw new PersistenceException(e); @@ -603,31 +609,31 @@ public List getTermRelations(Vocabulary vocabulary) { try { return em.createNativeQuery(""" - SELECT DISTINCT ?object ?relation ?subject WHERE { - ?term a ?termType; - ?inVocabulary ?vocabulary . - - { - ?term ?relation ?secondTerm . - ?secondTerm a ?termType; - ?inVocabulary ?secondVocabulary . - - BIND(?term as ?object) - BIND(?secondTerm as ?subject) - } UNION { - ?secondTerm ?relation ?term . - ?secondTerm a ?termType; - ?inVocabulary ?secondVocabulary . - - BIND(?secondTerm as ?object) - BIND(?term as ?subject) - } - - FILTER(?relation IN (?deniedRelations)) - FILTER(?object != ?subject) - FILTER(?secondVocabulary != ?vocabulary) - } ORDER by ?object ?relation ?subject - """, "RDFStatement" + SELECT DISTINCT ?object ?relation ?subject WHERE { + ?term a ?termType; + ?inVocabulary ?vocabulary . + + { + ?term ?relation ?secondTerm . + ?secondTerm a ?termType; + ?inVocabulary ?secondVocabulary . + + BIND(?term as ?object) + BIND(?secondTerm as ?subject) + } UNION { + ?secondTerm ?relation ?term . + ?secondTerm a ?termType; + ?inVocabulary ?secondVocabulary . + + BIND(?secondTerm as ?object) + BIND(?term as ?subject) + } + + FILTER(?relation IN (?deniedRelations)) + FILTER(?object != ?subject) + FILTER(?secondVocabulary != ?vocabulary) + } ORDER by ?object ?relation ?subject + """, "RDFStatement" ).setMaxResults(DEFAULT_PAGE_SIZE) .setParameter("termType", termType) .setParameter("inVocabulary", inVocabulary) @@ -638,4 +644,32 @@ public List getTermRelations(Vocabulary vocabulary) { throw new PersistenceException(e); } } + + /** + * Returns the list of all distinct languages (language tags) used by terms in the specified vocabulary. + * + * @param vocabularyUri Vocabulary identifier + * @return List of distinct languages + */ + public List getLanguages(URI vocabularyUri) { + Objects.requireNonNull(vocabularyUri); + try { + return em.createNativeQuery(""" + SELECT DISTINCT ?lang WHERE { + ?x a ?type ; + ?inVocabulary ?vocabulary ; + ?labelProp ?label . + BIND (LANG(?label) as ?lang) + } + """, String.class) + .setParameter("type", URI.create(SKOS.CONCEPT)) + .setParameter("inVocabulary", + URI.create(cz.cvut.kbss.termit.util.Vocabulary.s_p_je_pojmem_ze_slovniku)) + .setParameter("vocabulary", vocabularyUri) + .setParameter("labelProp", URI.create(SKOS.PREF_LABEL)) + .getResultList(); + } catch (RuntimeException e) { + throw new PersistenceException(e); + } + } } diff --git a/src/main/java/cz/cvut/kbss/termit/rest/VocabularyController.java b/src/main/java/cz/cvut/kbss/termit/rest/VocabularyController.java index e8cd5afb4..2f4d4d1a9 100644 --- a/src/main/java/cz/cvut/kbss/termit/rest/VocabularyController.java +++ b/src/main/java/cz/cvut/kbss/termit/rest/VocabularyController.java @@ -311,6 +311,22 @@ public List getDetailedHistoryOfContent( return vocabularyService.getDetailedHistoryOfContent(vocabulary, pageReq); } + @Operation(security = {@SecurityRequirement(name = "bearer-key")}, + description = "Gets a list of languages used in the vocabulary.") + @ApiResponses({ + @ApiResponse(responseCode = "200", description = "List of languages.") + }) + @GetMapping(value = "/{localName}/languages", produces = {MediaType.APPLICATION_JSON_VALUE, JsonLd.MEDIA_TYPE}) + public List getLanguages( + @Parameter(description = ApiDoc.ID_LOCAL_NAME_DESCRIPTION, + example = ApiDoc.ID_LOCAL_NAME_EXAMPLE) @PathVariable String localName, + @Parameter(description = ApiDoc.ID_NAMESPACE_DESCRIPTION, + example = ApiDoc.ID_NAMESPACE_EXAMPLE) @RequestParam(name = QueryParams.NAMESPACE, + required = false) Optional namespace) { + final URI vocabularyUri = resolveVocabularyUri(localName, namespace); + return vocabularyService.getLanguages(vocabularyUri); + } + @Operation(security = {@SecurityRequirement(name = "bearer-key")}, description = "Updates metadata of vocabulary with the specified identifier.") @ApiResponses({ diff --git a/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java b/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java index fe6d9b20a..6f265656c 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java @@ -316,7 +316,7 @@ public List getChangesOfContent(Vocabulary vocabulary) { * Gets content change records of the specified vocabulary. * * @param vocabulary Vocabulary whose content changes to get - * @param pageReq Specification of the size and number of the page to return + * @param pageReq Specification of the size and number of the page to return * @return List of change records, ordered by date in descending order */ public List getDetailedHistoryOfContent(Vocabulary vocabulary, Pageable pageReq) { @@ -522,6 +522,17 @@ public AccessLevel getAccessLevel(Vocabulary vocabulary) { return authorizationService.getAccessLevel(vocabulary); } + /** + * Gets the list of languages used in the specified vocabulary. + * + * @param vocabularyUri Vocabulary identifier + * @return List of languages + */ + @PreAuthorize("@vocabularyAuthorizationService.canRead(#vocabularyUri)") + public List getLanguages(URI vocabularyUri) { + return repositoryService.getLanguages(vocabularyUri); + } + @Override public void setApplicationEventPublisher(@Nonnull ApplicationEventPublisher eventPublisher) { this.eventPublisher = eventPublisher; diff --git a/src/main/java/cz/cvut/kbss/termit/service/repository/VocabularyRepositoryService.java b/src/main/java/cz/cvut/kbss/termit/service/repository/VocabularyRepositoryService.java index 6be0b86d4..6cffad957 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/repository/VocabularyRepositoryService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/repository/VocabularyRepositoryService.java @@ -372,4 +372,15 @@ public Vocabulary findVersionValidAt(Vocabulary vocabulary, Instant at) { public PrefixDeclaration resolvePrefix(URI vocabularyUri) { return vocabularyDao.resolvePrefix(vocabularyUri); } + + /** + * Returns the list of all distinct languages (language tags) used by terms in the specified vocabulary. + * + * @param vocabularyUri Vocabulary identifier + * @return List of distinct languages + */ + @Transactional(readOnly = true) + public List getLanguages(URI vocabularyUri) { + return vocabularyDao.getLanguages(vocabularyUri); + } } diff --git a/src/test/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDaoTest.java b/src/test/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDaoTest.java index 23b72777c..c7d063d1f 100644 --- a/src/test/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDaoTest.java +++ b/src/test/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDaoTest.java @@ -83,6 +83,7 @@ import static cz.cvut.kbss.termit.environment.util.ContainsSameEntities.containsSameEntities; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -927,4 +928,23 @@ void getAnyExternalRelationsReturnsTermsWithBothRelations(URI termRelation) { } }); } + + @Test + void getLanguagesReturnsDistinctLanguagesUsedByVocabularyTerms() { + final Vocabulary vocabulary = Generator.generateVocabularyWithId(); + final Term term = Generator.generateTermWithId(vocabulary.getUri()); + final Term term2 = Generator.generateTermWithId(vocabulary.getUri()); + term2.getLabel().set("cs", "Název v češtině"); + transactional(() -> { + em.persist(vocabulary, descriptorFor(vocabulary)); + em.persist(term, descriptorFactory.termDescriptor(term)); + em.persist(term2, descriptorFactory.termDescriptor(term2)); + Generator.addTermInVocabularyRelationship(term, vocabulary.getUri(), em); + Generator.addTermInVocabularyRelationship(term2, vocabulary.getUri(), em); + }); + + final List languages = sut.getLanguages(vocabulary.getUri()); + assertEquals(2, languages.size()); + assertThat(languages, hasItems(Environment.LANGUAGE, "cs")); + } } diff --git a/src/test/java/cz/cvut/kbss/termit/rest/VocabularyControllerTest.java b/src/test/java/cz/cvut/kbss/termit/rest/VocabularyControllerTest.java index 0d1c7444d..32d3aa47c 100644 --- a/src/test/java/cz/cvut/kbss/termit/rest/VocabularyControllerTest.java +++ b/src/test/java/cz/cvut/kbss/termit/rest/VocabularyControllerTest.java @@ -642,4 +642,16 @@ void getExcelTemplateFileReturnsExcelTemplateFileRetrievedFromServiceAsAttachmen assertThat(mvcResult.getResponse().getHeader(HttpHeaders.CONTENT_DISPOSITION), containsString("filename=\"termit-import.xlsx\"")); } + + @Test + void getLanguagesRetrievesAndReturnsListOfLanguagesUsedInVocabulary() throws Exception { + when(idResolverMock.resolveIdentifier(NAMESPACE, FRAGMENT)).thenReturn(VOCABULARY_URI); + final List languages = List.of(Environment.LANGUAGE, "cs", "de"); + when(serviceMock.getLanguages(VOCABULARY_URI)).thenReturn(languages); + + final MvcResult mvcResult = mockMvc.perform(get(PATH + "/" + FRAGMENT + "/languages").queryParam(QueryParams.NAMESPACE, NAMESPACE)).andReturn(); + final List result = readValue(mvcResult, new TypeReference>() {}); + assertEquals(languages, result); + verify(serviceMock).getLanguages(VOCABULARY_URI); + } } diff --git a/src/test/resources/application.yml b/src/test/resources/application.yml index 56f473bd6..9365e2b7f 100644 --- a/src/test/resources/application.yml +++ b/src/test/resources/application.yml @@ -1,3 +1,4 @@ +application.version: DEV spring: servlet: multipart: From f3f6dafb9ec4abc962301386cf321a73f452af95 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Mon, 11 Nov 2024 16:14:46 +0100 Subject: [PATCH 05/14] [GH-309] Allow running in development mode without configuring mail server. --- .../kbss/termit/service/mail/Postman.java | 11 ++++- .../cz/cvut/kbss/termit/util/Constants.java | 45 +++++++------------ .../java/cz/cvut/kbss/termit/util/Utils.java | 43 +++++++++++------- 3 files changed, 52 insertions(+), 47 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/service/mail/Postman.java b/src/main/java/cz/cvut/kbss/termit/service/mail/Postman.java index cb66781e9..04cd590ce 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/mail/Postman.java +++ b/src/main/java/cz/cvut/kbss/termit/service/mail/Postman.java @@ -19,6 +19,7 @@ import cz.cvut.kbss.termit.exception.PostmanException; import cz.cvut.kbss.termit.exception.ValidationException; +import cz.cvut.kbss.termit.util.Utils; import jakarta.mail.MessagingException; import jakarta.mail.internet.InternetAddress; import jakarta.mail.internet.MimeMessage; @@ -65,7 +66,12 @@ public Postman(Environment env, @Autowired(required = false) JavaMailSender mail @PostConstruct public void postConstruct() { - if(mailSender == null) { + if (mailSender == null) { + if (Utils.isDevelopmentProfile(env.getActiveProfiles())) { + LOG.warn( + "Mail server not configured but running in development mode. Will not be able to send messages."); + return; + } throw new ValidationException("Mail server not configured."); } } @@ -86,7 +92,8 @@ public void sendMessage(Message message) { final MimeMessage mail = mailSender.createMimeMessage(); final MimeMessageHelper helper = new MimeMessageHelper(mail, true); - helper.setFrom(new InternetAddress(sender != null ? sender : senderUsername, FROM_NICKNAME, StandardCharsets.UTF_8.toString())); + helper.setFrom(new InternetAddress(sender != null ? sender : senderUsername, FROM_NICKNAME, + StandardCharsets.UTF_8.toString())); helper.setTo(message.getRecipients().toArray(new String[]{})); helper.setSubject(message.getSubject()); helper.setText(message.getContent(), true); diff --git a/src/main/java/cz/cvut/kbss/termit/util/Constants.java b/src/main/java/cz/cvut/kbss/termit/util/Constants.java index 5d7ead6a9..7cb925992 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/Constants.java +++ b/src/main/java/cz/cvut/kbss/termit/util/Constants.java @@ -153,6 +153,23 @@ public class Constants { "Notation", "Example", "References") ); + + /** + * the maximum amount of data to buffer when sending messages to a WebSocket session + */ + public static final int WEBSOCKET_SEND_BUFFER_SIZE_LIMIT = Integer.MAX_VALUE; + + /** + * Set the maximum time allowed in milliseconds after the WebSocket connection is established + * and before the first sub-protocol message is received. + */ + public static final int WEBSOCKET_TIME_TO_FIRST_MESSAGE = 15 * 1000 /* 15s */; + + /** + * Development Spring profile. + */ + public static final String DEVELOPMENT_PROFILE = "development"; + private Constants() { throw new AssertionError(); } @@ -247,32 +264,4 @@ private QueryParams() { throw new AssertionError(); } } - - public static final class DebouncingGroups { - - /** - * Text analysis of all terms in specific vocabulary - */ - public static final String TEXT_ANALYSIS_VOCABULARY_TERMS_ALL_DEFINITIONS = "TEXT_ANALYSIS_VOCABULARY_TERMS_ALL_DEFINITIONS"; - - /** - * Text analysis of all vocabularies - */ - public static final String TEXT_ANALYSIS_VOCABULARY = "TEXT_ANALYSIS_VOCABULARY"; - - private DebouncingGroups() { - throw new AssertionError(); - } - } - - /** - * the maximum amount of data to buffer when sending messages to a WebSocket session - */ - public static final int WEBSOCKET_SEND_BUFFER_SIZE_LIMIT = Integer.MAX_VALUE; - - /** - * Set the maximum time allowed in milliseconds after the WebSocket connection is established - * and before the first sub-protocol message is received. - */ - public static final int WEBSOCKET_TIME_TO_FIRST_MESSAGE = 15 * 1000 /* 15s */; } diff --git a/src/main/java/cz/cvut/kbss/termit/util/Utils.java b/src/main/java/cz/cvut/kbss/termit/util/Utils.java index f8857028d..7adf76742 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/Utils.java +++ b/src/main/java/cz/cvut/kbss/termit/util/Utils.java @@ -44,6 +44,7 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -194,13 +195,20 @@ public static String getVocabularyIri(final Set conceptUris, String term if (conceptUris.isEmpty()) { throw new IllegalArgumentException("No namespace candidate."); } - final Iterator i = conceptUris.iterator(); - final String conceptUri = i.next(); + final String namespace = extractNamespace(termSeparator, conceptUri); + for (final String s : conceptUris) { + if (!s.startsWith(namespace)) { + throw new IllegalArgumentException( + "Not all Concept IRIs have the same namespace: " + conceptUri + " vs. " + namespace); + } + } + return namespace; + } + private static String extractNamespace(String termSeparator, String conceptUri) { final String separator; - if (conceptUri.lastIndexOf(termSeparator) > 0) { separator = termSeparator; } else if (conceptUri.lastIndexOf("#") > 0) { @@ -210,16 +218,7 @@ public static String getVocabularyIri(final Set conceptUris, String term } else { throw new IllegalArgumentException("The IRI does not have a proper format: " + conceptUri); } - - final String namespace = conceptUri.substring(0, conceptUri.lastIndexOf(separator)); - - for (final String s : conceptUris) { - if (!s.startsWith(namespace)) { - throw new IllegalArgumentException( - "Not all Concept IRIs have the same namespace: " + conceptUri + " vs. " + namespace); - } - } - return namespace; + return conceptUri.substring(0, conceptUri.lastIndexOf(separator)); } /** @@ -402,15 +401,25 @@ public static void pruneBlankTranslations(MultilingualString str) { /** * Converts the map into a string - * @return Empty string when the map is {@code null}, otherwise the String in format - * {@code {key=value, key=value}} + * + * @return Empty string when the map is {@code null}, otherwise the String in format {@code {key=value, key=value}} */ public static String mapToString(Map map) { if (map == null) { return ""; } return map.keySet().stream() - .map(key -> key + "=" + map.get(key)) - .collect(Collectors.joining(", ", "{", "}")); + .map(key -> key + "=" + map.get(key)) + .collect(Collectors.joining(", ", "{", "}")); + } + + /** + * Checks whether the {@code development} profile is active. + * + * @param activeProfiles Array of active profiles + * @return {@code true} if the {@code development} profile is active, {@code false} otherwise + */ + public static boolean isDevelopmentProfile(String[] activeProfiles) { + return Arrays.binarySearch(activeProfiles, Constants.DEVELOPMENT_PROFILE) != -1; } } From 5fbccba0d7b67711c9535693cfeca0d563a06da4 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Wed, 13 Nov 2024 16:30:00 +0100 Subject: [PATCH 06/14] [GH-309] Remove text analysis service URL from application.yml It is not necessary in development. When one wants to test TermIt with text analysis, they should configure the real URL of the service. --- src/main/resources/application.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 8d9cae801..655043d51 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -66,8 +66,6 @@ termit: separator: /verze file: storage: /tmp/termit - textAnalysis: - url: http://localhost:8081/annotace/annotate changetracking: context: extension: /zmeny From 742a0174dcdc0da9826249710cb050dd70be0255 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Wed, 13 Nov 2024 16:32:01 +0100 Subject: [PATCH 07/14] [GH-309] Document the development Spring profile. --- doc/setup.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/setup.md b/doc/setup.md index 839fe0b3c..af068c823 100644 --- a/doc/setup.md +++ b/doc/setup.md @@ -39,6 +39,7 @@ by the application: * `lucene` - decides whether Lucene text indexing is enabled and should be used in full text search queries. * `admin-registration-only` - decides whether new users can be registered only by application admin, or whether anyone can register. * `no-cache` - disables Ehcache, which is used to cache lists of resources and vocabularies for faster retrieval, and persistence cache. +* `development` - indicates that the application is running is development. This, for example, means that mail server does not need to be configured. The `lucene` Spring profile is activated automatically by the `graphdb` Maven. `admin-registration-only` and `no-cache` have to be added either in `application.yml` directly, or one can pass the parameter to Maven build, e.g.: From f44f7b08f1f2ac4f6922b74797f547845c76a3ea Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Tue, 19 Nov 2024 10:03:45 +0100 Subject: [PATCH 08/14] [kbss-cvut/termit-ui#553] Check if text analysis service supports file language before annotation. --- ...upportedTextAnalysisLanguageException.java | 14 ++++ .../business/AccessControlListService.java | 2 +- .../service/business/ResourceService.java | 8 ++ .../service/document/TextAnalysisService.java | 77 +++++++++++++++++-- .../cvut/kbss/termit/util/Configuration.java | 13 ++++ .../handler/WebSocketExceptionHandler.java | 16 +++- src/main/resources/application.yml | 1 + .../service/business/ResourceServiceTest.java | 18 +++++ .../document/TextAnalysisServiceTest.java | 54 ++++++++++++- .../cvut/kbss/termit/util/VocabularyTest.java | 1 - src/test/resources/application.yml | 3 +- 11 files changed, 195 insertions(+), 12 deletions(-) create mode 100644 src/main/java/cz/cvut/kbss/termit/exception/UnsupportedTextAnalysisLanguageException.java diff --git a/src/main/java/cz/cvut/kbss/termit/exception/UnsupportedTextAnalysisLanguageException.java b/src/main/java/cz/cvut/kbss/termit/exception/UnsupportedTextAnalysisLanguageException.java new file mode 100644 index 000000000..3ddb95c60 --- /dev/null +++ b/src/main/java/cz/cvut/kbss/termit/exception/UnsupportedTextAnalysisLanguageException.java @@ -0,0 +1,14 @@ +package cz.cvut.kbss.termit.exception; + +import cz.cvut.kbss.termit.model.Asset; +import cz.cvut.kbss.termit.model.resource.File; + +/** + * Indicates that a language is not supported by the text analysis service. + */ +public class UnsupportedTextAnalysisLanguageException extends TermItException { + + public UnsupportedTextAnalysisLanguageException(String message, Asset asset) { + super(message, asset instanceof File ? "error.annotation.file.unsupportedLanguage" : "error.annotation.term.unsupportedLanguage"); + } +} diff --git a/src/main/java/cz/cvut/kbss/termit/service/business/AccessControlListService.java b/src/main/java/cz/cvut/kbss/termit/service/business/AccessControlListService.java index 4b6cdc889..c2b5772af 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/business/AccessControlListService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/business/AccessControlListService.java @@ -32,7 +32,7 @@ /** * Service for managing {@link AccessControlList}s (ACLs). *

- * Note that only management of ACLs is supported by this service. Access control itself is handled by TODO. + * Note that only management of ACLs is supported by this service. Access control itself is handled by {@link cz.cvut.kbss.termit.service.security.authorization.acl.AccessControlListBasedAuthorizationService}. */ public interface AccessControlListService { diff --git a/src/main/java/cz/cvut/kbss/termit/service/business/ResourceService.java b/src/main/java/cz/cvut/kbss/termit/service/business/ResourceService.java index 08aee833d..0156a8738 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/business/ResourceService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/business/ResourceService.java @@ -24,6 +24,7 @@ import cz.cvut.kbss.termit.exception.InvalidParameterException; import cz.cvut.kbss.termit.exception.NotFoundException; import cz.cvut.kbss.termit.exception.UnsupportedAssetOperationException; +import cz.cvut.kbss.termit.exception.UnsupportedTextAnalysisLanguageException; import cz.cvut.kbss.termit.model.TextAnalysisRecord; import cz.cvut.kbss.termit.model.Vocabulary; import cz.cvut.kbss.termit.model.changetracking.AbstractChangeRecord; @@ -300,6 +301,7 @@ public void runTextAnalysis(Resource resource, Set vocabularies) { verifyFileOperationPossible(resource, "Text analysis"); LOG.trace("Invoking text analysis on resource {}.", resource); final File file = (File) resource; + verifyLanguageSupported(file); if (vocabularies.isEmpty()) { if (file.getDocument() == null || file.getDocument().getVocabulary() == null) { throw new UnsupportedAssetOperationException( @@ -313,6 +315,12 @@ public void runTextAnalysis(Resource resource, Set vocabularies) { } } + private void verifyLanguageSupported(File file) { + if (!textAnalysisService.supportsLanguage(file)) { + throw new UnsupportedTextAnalysisLanguageException("Text analysis service does not support language " + file.getLanguage(), file); + } + } + private Set includeImportedVocabularies(Set providedVocabularies) { final Set result = new HashSet<>(providedVocabularies); providedVocabularies.forEach(uri -> { diff --git a/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java b/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java index 12bddd7e4..18da62044 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java @@ -20,11 +20,15 @@ import cz.cvut.kbss.termit.dto.TextAnalysisInput; import cz.cvut.kbss.termit.event.FileTextAnalysisFinishedEvent; import cz.cvut.kbss.termit.event.TermDefinitionTextAnalysisFinishedEvent; +import cz.cvut.kbss.termit.exception.TermItException; +import cz.cvut.kbss.termit.exception.UnsupportedTextAnalysisLanguageException; import cz.cvut.kbss.termit.exception.WebServiceIntegrationException; import cz.cvut.kbss.termit.model.AbstractTerm; +import cz.cvut.kbss.termit.model.Asset; import cz.cvut.kbss.termit.model.TextAnalysisRecord; import cz.cvut.kbss.termit.model.resource.File; import cz.cvut.kbss.termit.persistence.dao.TextAnalysisRecordDao; +import cz.cvut.kbss.termit.rest.handler.ErrorInfo; import cz.cvut.kbss.termit.util.Configuration; import cz.cvut.kbss.termit.util.Utils; import cz.cvut.kbss.termit.util.throttle.Throttle; @@ -32,20 +36,24 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.core.ParameterizedTypeReference; import org.springframework.core.io.Resource; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; import java.io.IOException; import java.io.InputStream; import java.net.URI; import java.util.HashSet; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -67,6 +75,8 @@ public class TextAnalysisService { private final ApplicationEventPublisher eventPublisher; + private Set supportedLanguages; + @Autowired public TextAnalysisService(RestTemplate restClient, Configuration config, DocumentManager documentManager, AnnotationGenerator annotationGenerator, TextAnalysisRecordDao recordDao, @@ -126,6 +136,8 @@ private void invokeTextAnalysisOnFile(File file, TextAnalysisInput input) { storeTextAnalysisRecord(file, input); } catch (WebServiceIntegrationException e) { throw e; + } catch (HttpClientErrorException e) { + throw handleTextAnalysisInvocationClientException(e, file); } catch (RuntimeException e) { throw new WebServiceIntegrationException("Text analysis invocation failed.", e); } catch (IOException e) { @@ -140,11 +152,10 @@ private Optional invokeTextAnalysisService(TextAnalysisInput input) { return Optional.empty(); } final HttpHeaders headers = new HttpHeaders(); - headers.add(HttpHeaders.ACCEPT, MediaType.APPLICATION_XML_VALUE); - LOG.debug("Invoking text analysis service at '{}' on input: {}", config.getTextAnalysis().getUrl(), input); - final ResponseEntity resp = restClient - .exchange(config.getTextAnalysis().getUrl(), HttpMethod.POST, - new HttpEntity<>(input, headers), Resource.class); + headers.addAll(HttpHeaders.ACCEPT, List.of(MediaType.APPLICATION_JSON_VALUE, MediaType.APPLICATION_XML_VALUE)); + LOG.debug("Invoking text analysis service at '{}' on input: {}", taUrl, input); + final ResponseEntity resp = restClient.exchange(taUrl, HttpMethod.POST, + new HttpEntity<>(input, headers), Resource.class); if (!resp.hasBody()) { throw new WebServiceIntegrationException("Text analysis service returned empty response."); } @@ -161,6 +172,16 @@ private void storeTextAnalysisRecord(File file, TextAnalysisInput config) { recordDao.persist(record); } + private TermItException handleTextAnalysisInvocationClientException(HttpClientErrorException ex, Asset asset) { + if (ex.getStatusCode() == HttpStatus.CONFLICT) { + final ErrorInfo errorInfo = ex.getResponseBodyAs(ErrorInfo.class); + if (errorInfo != null && errorInfo.getMessage().contains("language")) { + throw new UnsupportedTextAnalysisLanguageException(errorInfo.getMessage(),asset); + } + } + throw new WebServiceIntegrationException("Text analysis invocation failed.", ex); + } + /** * Gets the latest {@link TextAnalysisRecord} for the specified Resource. * @@ -205,10 +226,56 @@ private void invokeTextAnalysisOnTerm(AbstractTerm term, TextAnalysisInput input } } catch (WebServiceIntegrationException e) { throw e; + } catch (HttpClientErrorException e) { + throw handleTextAnalysisInvocationClientException(e, term); } catch (RuntimeException e) { throw new WebServiceIntegrationException("Text analysis invocation failed.", e); } catch (IOException e) { throw new WebServiceIntegrationException("Unable to read text analysis result from response.", e); } } + + /** + * Checks whether the text analysis service supports the language of the specified file. + *

+ * If the text analysis service does not provide endpoint for getting supported languages (or it is not configured), + * it is assumed that any language is supported. + *

+ * If the file does not have language set, it is assumed that it is supported as well. + * + * @param file File to be analyzed + * @return {@code true} if the file language is supported, {@code false} otherwise + */ + public boolean supportsLanguage(File file) { + Objects.requireNonNull(file); + return file.getLanguage() == null || getSupportedLanguages().isEmpty() || getSupportedLanguages().contains( + file.getLanguage()); + } + + private synchronized Set getSupportedLanguages() { + if (supportedLanguages != null) { + return supportedLanguages; + } + final String languagesEndpointUrl = config.getTextAnalysis().getLanguagesUrl(); + if (languagesEndpointUrl == null || languagesEndpointUrl.isBlank()) { + LOG.warn( + "Text analysis service languages endpoint URL not configured. Assuming any language is supported."); + this.supportedLanguages = Set.of(); + } else { + try { + LOG.debug("Getting list of supported languages from text analysis service at '{}'.", + languagesEndpointUrl); + ResponseEntity> response = restClient.exchange(languagesEndpointUrl, HttpMethod.GET, null, + new ParameterizedTypeReference<>() { + }); + this.supportedLanguages = response.getBody(); + LOG.trace("Text analysis supported languages: {}", supportedLanguages); + } catch (RuntimeException e) { + LOG.error("Unable to get list of supported languages from text analysis service at '{}'.", + languagesEndpointUrl, e); + this.supportedLanguages = Set.of(); + } + } + return supportedLanguages; + } } diff --git a/src/main/java/cz/cvut/kbss/termit/util/Configuration.java b/src/main/java/cz/cvut/kbss/termit/util/Configuration.java index 8a655df59..4785f9eb6 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/Configuration.java +++ b/src/main/java/cz/cvut/kbss/termit/util/Configuration.java @@ -673,6 +673,11 @@ public static class TextAnalysis { */ private String url; + /** + * URL of the endpoint providing list of languages supported by the text analysis service. + */ + private String languagesUrl; + /** * Score threshold for a term occurrence for it to be saved into the repository. */ @@ -693,6 +698,14 @@ public void setUrl(String url) { this.url = url; } + public String getLanguagesUrl() { + return languagesUrl; + } + + public void setLanguagesUrl(String languagesUrl) { + this.languagesUrl = languagesUrl; + } + public String getTermOccurrenceMinScore() { return termOccurrenceMinScore; } diff --git a/src/main/java/cz/cvut/kbss/termit/websocket/handler/WebSocketExceptionHandler.java b/src/main/java/cz/cvut/kbss/termit/websocket/handler/WebSocketExceptionHandler.java index c5869701b..c6042bb9a 100644 --- a/src/main/java/cz/cvut/kbss/termit/websocket/handler/WebSocketExceptionHandler.java +++ b/src/main/java/cz/cvut/kbss/termit/websocket/handler/WebSocketExceptionHandler.java @@ -19,6 +19,7 @@ import cz.cvut.kbss.termit.exception.TermItException; import cz.cvut.kbss.termit.exception.UnsupportedOperationException; import cz.cvut.kbss.termit.exception.UnsupportedSearchFacetException; +import cz.cvut.kbss.termit.exception.UnsupportedTextAnalysisLanguageException; import cz.cvut.kbss.termit.exception.ValidationException; import cz.cvut.kbss.termit.exception.WebServiceIntegrationException; import cz.cvut.kbss.termit.exception.importing.UnsupportedImportMediaTypeException; @@ -87,7 +88,8 @@ private static ErrorInfo errorInfo(Message message, Throwable e) { } private static ErrorInfo errorInfo(Message message, TermItException e) { - return ErrorInfo.createParametrizedWithMessage(e.getMessage(), e.getMessageId(), destination(message), e.getParameters()); + return ErrorInfo.createParametrizedWithMessage(e.getMessage(), e.getMessageId(), destination(message), + e.getParameters()); } @MessageExceptionHandler @@ -95,7 +97,7 @@ public void messageDeliveryException(Message message, MessageDeliveryExceptio // messages without destination will be logged only on trace (hasDestination(message) ? LOG.atError() : LOG.atTrace()) .setMessage("Failed to send message with destination {}: {}") - .addArgument(()-> destination(message)) + .addArgument(() -> destination(message)) .addArgument(e.getMessage()) .setCause(e.getCause()) .log(); @@ -226,7 +228,8 @@ public ErrorInfo invalidParameter(Message message, InvalidParameterException @MessageExceptionHandler public ErrorInfo maxUploadSizeExceededException(Message message, MaxUploadSizeExceededException e) { logException(e, message); - return ErrorInfo.createWithMessageAndMessageId(e.getMessage(), "error.file.maxUploadSizeExceeded", destination(message)); + return ErrorInfo.createWithMessageAndMessageId(e.getMessage(), "error.file.maxUploadSizeExceeded", + destination(message)); } @MessageExceptionHandler @@ -271,4 +274,11 @@ public ErrorInfo uriSyntaxException(Message message, URISyntaxException e) { logException(e, message); return errorInfo(message, e); } + + @MessageExceptionHandler + public ErrorInfo unsupportedTextAnalysisLanguageException(Message message, + UnsupportedTextAnalysisLanguageException e) { + logException(e, message); + return errorInfo(message, e); + } } diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 8d9cae801..b6fa55209 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -68,6 +68,7 @@ termit: storage: /tmp/termit textAnalysis: url: http://localhost:8081/annotace/annotate + languagesUrl: http://localhost:8081/annotace/languages changetracking: context: extension: /zmeny diff --git a/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java b/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java index 2777f42fe..cae57e7e8 100644 --- a/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java +++ b/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java @@ -24,6 +24,7 @@ import cz.cvut.kbss.termit.exception.NotFoundException; import cz.cvut.kbss.termit.exception.TermItException; import cz.cvut.kbss.termit.exception.UnsupportedAssetOperationException; +import cz.cvut.kbss.termit.exception.UnsupportedTextAnalysisLanguageException; import cz.cvut.kbss.termit.model.TextAnalysisRecord; import cz.cvut.kbss.termit.model.Vocabulary; import cz.cvut.kbss.termit.model.changetracking.AbstractChangeRecord; @@ -203,6 +204,7 @@ void runTextAnalysisInvokesTextAnalysisWithVocabularyRelatedToFilesDocument() { file.setDocument(Generator.generateDocumentWithId()); final Vocabulary vocabulary = Generator.generateVocabularyWithId(); file.getDocument().setVocabulary(vocabulary.getUri()); + when(textAnalysisService.supportsLanguage(file)).thenReturn(true); sut.runTextAnalysis(file, Collections.emptySet()); verify(textAnalysisService).analyzeFile(file, Collections.singleton(vocabulary.getUri())); } @@ -218,6 +220,7 @@ void runTextAnalysisThrowsUnsupportedAssetOperationWhenResourceIsNotFile() { @Test void runTextAnalysisThrowsUnsupportedAssetOperationWhenFileHasNoVocabularyAndNoVocabulariesAreSpecifiedEither() { final File file = Generator.generateFileWithId("test.html"); + when(textAnalysisService.supportsLanguage(file)).thenReturn(true); assertThrows(UnsupportedAssetOperationException.class, () -> sut.runTextAnalysis(file, Collections.emptySet())); verify(textAnalysisService, never()).analyzeFile(any(), anySet()); @@ -227,6 +230,7 @@ void runTextAnalysisThrowsUnsupportedAssetOperationWhenFileHasNoVocabularyAndNoV void runTextAnalysisInvokesAnalysisWithCustomVocabulariesWhenSpecified() { final File file = Generator.generateFileWithId("test.html"); final Set vocabularies = new HashSet<>(Arrays.asList(Generator.generateUri(), Generator.generateUri())); + when(textAnalysisService.supportsLanguage(file)).thenReturn(true); sut.runTextAnalysis(file, vocabularies); verify(textAnalysisService).analyzeFile(file, vocabularies); } @@ -240,6 +244,7 @@ void runTextAnalysisInvokesAnalysisAlsoWithImportedVocabulariesOfVocabularyRElat final Set imported = new HashSet<>(Arrays.asList(Generator.generateUri(), Generator.generateUri())); when(vocabularyService.getReference(vocabulary.getUri())).thenReturn(vocabulary); when(vocabularyService.getTransitivelyImportedVocabularies(vocabulary)).thenReturn(imported); + when(textAnalysisService.supportsLanguage(file)).thenReturn(true); sut.runTextAnalysis(file, Collections.emptySet()); final Set expected = new HashSet<>(imported); @@ -259,6 +264,7 @@ void runTextAnalysisInvokesAnalysisWithProvidedVocabulariesAndTheirImports() { when(vocabularyService.getTransitivelyImportedVocabularies(vOne)).thenReturn(vOneImports); when(vocabularyService.getReference(vTwo.getUri())).thenReturn(vTwo); when(vocabularyService.getTransitivelyImportedVocabularies(vTwo)).thenReturn(vTwoImports); + when(textAnalysisService.supportsLanguage(file)).thenReturn(true); sut.runTextAnalysis(file, new HashSet<>(Arrays.asList(vOne.getUri(), vTwo.getUri()))); final Set expected = new HashSet<>(vOneImports); @@ -554,4 +560,16 @@ void addFileToDocumentDoesNotModifyLanguageWhenItIsAlreadySet() { verify(resourceRepositoryService).persist(file, vocabulary); assertEquals("cs", file.getLanguage()); } + + @Test + void runTextAnalysisThrowsUnsupportedTextAnalysisExceptionWhenTextAnalysisServiceDoesNotSupportFileLanguage() { + final File file = Generator.generateFileWithId("test.html"); + file.setDocument(Generator.generateDocumentWithId()); + final Vocabulary vocabulary = Generator.generateVocabularyWithId(); + file.getDocument().setVocabulary(vocabulary.getUri()); + file.setLanguage("sk"); + when(textAnalysisService.supportsLanguage(file)).thenReturn(false); + assertThrows(UnsupportedTextAnalysisLanguageException.class, () -> sut.runTextAnalysis(file, Set.of(vocabulary.getUri()))); + verify(textAnalysisService).supportsLanguage(file); + } } diff --git a/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java b/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java index 6bdc27284..794753204 100644 --- a/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java +++ b/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java @@ -27,14 +27,17 @@ import cz.cvut.kbss.termit.event.FileTextAnalysisFinishedEvent; import cz.cvut.kbss.termit.event.TermDefinitionTextAnalysisFinishedEvent; import cz.cvut.kbss.termit.exception.NotFoundException; +import cz.cvut.kbss.termit.exception.UnsupportedTextAnalysisLanguageException; import cz.cvut.kbss.termit.exception.WebServiceIntegrationException; import cz.cvut.kbss.termit.model.Term; import cz.cvut.kbss.termit.model.TextAnalysisRecord; import cz.cvut.kbss.termit.model.Vocabulary; import cz.cvut.kbss.termit.model.resource.File; import cz.cvut.kbss.termit.persistence.dao.TextAnalysisRecordDao; +import cz.cvut.kbss.termit.rest.handler.ErrorInfo; import cz.cvut.kbss.termit.service.BaseServiceTestRunner; import cz.cvut.kbss.termit.util.Configuration; +import cz.cvut.kbss.termit.util.Constants; import cz.cvut.kbss.termit.util.Utils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -70,6 +73,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -87,6 +91,7 @@ import static org.springframework.test.web.client.match.MockRestRequestMatchers.jsonPath; import static org.springframework.test.web.client.match.MockRestRequestMatchers.method; import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; +import static org.springframework.test.web.client.response.MockRestResponseCreators.withRequestConflict; import static org.springframework.test.web.client.response.MockRestResponseCreators.withServerError; import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; @@ -203,7 +208,7 @@ void analyzeFilePassesContentTypeAndAcceptHeadersToService() throws Exception { .andExpect(method(HttpMethod.POST)) .andExpect(content().string(objectMapper.writeValueAsString(input))) .andExpect(header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)) - .andExpect(header(HttpHeaders.ACCEPT, MediaType.APPLICATION_XML_VALUE)) + .andExpect(header(HttpHeaders.ACCEPT,MediaType.APPLICATION_JSON_VALUE, MediaType.APPLICATION_XML_VALUE)) .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); sut.analyzeFile(file, Collections.singleton(vocabulary.getUri())); mockServer.verify(); @@ -478,4 +483,51 @@ void analyzeFileUsesConfiguredPersistenceLanguageInTextAnalysisInvocationInputWh sut.analyzeFile(file, Collections.singleton(vocabulary.getUri())); mockServer.verify(); } + + @Test + void analyzeFileThrowsUnsupportedLanguageExceptionWhenTextAnalysisInvocationReturnsConflictWithUnsupportedLanguageError() + throws Exception { + file.setLanguage("de"); + final ErrorInfo respBody = ErrorInfo.createWithMessage("No taggers for language 'de' available.", + "/annotace/annotate"); + mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) + .andExpect(method(HttpMethod.POST)) + .andRespond(withRequestConflict().body(objectMapper.writeValueAsString(respBody)) + .contentType(MediaType.APPLICATION_JSON)); + + final UnsupportedTextAnalysisLanguageException ex = assertThrows(UnsupportedTextAnalysisLanguageException.class, + () -> sut.analyzeFile(file, + Collections.singleton( + vocabulary.getUri()))); + assertEquals("error.annotation.file.unsupportedLanguage", ex.getMessageId()); + } + + @Test + void supportsLanguageGetsListOfSupportedLanguagesFromTextAnalysisServiceAndChecksIfFileLanguageIsAmongThem() { + file.setLanguage("cs"); + mockServer.expect(requestTo(config.getTextAnalysis().getLanguagesUrl())) + .andExpect(method(HttpMethod.GET)) + .andRespond(withSuccess("[\"cs\", \"en\"]", MediaType.APPLICATION_JSON)); + assertTrue(sut.supportsLanguage(file)); + mockServer.verify(); + + file.setLanguage("de"); + assertFalse(sut.supportsLanguage(file)); + } + + @Test + void supportsLanguageReturnsTrueWhenTextAnalysisServiceLanguagesEndpointUrlIsNotConfigured() { + String endpointUrl = config.getTextAnalysis().getLanguagesUrl(); + file.setLanguage(Constants.DEFAULT_LANGUAGE); + config.getTextAnalysis().setLanguagesUrl(null); + assertTrue(sut.supportsLanguage(file)); + // Reset configuration state + config.getTextAnalysis().setLanguagesUrl(endpointUrl); + } + + @Test + void supportsLanguageReturnsTrueWhenFileHasNoLanguageSet() { + file.setLanguage(null); + assertTrue(sut.supportsLanguage(file)); + } } diff --git a/src/test/java/cz/cvut/kbss/termit/util/VocabularyTest.java b/src/test/java/cz/cvut/kbss/termit/util/VocabularyTest.java index a35fd6534..7c4d6aac9 100644 --- a/src/test/java/cz/cvut/kbss/termit/util/VocabularyTest.java +++ b/src/test/java/cz/cvut/kbss/termit/util/VocabularyTest.java @@ -23,7 +23,6 @@ public class VocabularyTest { @Test - // @todo until https://github.com/kbss-cvut/jopa/issues/85 is resolved public void ensureContentHasCorrectUrl() { Assert.equals("http://rdfs.org/sioc/ns#content", Vocabulary.s_p_sioc_content); } diff --git a/src/test/resources/application.yml b/src/test/resources/application.yml index 9365e2b7f..258bfa622 100644 --- a/src/test/resources/application.yml +++ b/src/test/resources/application.yml @@ -30,7 +30,8 @@ termit: file: storage: /tmp/termit textAnalysis: - url: http://localhost/annotace + url: http://localhost/annotace/annotate + languagesUrl: http://localhost/annotace/languages termOccurrenceMinScore: 0.49 comments: context: http://onto.fel.cvut.cz/ontologies/komentare From 58c10e23e3d3399df67cd0f10e9dd5583852f3ff Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Tue, 19 Nov 2024 10:10:38 +0100 Subject: [PATCH 09/14] [kbss-cvut/termit-ui#553] Handle unsupported text analysis language exception - return 409 status. --- .../termit/rest/handler/RestExceptionHandler.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/java/cz/cvut/kbss/termit/rest/handler/RestExceptionHandler.java b/src/main/java/cz/cvut/kbss/termit/rest/handler/RestExceptionHandler.java index 0ea71c47c..53ba971a6 100644 --- a/src/main/java/cz/cvut/kbss/termit/rest/handler/RestExceptionHandler.java +++ b/src/main/java/cz/cvut/kbss/termit/rest/handler/RestExceptionHandler.java @@ -36,6 +36,7 @@ import cz.cvut.kbss.termit.exception.TermItException; import cz.cvut.kbss.termit.exception.UnsupportedOperationException; import cz.cvut.kbss.termit.exception.UnsupportedSearchFacetException; +import cz.cvut.kbss.termit.exception.UnsupportedTextAnalysisLanguageException; import cz.cvut.kbss.termit.exception.ValidationException; import cz.cvut.kbss.termit.exception.WebServiceIntegrationException; import cz.cvut.kbss.termit.exception.importing.UnsupportedImportMediaTypeException; @@ -99,7 +100,8 @@ private static ErrorInfo errorInfo(HttpServletRequest request, Throwable e) { } private static ErrorInfo errorInfo(HttpServletRequest request, TermItException e) { - return ErrorInfo.createParametrizedWithMessage(e.getMessage(), e.getMessageId(), request.getRequestURI(), e.getParameters()); + return ErrorInfo.createParametrizedWithMessage(e.getMessage(), e.getMessageId(), request.getRequestURI(), + e.getParameters()); } @ExceptionHandler(PersistenceException.class) @@ -290,4 +292,11 @@ public ResponseEntity uriSyntaxException(HttpServletRequest request, .addParameter("char", Character.toString(e.getInput().charAt(e.getIndex()))); return new ResponseEntity<>(errorInfo(request, exception), HttpStatus.CONFLICT); } + + @ExceptionHandler + public ResponseEntity unsupportedTextAnalysisLanguageException(HttpServletRequest request, + UnsupportedTextAnalysisLanguageException e) { + logException(e, request); + return new ResponseEntity<>(errorInfo(request, e), HttpStatus.CONFLICT); + } } From bf2c16dadb73a8266e70161f169ebc22f915b300 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Wed, 20 Nov 2024 09:58:28 +0100 Subject: [PATCH 10/14] [kbss-cvut/termit-ui#553] Add language to TextAnalysisRecord. --- .../kbss/termit/model/TextAnalysisRecord.java | 22 ++++++++++++++++--- .../service/document/TextAnalysisService.java | 2 +- .../TextAnalysisRecordDaoTest.java | 5 +++-- .../termit/rest/ResourceControllerTest.java | 2 +- .../service/business/ResourceServiceTest.java | 7 +++--- .../document/TextAnalysisServiceTest.java | 7 +++--- 6 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/model/TextAnalysisRecord.java b/src/main/java/cz/cvut/kbss/termit/model/TextAnalysisRecord.java index 837e55280..fe8dfe13d 100644 --- a/src/main/java/cz/cvut/kbss/termit/model/TextAnalysisRecord.java +++ b/src/main/java/cz/cvut/kbss/termit/model/TextAnalysisRecord.java @@ -17,10 +17,12 @@ */ package cz.cvut.kbss.termit.model; +import cz.cvut.kbss.jopa.model.annotations.OWLAnnotationProperty; import cz.cvut.kbss.jopa.model.annotations.OWLClass; import cz.cvut.kbss.jopa.model.annotations.OWLDataProperty; import cz.cvut.kbss.jopa.model.annotations.OWLObjectProperty; import cz.cvut.kbss.jopa.model.annotations.ParticipationConstraints; +import cz.cvut.kbss.jopa.vocabulary.DC; import cz.cvut.kbss.termit.model.resource.Resource; import cz.cvut.kbss.termit.util.Vocabulary; @@ -44,12 +46,16 @@ public class TextAnalysisRecord extends AbstractEntity { @OWLObjectProperty(iri = Vocabulary.s_p_ma_slovnik_pro_analyzu) private Set vocabularies; + @OWLAnnotationProperty(iri = DC.Terms.LANGUAGE, simpleLiteral = true) + private String language; + public TextAnalysisRecord() { } - public TextAnalysisRecord(Instant date, Resource analyzedResource) { + public TextAnalysisRecord(Instant date, Resource analyzedResource, String language) { this.date = date; this.analyzedResource = analyzedResource; + this.language = language; } public Instant getDate() { @@ -76,6 +82,14 @@ public void setVocabularies(Set vocabularies) { this.vocabularies = vocabularies; } + public String getLanguage() { + return language; + } + + public void setLanguage(String language) { + this.language = language; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -86,12 +100,13 @@ public boolean equals(Object o) { } return Objects.equals(date, that.date) && Objects.equals(analyzedResource, that.analyzedResource) && - Objects.equals(vocabularies, that.vocabularies); + Objects.equals(vocabularies, that.vocabularies) && + Objects.equals(language, that.language); } @Override public int hashCode() { - return Objects.hash(date, analyzedResource, vocabularies); + return Objects.hash(date, analyzedResource, vocabularies, language); } @Override @@ -100,6 +115,7 @@ public String toString() { "date=" + date + ",analyzedResource=" + analyzedResource + ",vocabularies=" + vocabularies + + ", language=" + language + "}"; } } diff --git a/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java b/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java index 18da62044..6ef927e72 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/document/TextAnalysisService.java @@ -167,7 +167,7 @@ private void storeTextAnalysisRecord(File file, TextAnalysisInput config) { LOG.trace("Creating record of text analysis event for file {}.", file); assert config.getVocabularyContexts() != null; - final TextAnalysisRecord record = new TextAnalysisRecord(Utils.timestamp(), file); + final TextAnalysisRecord record = new TextAnalysisRecord(Utils.timestamp(), file, config.getLanguage()); record.setVocabularies(new HashSet<>(config.getVocabularyContexts())); recordDao.persist(record); } diff --git a/src/test/java/cz/cvut/kbss/termit/persistence/TextAnalysisRecordDaoTest.java b/src/test/java/cz/cvut/kbss/termit/persistence/TextAnalysisRecordDaoTest.java index 7eb5a23e8..cc0c320d3 100644 --- a/src/test/java/cz/cvut/kbss/termit/persistence/TextAnalysisRecordDaoTest.java +++ b/src/test/java/cz/cvut/kbss/termit/persistence/TextAnalysisRecordDaoTest.java @@ -63,9 +63,10 @@ void setUp() { @Test void findLatestGetsLatestTextAnalysisRecordForResource() { final URI vocabulary = Generator.generateUri(); - final TextAnalysisRecord old = new TextAnalysisRecord(Instant.ofEpochMilli(System.currentTimeMillis() - 10000), resource); + final TextAnalysisRecord old = new TextAnalysisRecord(Instant.ofEpochMilli(System.currentTimeMillis() - 10000), + resource, Environment.LANGUAGE); old.setVocabularies(Collections.singleton(vocabulary)); - final TextAnalysisRecord latest = new TextAnalysisRecord(Utils.timestamp(), resource); + final TextAnalysisRecord latest = new TextAnalysisRecord(Utils.timestamp(), resource, Environment.LANGUAGE); latest.setVocabularies(Collections.singleton(vocabulary)); transactional(() -> { sut.persist(old); diff --git a/src/test/java/cz/cvut/kbss/termit/rest/ResourceControllerTest.java b/src/test/java/cz/cvut/kbss/termit/rest/ResourceControllerTest.java index bd50b7258..2d136b8e1 100644 --- a/src/test/java/cz/cvut/kbss/termit/rest/ResourceControllerTest.java +++ b/src/test/java/cz/cvut/kbss/termit/rest/ResourceControllerTest.java @@ -326,7 +326,7 @@ void getLatestTextAnalysisRecordRetrievesAnalysisRecordFromService() throws Exce final File file = generateFile(); when(identifierResolverMock.resolveIdentifier(RESOURCE_NAMESPACE, FILE_NAME)).thenReturn(file.getUri()); when(resourceServiceMock.findRequired(file.getUri())).thenReturn(file); - final TextAnalysisRecord record = new TextAnalysisRecord(Utils.timestamp(), file); + final TextAnalysisRecord record = new TextAnalysisRecord(Utils.timestamp(), file, Environment.LANGUAGE); record.setVocabularies(Collections.singleton(Generator.generateUri())); when(resourceServiceMock.findLatestTextAnalysisRecord(file)).thenReturn(record); final MvcResult mvcResult = mockMvc.perform(get(PATH + "/" + FILE_NAME + "/text-analysis/records/latest") diff --git a/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java b/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java index cae57e7e8..ab93c17c7 100644 --- a/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java +++ b/src/test/java/cz/cvut/kbss/termit/service/business/ResourceServiceTest.java @@ -37,7 +37,6 @@ import cz.cvut.kbss.termit.service.repository.ChangeRecordService; import cz.cvut.kbss.termit.service.repository.ResourceRepositoryService; import cz.cvut.kbss.termit.util.Configuration; -import cz.cvut.kbss.termit.util.Constants; import cz.cvut.kbss.termit.util.TypeAwareByteArrayResource; import cz.cvut.kbss.termit.util.TypeAwareResource; import cz.cvut.kbss.termit.util.Utils; @@ -388,7 +387,7 @@ void removeFileThrowsTermItExceptionWhenFileIsNotLinkedToDocument() { @Test void findLatestTextAnalysisRecordRetrievesLatestTextAnalysisRecordForResource() { final File file = Generator.generateFileWithId("test.html"); - final TextAnalysisRecord record = new TextAnalysisRecord(Utils.timestamp(), file); + final TextAnalysisRecord record = new TextAnalysisRecord(Utils.timestamp(), file, Environment.LANGUAGE); when(textAnalysisService.findLatestAnalysisRecord(file)).thenReturn(Optional.of(record)); final TextAnalysisRecord result = sut.findLatestTextAnalysisRecord(file); @@ -530,7 +529,7 @@ void getContentWithoutUnconfirmedOccurrencesRemovesUnconfirmedOccurrencesFromFil @Test void addFileToDocumentSetsFileLanguageToDefaultConfiguredWhenNotProvided() { - config.getPersistence().setLanguage(Constants.DEFAULT_LANGUAGE); + config.getPersistence().setLanguage(Environment.LANGUAGE); final Vocabulary vocabulary = Generator.generateVocabularyWithId(); final Document document = Generator.generateDocumentWithId(); document.setVocabulary(vocabulary.getUri()); @@ -546,7 +545,7 @@ void addFileToDocumentSetsFileLanguageToDefaultConfiguredWhenNotProvided() { @Test void addFileToDocumentDoesNotModifyLanguageWhenItIsAlreadySet() { - config.getPersistence().setLanguage(Constants.DEFAULT_LANGUAGE); + config.getPersistence().setLanguage(Environment.LANGUAGE); final Vocabulary vocabulary = Generator.generateVocabularyWithId(); final Document document = Generator.generateDocumentWithId(); document.setVocabulary(vocabulary.getUri()); diff --git a/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java b/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java index 794753204..9a049a40a 100644 --- a/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java +++ b/src/test/java/cz/cvut/kbss/termit/service/document/TextAnalysisServiceTest.java @@ -37,7 +37,6 @@ import cz.cvut.kbss.termit.rest.handler.ErrorInfo; import cz.cvut.kbss.termit.service.BaseServiceTestRunner; import cz.cvut.kbss.termit.util.Configuration; -import cz.cvut.kbss.termit.util.Constants; import cz.cvut.kbss.termit.util.Utils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -326,6 +325,7 @@ void analyzeFileBacksUpFileContentBeforeSavingNewAnalyzedContent() throws Except @Test void analyzeFileCreatesTextAnalysisRecord() { + file.setLanguage("cs"); mockServer.expect(requestTo(config.getTextAnalysis().getUrl())) .andExpect(method(HttpMethod.POST)).andExpect(content().string(containsString(CONTENT))) .andRespond(withSuccess(CONTENT, MediaType.APPLICATION_XML)); @@ -334,11 +334,12 @@ void analyzeFileCreatesTextAnalysisRecord() { verify(textAnalysisRecordDao).persist(captor.capture()); assertEquals(file, captor.getValue().getAnalyzedResource()); assertEquals(Collections.singleton(vocabulary.getUri()), captor.getValue().getVocabularies()); + assertEquals(file.getLanguage(), captor.getValue().getLanguage()); } @Test void findLatestAnalysisRecordFindsLatestTextAnalysisRecordForResource() { - final TextAnalysisRecord record = new TextAnalysisRecord(Utils.timestamp(), file); + final TextAnalysisRecord record = new TextAnalysisRecord(Utils.timestamp(), file, Environment.LANGUAGE); record.setVocabularies(Collections.singleton(vocabulary.getUri())); when(textAnalysisRecordDao.findLatest(file)).thenReturn(Optional.of(record)); @@ -518,7 +519,7 @@ void supportsLanguageGetsListOfSupportedLanguagesFromTextAnalysisServiceAndCheck @Test void supportsLanguageReturnsTrueWhenTextAnalysisServiceLanguagesEndpointUrlIsNotConfigured() { String endpointUrl = config.getTextAnalysis().getLanguagesUrl(); - file.setLanguage(Constants.DEFAULT_LANGUAGE); + file.setLanguage(Environment.LANGUAGE); config.getTextAnalysis().setLanguagesUrl(null); assertTrue(sut.supportsLanguage(file)); // Reset configuration state From 851326211a408bcb2670be53fa5845b713fe782c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Ka=C5=88ka?= Date: Tue, 19 Nov 2024 17:41:52 +0100 Subject: [PATCH 11/14] [Bug #314] Fix ThrottledFuture throwing on "then action" when completed exceptionally --- .../kbss/termit/util/throttle/ThrottledFuture.java | 10 +++++++++- .../termit/util/throttle/ThrottledFutureTest.java | 12 ++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java b/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java index e32f8ef40..e6e492474 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java +++ b/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java @@ -245,7 +245,7 @@ public boolean isRunning() { public ThrottledFuture then(Consumer action) { try { callbackLock.lock(); - if (future.isDone() && !future.isCancelled()) { + if (future.isDone() && !future.isCancelled() && !future.isCompletedExceptionally()) { try { action.accept(future.get()); } catch (InterruptedException e) { @@ -262,4 +262,12 @@ public ThrottledFuture then(Consumer action) { } return this; } + + /** + * @return {@code true} if this future completed + * exceptionally + */ + public boolean isCompletedExceptionally() { + return future.isCompletedExceptionally(); + } } diff --git a/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java b/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java index bf8f4f4e0..039a2e3d8 100644 --- a/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java +++ b/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java @@ -138,6 +138,18 @@ void thenActionIsNotExecutedOnceFutureIsCancelled() { assertFalse(completed.get()); } + @Test + void thenActionIsNotExecutedWhenFutureCompletedExceptionally() { + final AtomicBoolean completed = new AtomicBoolean(false); + final ThrottledFuture future = ThrottledFuture.of(() -> { + throw new RuntimeException(); + }); + future.run(null); + assertFalse(completed.get()); + future.then(futureResult -> completed.set(true)); + assertFalse(completed.get()); + } + @Test void callingRunWillExecuteFutureOnlyOnce() { AtomicInteger count = new AtomicInteger(0); From 1462c058b789f41cced1e9619784d62131a4593e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Ka=C5=88ka?= Date: Wed, 20 Nov 2024 13:38:17 +0100 Subject: [PATCH 12/14] [Bug #314] Change ThrottledFuture#then to accept the completed future --- .../termit/persistence/dao/VocabularyDao.java | 4 +- .../service/business/VocabularyService.java | 4 +- .../VocabularyRepositoryService.java | 4 +- .../termit/util/throttle/CacheableFuture.java | 2 +- .../termit/util/throttle/ChainableFuture.java | 13 +++--- .../termit/util/throttle/ThrottledFuture.java | 41 ++++++++++--------- .../websocket/VocabularySocketController.java | 9 ++-- .../util/throttle/ThrottledFutureTest.java | 14 +++---- 8 files changed, 48 insertions(+), 43 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java b/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java index 9a9a7d734..2b215ed33 100644 --- a/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java +++ b/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java @@ -48,7 +48,7 @@ import cz.cvut.kbss.termit.service.snapshot.SnapshotProvider; import cz.cvut.kbss.termit.util.Configuration; import cz.cvut.kbss.termit.util.Utils; -import cz.cvut.kbss.termit.util.throttle.CacheableFuture; +import cz.cvut.kbss.termit.util.throttle.ThrottledFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -366,7 +366,7 @@ public void refreshLastModified(RefreshLastModifiedEvent event) { } @Transactional - public CacheableFuture> validateContents(URI vocabulary) { + public ThrottledFuture> validateContents(URI vocabulary) { final VocabularyContentValidator validator = context.getBean(VocabularyContentValidator.class); final Collection importClosure = getTransitivelyImportedVocabularies(vocabulary); importClosure.add(vocabulary); diff --git a/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java b/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java index 6f265656c..5ebc83804 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/business/VocabularyService.java @@ -46,8 +46,8 @@ import cz.cvut.kbss.termit.util.TypeAwareClasspathResource; import cz.cvut.kbss.termit.util.TypeAwareFileSystemResource; import cz.cvut.kbss.termit.util.TypeAwareResource; -import cz.cvut.kbss.termit.util.throttle.CacheableFuture; import cz.cvut.kbss.termit.util.throttle.Throttle; +import cz.cvut.kbss.termit.util.throttle.ThrottledFuture; import jakarta.annotation.Nonnull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -384,7 +384,7 @@ public void remove(Vocabulary asset) { * * @param vocabulary Vocabulary to validate */ - public CacheableFuture> validateContents(URI vocabulary) { + public ThrottledFuture> validateContents(URI vocabulary) { return repositoryService.validateContents(vocabulary); } diff --git a/src/main/java/cz/cvut/kbss/termit/service/repository/VocabularyRepositoryService.java b/src/main/java/cz/cvut/kbss/termit/service/repository/VocabularyRepositoryService.java index 6cffad957..8f9090820 100644 --- a/src/main/java/cz/cvut/kbss/termit/service/repository/VocabularyRepositoryService.java +++ b/src/main/java/cz/cvut/kbss/termit/service/repository/VocabularyRepositoryService.java @@ -42,7 +42,7 @@ import cz.cvut.kbss.termit.util.Configuration; import cz.cvut.kbss.termit.util.Constants; import cz.cvut.kbss.termit.util.Utils; -import cz.cvut.kbss.termit.util.throttle.CacheableFuture; +import cz.cvut.kbss.termit.util.throttle.ThrottledFuture; import cz.cvut.kbss.termit.workspace.EditableVocabularies; import jakarta.annotation.Nonnull; import jakarta.validation.Validator; @@ -334,7 +334,7 @@ private void ensureNoTermRelationsExists(Vocabulary vocabulary) throws AssetRemo } } - public CacheableFuture> validateContents(URI vocabulary) { + public ThrottledFuture> validateContents(URI vocabulary) { return vocabularyDao.validateContents(vocabulary); } diff --git a/src/main/java/cz/cvut/kbss/termit/util/throttle/CacheableFuture.java b/src/main/java/cz/cvut/kbss/termit/util/throttle/CacheableFuture.java index f1dd254a5..b6afe3872 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/throttle/CacheableFuture.java +++ b/src/main/java/cz/cvut/kbss/termit/util/throttle/CacheableFuture.java @@ -11,7 +11,7 @@ * A future which can provide a cached result before its completion. * @see Future */ -public interface CacheableFuture extends ChainableFuture { +public interface CacheableFuture extends Future { /** * @return the cached result when available diff --git a/src/main/java/cz/cvut/kbss/termit/util/throttle/ChainableFuture.java b/src/main/java/cz/cvut/kbss/termit/util/throttle/ChainableFuture.java index 0d8b63d6c..12d2b915e 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/throttle/ChainableFuture.java +++ b/src/main/java/cz/cvut/kbss/termit/util/throttle/ChainableFuture.java @@ -3,14 +3,15 @@ import java.util.concurrent.Future; import java.util.function.Consumer; -public interface ChainableFuture extends Future { +public interface ChainableFuture> extends Future { /** - * Executes this action once the future is completed normally. - * Action is not executed on exceptional completion. + * Executes this action once the future is completed. + * Action is executed no matter if the future is completed successfully, exceptionally or cancelled. *

- * If the future is already completed, action is executed synchronously. - * @param action action to be executed + * If the future is already completed, it is executed synchronously. + * @param action action receiving this future after completion + * @return this future */ - ChainableFuture then(Consumer action); + ChainableFuture then(Consumer action); } diff --git a/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java b/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java index e6e492474..b32e6e095 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java +++ b/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java @@ -1,6 +1,5 @@ package cz.cvut.kbss.termit.util.throttle; -import cz.cvut.kbss.termit.exception.TermItException; import cz.cvut.kbss.termit.util.Utils; import cz.cvut.kbss.termit.util.longrunning.LongRunningTask; import jakarta.annotation.Nonnull; @@ -20,7 +19,7 @@ import java.util.function.Consumer; import java.util.function.Supplier; -public class ThrottledFuture implements CacheableFuture, LongRunningTask { +public class ThrottledFuture implements CacheableFuture, ChainableFuture>, LongRunningTask { private final ReentrantLock lock = new ReentrantLock(); private final ReentrantLock callbackLock = new ReentrantLock(); @@ -33,7 +32,7 @@ public class ThrottledFuture implements CacheableFuture, LongRunningTask { private @Nullable Supplier task; - private final List> onCompletion = new ArrayList<>(); + private final List>> onCompletion = new ArrayList<>(); private final AtomicReference startedAt = new AtomicReference<>(null); @@ -90,7 +89,16 @@ public ThrottledFuture setCachedResult(@Nullable final T cachedResult) { @Override public boolean cancel(boolean mayInterruptIfRunning) { - return future.cancel(mayInterruptIfRunning); + if(!future.cancel(mayInterruptIfRunning)) { + return false; + } + + if (task != null) { + callbackLock.lock(); + onCompletion.forEach(c -> c.accept(this)); + callbackLock.unlock(); + } + return true; } @Override @@ -124,7 +132,7 @@ public T get(long timeout, @Nonnull TimeUnit unit) * @return If the current task is already running, was canceled or already completed, returns a new future for the given task. * Otherwise, replaces the current task and returns self. */ - protected ThrottledFuture update(Supplier task, @Nonnull List> onCompletion) { + protected ThrottledFuture update(Supplier task, @Nonnull List>> onCompletion) { boolean locked = false; try { locked = lock.tryLock(); @@ -201,14 +209,16 @@ protected void run(@Nullable Consumer> startedCallback) { T result = null; if (task != null) { result = task.get(); - final T finalResult = result; - callbackLock.lock(); - onCompletion.forEach(c -> c.accept(finalResult)); - callbackLock.unlock(); } future.complete(result); } catch (Exception e) { future.completeExceptionally(e); + } finally { + if (task != null) { + callbackLock.lock(); + onCompletion.forEach(c -> c.accept(this)); + callbackLock.unlock(); + } } } finally { if (locked) { @@ -242,18 +252,11 @@ public boolean isRunning() { } @Override - public ThrottledFuture then(Consumer action) { + public ThrottledFuture then(Consumer> action) { try { callbackLock.lock(); - if (future.isDone() && !future.isCancelled() && !future.isCompletedExceptionally()) { - try { - action.accept(future.get()); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new TermItException(e); - } catch (ExecutionException e) { - throw new TermItException(e); - } + if (future.isDone()) { + action.accept(this); } else { onCompletion.add(action); } diff --git a/src/main/java/cz/cvut/kbss/termit/websocket/VocabularySocketController.java b/src/main/java/cz/cvut/kbss/termit/websocket/VocabularySocketController.java index 00c2e8b83..f244358ed 100644 --- a/src/main/java/cz/cvut/kbss/termit/websocket/VocabularySocketController.java +++ b/src/main/java/cz/cvut/kbss/termit/websocket/VocabularySocketController.java @@ -11,7 +11,7 @@ import cz.cvut.kbss.termit.service.business.VocabularyService; import cz.cvut.kbss.termit.util.Configuration; import cz.cvut.kbss.termit.util.Constants; -import cz.cvut.kbss.termit.util.throttle.CacheableFuture; +import cz.cvut.kbss.termit.util.throttle.ThrottledFuture; import jakarta.annotation.Nonnull; import org.springframework.context.event.EventListener; import org.springframework.messaging.MessageHeaders; @@ -53,7 +53,7 @@ public void validateVocabulary(@DestinationVariable String localName, final URI identifier = resolveIdentifier(namespace.orElse(config.getNamespace().getVocabulary()), localName); final Vocabulary vocabulary = vocabularyService.getReference(identifier); - final CacheableFuture> future = vocabularyService.validateContents(vocabulary.getUri()); + final ThrottledFuture> future = vocabularyService.validateContents(vocabulary.getUri()); future.getNow().ifPresentOrElse(validationResults -> // if there is a result present (returned from cache), send it @@ -66,14 +66,15 @@ public void validateVocabulary(@DestinationVariable String localName, messageHeaders ), () -> // otherwise reply will be sent once the future is resolved - future.then(results -> + future.then(completedFuture -> + completedFuture.getNow().ifPresent(results -> sendToSession( WebSocketDestinations.VOCABULARIES_VALIDATION, results, getHeaders(identifier, Map.of("cached", false)), messageHeaders - )) + ))) ); } diff --git a/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java b/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java index 039a2e3d8..adf349a03 100644 --- a/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java +++ b/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java @@ -299,8 +299,8 @@ void transferUpdatesSecondFutureWithTask() { @Test void transferUpdatesSecondFutureWithCallbacks() { - final Consumer firstCallback = (result) -> {}; - final Consumer secondCallback = (result) -> {}; + final Consumer> firstCallback = (result) -> {}; + final Consumer> secondCallback = (result) -> {}; final ThrottledFuture firstFuture = ThrottledFuture.of(()->"").then(firstCallback); final ThrottledFuture secondFuture = ThrottledFuture.of(()->"").then(secondCallback); final ThrottledFuture mocked = mock(ThrottledFuture.class); @@ -323,14 +323,14 @@ void transferUpdatesSecondFutureWithCallbacks() { @Test void callbacksAreClearedAfterTransferring() { - final Consumer firstCallback = (result) -> {}; - final Consumer secondCallback = (result) -> {}; + final Consumer> firstCallback = (result) -> {}; + final Consumer> secondCallback = (result) -> {}; final ThrottledFuture future = ThrottledFuture.of(()->"").then(firstCallback).then(secondCallback); final ThrottledFuture mocked = mock(ThrottledFuture.class); future.transfer(mocked); - final ArgumentCaptor>> captor = ArgumentCaptor.forClass(List.class); + final ArgumentCaptor>>> captor = ArgumentCaptor.forClass(List.class); verify(mocked).update(notNull(), captor.capture()); // captor takes the original list from the future @@ -386,8 +386,8 @@ void updateSetsTask() { @Test void updateAddsCallbacksToTheCurrentOnes() { - final Consumer callback = result -> {}; - final Consumer originalCallback = result -> {}; + final Consumer> callback = result -> {}; + final Consumer> originalCallback = result -> {}; final ThrottledFuture future = ThrottledFuture.of(() -> "").then(originalCallback); future.update(()->"", List.of(callback)); From 3f938ecd48b26a01ec7cbeb1153fe3a341f049a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Ka=C5=88ka?= Date: Wed, 20 Nov 2024 14:01:23 +0100 Subject: [PATCH 13/14] [Bug #314] Update tests for ThrottledFuture#then accepting the completed future --- .../util/throttle/ThrottledFutureTest.java | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java b/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java index adf349a03..e3fc38c53 100644 --- a/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java +++ b/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java @@ -85,8 +85,7 @@ void getNowReturnsEmptyWhenCacheIsNull() { @Test void thenActionIsExecutedSynchronouslyWhenFutureIsAlreadyDoneAndNotCanceled() { - final Object result = new Object(); - final ThrottledFuture future = ThrottledFuture.of(() -> result); + final ThrottledFuture future = ThrottledFuture.of(() -> null); final AtomicBoolean completed = new AtomicBoolean(false); final AtomicReference futureResult = new AtomicReference<>(null); future.run(null); @@ -97,25 +96,24 @@ void thenActionIsExecutedSynchronouslyWhenFutureIsAlreadyDoneAndNotCanceled() { futureResult.set(fResult); }); assertTrue(completed.get()); - assertEquals(result, futureResult.get()); + assertEquals(future, futureResult.get()); } @Test - void thenActionIsNotExecutedWhenFutureIsAlreadyCancelled() { + void thenActionIsExecutedWhenFutureIsAlreadyCancelled() { final ThrottledFuture future = ThrottledFuture.of(Object::new); final AtomicBoolean completed = new AtomicBoolean(false); future.cancel(false); assertTrue(future.isCancelled()); future.then(result -> completed.set(true)); - assertFalse(completed.get()); + assertTrue(completed.get()); } @Test void thenActionIsExecutedOnceFutureIsRun() { - final Object result = new Object(); final AtomicBoolean completed = new AtomicBoolean(false); final AtomicReference fResult = new AtomicReference<>(null); - final ThrottledFuture future = ThrottledFuture.of(() -> result); + final ThrottledFuture future = ThrottledFuture.of(() -> null); future.then(futureResult -> { completed.set(true); fResult.set(futureResult); @@ -124,22 +122,22 @@ void thenActionIsExecutedOnceFutureIsRun() { assertFalse(completed.get()); // action was not executed yet future.run(null); assertTrue(completed.get()); - assertEquals(result, fResult.get()); + assertEquals(future, fResult.get()); } @Test - void thenActionIsNotExecutedOnceFutureIsCancelled() { + void thenActionIsExecutedOnceFutureIsCancelled() { final Object result = new Object(); final AtomicBoolean completed = new AtomicBoolean(false); final ThrottledFuture future = ThrottledFuture.of(() -> result); future.then(futureResult -> completed.set(true)); assertFalse(completed.get()); // action was not executed yet future.cancel(false); - assertFalse(completed.get()); + assertTrue(completed.get()); } @Test - void thenActionIsNotExecutedWhenFutureCompletedExceptionally() { + void thenActionIsExecutedWhenFutureCompletedExceptionally() { final AtomicBoolean completed = new AtomicBoolean(false); final ThrottledFuture future = ThrottledFuture.of(() -> { throw new RuntimeException(); @@ -147,7 +145,7 @@ void thenActionIsNotExecutedWhenFutureCompletedExceptionally() { future.run(null); assertFalse(completed.get()); future.then(futureResult -> completed.set(true)); - assertFalse(completed.get()); + assertTrue(completed.get()); } @Test From 8315b90bb986da00d01d3c68fb87acfacc31cb4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Ka=C5=88ka?= Date: Wed, 20 Nov 2024 16:47:45 +0100 Subject: [PATCH 14/14] [Bug #314] Add tests ensuring proper ThrottledFuture#then callbacks execution when future is cancelled. --- .../termit/util/throttle/ChainableFuture.java | 2 + .../termit/util/throttle/ThrottledFuture.java | 5 +- .../util/throttle/ThrottledFutureTest.java | 54 ++++++++++++++++++- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/util/throttle/ChainableFuture.java b/src/main/java/cz/cvut/kbss/termit/util/throttle/ChainableFuture.java index 12d2b915e..831f00d52 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/throttle/ChainableFuture.java +++ b/src/main/java/cz/cvut/kbss/termit/util/throttle/ChainableFuture.java @@ -10,6 +10,8 @@ public interface ChainableFuture> extends Fut * Action is executed no matter if the future is completed successfully, exceptionally or cancelled. *

* If the future is already completed, it is executed synchronously. + *

+ * Note that you must use the future passed as the parameter and not the original future object. * @param action action receiving this future after completion * @return this future */ diff --git a/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java b/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java index b32e6e095..045d06cdf 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java +++ b/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottledFuture.java @@ -89,11 +89,12 @@ public ThrottledFuture setCachedResult(@Nullable final T cachedResult) { @Override public boolean cancel(boolean mayInterruptIfRunning) { + final boolean wasCanceled = isCancelled(); if(!future.cancel(mayInterruptIfRunning)) { return false; } - if (task != null) { + if (!wasCanceled && task != null) { callbackLock.lock(); onCompletion.forEach(c -> c.accept(this)); callbackLock.unlock(); @@ -268,7 +269,7 @@ public ThrottledFuture then(Consumer> action) { /** * @return {@code true} if this future completed - * exceptionally + * exceptionally or was cancelled. */ public boolean isCompletedExceptionally() { return future.isCompletedExceptionally(); diff --git a/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java b/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java index e3fc38c53..b051471ab 100644 --- a/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java +++ b/src/test/java/cz/cvut/kbss/termit/util/throttle/ThrottledFutureTest.java @@ -137,7 +137,59 @@ void thenActionIsExecutedOnceFutureIsCancelled() { } @Test - void thenActionIsExecutedWhenFutureCompletedExceptionally() { + void thenActionIsExecutedOnlyOnceWhenFutureIsCancelled() { + final AtomicInteger executionCount = new AtomicInteger(0); + final ThrottledFuture future = ThrottledFuture.of(() -> null); + future.then(f -> executionCount.incrementAndGet()); + assertEquals(0, executionCount.get()); + future.cancel(false); + assertEquals(1, executionCount.get()); + future.cancel(false); + future.cancel(true); + assertEquals(1, executionCount.get()); + } + + @Test + void thenActionIsExecutedWhenFutureCompletesExceptionally() { + final AtomicBoolean completed = new AtomicBoolean(false); + final ThrottledFuture future = ThrottledFuture.of(() -> { + throw new RuntimeException(); + }); + future.then(futureResult -> completed.set(true)); + assertFalse(completed.get()); + future.run(null); + assertTrue(completed.get()); + } + + @Test + void isCompletedExceptionallyReturnsTrueWhenFutureCompletesExceptionally() { + final ThrottledFuture future = ThrottledFuture.of(() -> { + throw new RuntimeException(); + }); + future.run(null); + assertTrue(future.isCompletedExceptionally()); + } + + @Test + void isCompletedExceptionallyReturnsFalseWhenFutureCompletesNormally() { + final ThrottledFuture future = ThrottledFuture.of(() -> null); + future.run(null); + assertFalse(future.isCompletedExceptionally()); + assertFalse(future.isCancelled()); + assertTrue(future.isDone()); + } + + @Test + void isCompletedExceptionallyReturnsTrueWhenFutureIsCancelled() { + final ThrottledFuture future = ThrottledFuture.of(() -> null); + future.cancel(false); + assertTrue(future.isCompletedExceptionally()); + assertTrue(future.isCancelled()); + assertTrue(future.isDone()); + } + + @Test + void thenActionIsExecutedWhenFutureIsAlreadyCompletedExceptionally() { final AtomicBoolean completed = new AtomicBoolean(false); final ThrottledFuture future = ThrottledFuture.of(() -> { throw new RuntimeException();