Compare commits

...

6 Commits

Author SHA1 Message Date
Jens Kristian Villadsen 48b69e6e7b
Merge 42c248b4ef into 3f6d1eb29b 2024-09-26 02:08:11 +00:00
Thomas Papke 3f6d1eb29b
#5768 Upgrade to latest simple-java-mail (#6261) 2024-09-26 02:07:27 +00:00
Tadgh 377e44b6ca
attribution and pom change (#6309) 2024-09-25 20:38:22 +00:00
Jens Kristian Villadsen 42c248b4ef Added dedicated test case 2024-09-17 09:53:26 +02:00
Jens Kristian Villadsen 698c821dc0 Made tests go green 2024-09-16 16:11:38 +02:00
Jens Kristian Villadsen 72ba7cd1c2 Making sure that we are not accidentally overwriting an existing resource with the same ID from another IG by comparing identifier, url or whatever constitutes uniqueness. 2024-09-16 15:45:20 +02:00
12 changed files with 108 additions and 59 deletions

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 6290
title: "Previously, a specific migration task was using the `TRIM()` function, which does not exist in MSSQL 2012. This was causing migrations targeting MSSQL 2012 to fail.
This has been corrected and replaced with usage of a combination of LTRIM() and RTRIM(). Thanks to Primož Delopst at Better for the contribution!"

View File

@ -497,9 +497,10 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
IIdType id = theResource.getIdElement(); IIdType id = theResource.getIdElement();
RequestDetails requestDetails = createRequestDetails(); RequestDetails requestDetails = createRequestDetails();
String query = createSearchParameterMapFor(theResource).toNormalizedQueryString(myFhirContext);
try { try {
outcome = theDao.update(theResource, requestDetails); outcome = theDao.update(theResource, query, requestDetails);
} catch (ResourceVersionConflictException exception) { } catch (ResourceVersionConflictException exception) {
final Optional<IBaseResource> optResource = readResourceById(theDao, id); final Optional<IBaseResource> optResource = readResourceById(theDao, id);

View File

@ -18,6 +18,7 @@ import ca.uhn.fhir.jpa.searchparam.util.SearchParameterHelper;
import ca.uhn.fhir.mdm.log.Logs; import ca.uhn.fhir.mdm.log.Logs;
import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.server.SimpleBundleProvider; import ca.uhn.fhir.rest.server.SimpleBundleProvider;
import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException;
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
import ca.uhn.hapi.converters.canonical.VersionCanonicalizer; import ca.uhn.hapi.converters.canonical.VersionCanonicalizer;
import ca.uhn.test.util.LogbackTestExtension; import ca.uhn.test.util.LogbackTestExtension;
@ -131,6 +132,8 @@ public class PackageInstallerSvcImplTest {
private ArgumentCaptor<SearchParameter> mySearchParameterCaptor; private ArgumentCaptor<SearchParameter> mySearchParameterCaptor;
@Captor @Captor
private ArgumentCaptor<RequestDetails> myRequestDetailsCaptor; private ArgumentCaptor<RequestDetails> myRequestDetailsCaptor;
@Captor
private ArgumentCaptor<String> myMathcUrlCaptor;
@Test @Test
public void testPackageCompatibility() { public void testPackageCompatibility() {
@ -227,7 +230,43 @@ public class PackageInstallerSvcImplTest {
} }
} }
@Test
public void testDontTryToInstallDuplicateCodeSystem_CodeSystemAlreadyExistsWithSameIdButDifferentCanonicalUrl() throws IOException {
// Setup
// The CodeSystem that is already saved in the repository
CodeSystem existingCs = new CodeSystem();
existingCs.setId("CodeSystem/existingcs");
existingCs.setUrl("http://my-old-code-system");
existingCs.setContent(CodeSystem.CodeSystemContentMode.COMPLETE);
// A new code system in a package we're installing that has a
// different URL as the previously saved one, but the same ID.
CodeSystem newCs = new CodeSystem();
newCs.setId("CodeSystem/existingcs");
newCs.setUrl("http://my-new-code-system");
newCs.setContent(CodeSystem.CodeSystemContentMode.COMPLETE);
PackageInstallationSpec oldSpec = setupResourceInPackage(null, existingCs, myCodeSystemDao);
// Test
mySvc.install(oldSpec);
PackageInstallationSpec newSpec = setupResourceInPackage(null, newCs, myCodeSystemDao);
when(myCodeSystemDao.update(any(), any(), any())).thenThrow(new ResourceVersionConflictException("Resource already exists with that ID but with a different URL"));
mySvc.install(newSpec);
// Verify
verify(myCodeSystemDao, times(2)).search(mySearchParameterMapCaptor.capture(), any());
SearchParameterMap newMap = mySearchParameterMapCaptor.getValue();
assertEquals("?url=http%3A%2F%2Fmy-new-code-system", newMap.toNormalizedQueryString(myCtx));
verify(myCodeSystemDao, times(2)).update(myCodeSystemCaptor.capture(), any(String.class) , any(RequestDetails.class));
CodeSystem newCodeSystem = myCodeSystemCaptor.getValue();
assertEquals("existingcs", newCodeSystem.getIdPart());
}
@Test @Test
@ -257,7 +296,7 @@ public class PackageInstallerSvcImplTest {
SearchParameterMap map = mySearchParameterMapCaptor.getValue(); SearchParameterMap map = mySearchParameterMapCaptor.getValue();
assertEquals("?url=http%3A%2F%2Fmy-code-system", map.toNormalizedQueryString(myCtx)); assertEquals("?url=http%3A%2F%2Fmy-code-system", map.toNormalizedQueryString(myCtx));
verify(myCodeSystemDao, times(1)).update(myCodeSystemCaptor.capture(), any(RequestDetails.class)); verify(myCodeSystemDao, times(1)).update(myCodeSystemCaptor.capture(), any(String.class) , any(RequestDetails.class));
CodeSystem codeSystem = myCodeSystemCaptor.getValue(); CodeSystem codeSystem = myCodeSystemCaptor.getValue();
assertEquals("existingcs", codeSystem.getIdPart()); assertEquals("existingcs", codeSystem.getIdPart());
} }
@ -295,9 +334,9 @@ public class PackageInstallerSvcImplTest {
if (theInstallType == InstallType.CREATE) { if (theInstallType == InstallType.CREATE) {
verify(mySearchParameterDao, times(1)).create(mySearchParameterCaptor.capture(), myRequestDetailsCaptor.capture()); verify(mySearchParameterDao, times(1)).create(mySearchParameterCaptor.capture(), myRequestDetailsCaptor.capture());
} else if (theInstallType == InstallType.UPDATE_WITH_EXISTING){ } else if (theInstallType == InstallType.UPDATE_WITH_EXISTING){
verify(mySearchParameterDao, times(2)).update(mySearchParameterCaptor.capture(), myRequestDetailsCaptor.capture()); verify(mySearchParameterDao, times(2)).update(mySearchParameterCaptor.capture(), myMathcUrlCaptor.capture(), myRequestDetailsCaptor.capture());
} else { } else {
verify(mySearchParameterDao, times(1)).update(mySearchParameterCaptor.capture(), myRequestDetailsCaptor.capture()); verify(mySearchParameterDao, times(1)).update(mySearchParameterCaptor.capture(), myMathcUrlCaptor.capture(), myRequestDetailsCaptor.capture());
} }
Iterator<SearchParameter> iteratorSP = mySearchParameterCaptor.getAllValues().iterator(); Iterator<SearchParameter> iteratorSP = mySearchParameterCaptor.getAllValues().iterator();
@ -366,6 +405,7 @@ public class PackageInstallerSvcImplTest {
if (theId != null) { if (theId != null) {
searchParameter.setId(new IdType("SearchParameter", theId)); searchParameter.setId(new IdType("SearchParameter", theId));
} }
searchParameter.setUrl("http://example.com/SearchParameter");
searchParameter.setCode("someCode"); searchParameter.setCode("someCode");
theBase.forEach(base -> searchParameter.getBase().add(new CodeType(base))); theBase.forEach(base -> searchParameter.getBase().add(new CodeType(base)));
searchParameter.setExpression("someExpression"); searchParameter.setExpression("someExpression");

View File

@ -70,6 +70,11 @@
<artifactId>jakarta.servlet-api</artifactId> <artifactId>jakarta.servlet-api</artifactId>
<scope>provided</scope> <scope>provided</scope>
</dependency> </dependency>
<dependency>
<groupId>jakarta.mail</groupId>
<artifactId>jakarta.mail-api</artifactId>
<optional>true</optional>
</dependency>
<!-- test dependencies --> <!-- test dependencies -->
<dependency> <dependency>

View File

@ -28,8 +28,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import javax.mail.internet.InternetAddress; import jakarta.mail.internet.InternetAddress;
import javax.mail.internet.MimeMessage; import jakarta.mail.internet.MimeMessage;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;

View File

@ -17,8 +17,8 @@ import org.junit.jupiter.api.extension.RegisterExtension;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import javax.mail.internet.InternetAddress; import jakarta.mail.internet.InternetAddress;
import javax.mail.internet.MimeMessage; import jakarta.mail.internet.MimeMessage;
import java.util.Arrays; import java.util.Arrays;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;

View File

@ -26,8 +26,8 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.extension.RegisterExtension;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import javax.mail.internet.InternetAddress; import jakarta.mail.internet.InternetAddress;
import javax.mail.internet.MimeMessage; import jakarta.mail.internet.MimeMessage;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;

View File

@ -79,25 +79,11 @@
<dependency> <dependency>
<groupId>org.simplejavamail</groupId> <groupId>org.simplejavamail</groupId>
<artifactId>simple-java-mail</artifactId> <artifactId>simple-java-mail</artifactId>
<!-- Excluded in favor of jakarta.activation:jakarta.activation-api -->
<exclusions>
<exclusion>
<groupId>com.sun.activation</groupId>
<artifactId>jakarta.activation</artifactId>
</exclusion>
</exclusions>
</dependency> </dependency>
<dependency> <dependency>
<groupId>com.icegreen</groupId> <groupId>com.icegreen</groupId>
<artifactId>greenmail</artifactId> <artifactId>greenmail</artifactId>
<scope>compile</scope> <scope>compile</scope>
<!-- Excluded in favor of jakarta.activation:jakarta.activation-api -->
<exclusions>
<exclusion>
<groupId>com.sun.activation</groupId>
<artifactId>jakarta.activation</artifactId>
</exclusion>
</exclusions>
</dependency> </dependency>
</dependencies> </dependencies>

View File

@ -21,9 +21,9 @@ package ca.uhn.fhir.rest.server.mail;
import jakarta.annotation.Nonnull; import jakarta.annotation.Nonnull;
import org.simplejavamail.api.email.Email; import org.simplejavamail.api.email.Email;
import org.simplejavamail.api.mailer.AsyncResponse;
import java.util.List; import java.util.List;
import java.util.function.Consumer;
public interface IMailSvc { public interface IMailSvc {
void sendMail(@Nonnull List<Email> theEmails); void sendMail(@Nonnull List<Email> theEmails);
@ -31,7 +31,5 @@ public interface IMailSvc {
void sendMail(@Nonnull Email theEmail); void sendMail(@Nonnull Email theEmail);
void sendMail( void sendMail(
@Nonnull Email theEmail, @Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull Consumer<Throwable> theErrorHandler);
@Nonnull Runnable theOnSuccess,
@Nonnull AsyncResponse.ExceptionConsumer theErrorHandler);
} }

View File

@ -20,12 +20,9 @@
package ca.uhn.fhir.rest.server.mail; package ca.uhn.fhir.rest.server.mail;
import jakarta.annotation.Nonnull; import jakarta.annotation.Nonnull;
import org.apache.commons.lang3.Validate;
import org.simplejavamail.MailException; import org.simplejavamail.MailException;
import org.simplejavamail.api.email.Email; import org.simplejavamail.api.email.Email;
import org.simplejavamail.api.email.Recipient; import org.simplejavamail.api.email.Recipient;
import org.simplejavamail.api.mailer.AsyncResponse;
import org.simplejavamail.api.mailer.AsyncResponse.ExceptionConsumer;
import org.simplejavamail.api.mailer.Mailer; import org.simplejavamail.api.mailer.Mailer;
import org.simplejavamail.api.mailer.config.TransportStrategy; import org.simplejavamail.api.mailer.config.TransportStrategy;
import org.simplejavamail.mailer.MailerBuilder; import org.simplejavamail.mailer.MailerBuilder;
@ -33,6 +30,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import java.util.List; import java.util.List;
import java.util.Objects;
import java.util.function.Consumer;
import java.util.stream.Collectors; import java.util.stream.Collectors;
public class MailSvc implements IMailSvc { public class MailSvc implements IMailSvc {
@ -42,14 +41,14 @@ public class MailSvc implements IMailSvc {
private final Mailer myMailer; private final Mailer myMailer;
public MailSvc(@Nonnull MailConfig theMailConfig) { public MailSvc(@Nonnull MailConfig theMailConfig) {
Validate.notNull(theMailConfig); Objects.requireNonNull(theMailConfig);
myMailConfig = theMailConfig; myMailConfig = theMailConfig;
myMailer = makeMailer(myMailConfig); myMailer = makeMailer(myMailConfig);
} }
@Override @Override
public void sendMail(@Nonnull List<Email> theEmails) { public void sendMail(@Nonnull List<Email> theEmails) {
Validate.notNull(theEmails); Objects.requireNonNull(theEmails);
theEmails.forEach(theEmail -> send(theEmail, new OnSuccess(theEmail), new ErrorHandler(theEmail))); theEmails.forEach(theEmail -> send(theEmail, new OnSuccess(theEmail), new ErrorHandler(theEmail)));
} }
@ -60,21 +59,23 @@ public class MailSvc implements IMailSvc {
@Override @Override
public void sendMail( public void sendMail(
@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull ExceptionConsumer theErrorHandler) { @Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull Consumer<Throwable> theErrorHandler) {
send(theEmail, theOnSuccess, theErrorHandler); send(theEmail, theOnSuccess, theErrorHandler);
} }
private void send( private void send(
@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull ExceptionConsumer theErrorHandler) { @Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull Consumer<Throwable> theErrorHandler) {
Validate.notNull(theEmail); Objects.requireNonNull(theEmail);
Validate.notNull(theOnSuccess); Objects.requireNonNull(theOnSuccess);
Validate.notNull(theErrorHandler); Objects.requireNonNull(theErrorHandler);
try { try {
final AsyncResponse asyncResponse = myMailer.sendMail(theEmail, true); myMailer.sendMail(theEmail, true).whenComplete((result, ex) -> {
if (asyncResponse != null) { if (ex != null) {
asyncResponse.onSuccess(theOnSuccess); theErrorHandler.accept(ex);
asyncResponse.onException(theErrorHandler); } else {
} theOnSuccess.run();
}
});
} catch (MailException e) { } catch (MailException e) {
theErrorHandler.accept(e); theErrorHandler.accept(e);
} }
@ -117,7 +118,7 @@ public class MailSvc implements IMailSvc {
} }
} }
private class ErrorHandler implements ExceptionConsumer { private class ErrorHandler implements Consumer<Throwable> {
private final Email myEmail; private final Email myEmail;
private ErrorHandler(@Nonnull Email theEmail) { private ErrorHandler(@Nonnull Email theEmail) {
@ -125,7 +126,7 @@ public class MailSvc implements IMailSvc {
} }
@Override @Override
public void accept(Exception t) { public void accept(Throwable t) {
ourLog.error("Email not sent" + makeMessage(myEmail), t); ourLog.error("Email not sent" + makeMessage(myEmail), t);
} }
} }

View File

@ -4,6 +4,7 @@ import com.icegreen.greenmail.junit5.GreenMailExtension;
import com.icegreen.greenmail.util.GreenMailUtil; import com.icegreen.greenmail.util.GreenMailUtil;
import com.icegreen.greenmail.util.ServerSetupTest; import com.icegreen.greenmail.util.ServerSetupTest;
import jakarta.annotation.Nonnull; import jakarta.annotation.Nonnull;
import jakarta.mail.internet.MimeMessage;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.extension.RegisterExtension;
@ -11,7 +12,6 @@ import org.simplejavamail.MailException;
import org.simplejavamail.api.email.Email; import org.simplejavamail.api.email.Email;
import org.simplejavamail.email.EmailBuilder; import org.simplejavamail.email.EmailBuilder;
import javax.mail.internet.MimeMessage;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
@ -86,13 +86,14 @@ public class MailSvcIT {
@Test @Test
public void testSendMailWithInvalidToAddressExpectErrorHandler() { public void testSendMailWithInvalidToAddressExpectErrorHandler() {
// setup // setup
final Email email = withEmail("xyz"); String invalidEmailAdress = "xyz";
final Email email = withEmail(invalidEmailAdress);
// execute // execute
fixture.sendMail(email, fixture.sendMail(email,
() -> fail("Should not execute on Success"), () -> fail("Should not execute on Success"),
(e) -> { (e) -> {
assertTrue(e instanceof MailException); assertTrue(e instanceof MailException);
assertEquals("Invalid TO address: " + email, e.getMessage()); assertEquals("Invalid TO address: " + invalidEmailAdress, e.getMessage());
}); });
// validate // validate
assertTrue(ourGreenMail.waitForIncomingEmail(1000, 0)); assertTrue(ourGreenMail.waitForIncomingEmail(1000, 0));

34
pom.xml
View File

@ -869,6 +869,7 @@
<developer> <developer>
<id>delopst</id> <id>delopst</id>
<name>Primož Delopst</name> <name>Primož Delopst</name>
<organization>Better</organization>
</developer> </developer>
<developer> <developer>
<id>Zach Smith</id> <id>Zach Smith</id>
@ -1160,27 +1161,38 @@
<dependency> <dependency>
<groupId>org.simplejavamail</groupId> <groupId>org.simplejavamail</groupId>
<artifactId>simple-java-mail</artifactId> <artifactId>simple-java-mail</artifactId>
<version>6.6.1</version> <version>8.11.2</version>
<exclusions> <exclusions>
<exclusion> <exclusion>
<groupId>com.sun.activation</groupId> <groupId>com.github.bbottema</groupId>
<artifactId>jakarta.activation-api</artifactId> <artifactId>jetbrains-runtime-annotations</artifactId>
</exclusion> </exclusion>
<exclusion> <exclusion>
<groupId>com.sun.activation</groupId> <groupId>jakarta.mail</groupId>
<artifactId>jakarta.activation</artifactId> <artifactId>jakarta.mail-api</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>jakarta.mail</groupId>
<artifactId>jakarta.mail-api</artifactId>
<version>2.1.3</version>
</dependency>
<dependency>
<groupId>com.icegreen</groupId>
<artifactId>greenmail</artifactId>
<version>2.1.0-rc-1</version>
<exclusions>
<exclusion>
<groupId>jakarta.mail</groupId>
<artifactId>jakarta.mail-api</artifactId>
</exclusion> </exclusion>
</exclusions> </exclusions>
</dependency> </dependency>
<dependency>
<groupId>com.icegreen</groupId>
<artifactId>greenmail</artifactId>
<version>1.6.4</version>
</dependency>
<dependency> <dependency>
<groupId>com.icegreen</groupId> <groupId>com.icegreen</groupId>
<artifactId>greenmail-junit5</artifactId> <artifactId>greenmail-junit5</artifactId>
<version>1.6.4</version> <version>2.1.0-rc-1</version>
<scope>compile</scope> <scope>compile</scope>
</dependency> </dependency>
<!-- mail end --> <!-- mail end -->