diff --git a/src/main/java/io/kamax/mxisd/backend/rest/RestProfileProvider.java b/src/main/java/io/kamax/mxisd/backend/rest/RestProfileProvider.java index 64be7da..df61137 100644 --- a/src/main/java/io/kamax/mxisd/backend/rest/RestProfileProvider.java +++ b/src/main/java/io/kamax/mxisd/backend/rest/RestProfileProvider.java @@ -32,7 +32,7 @@ import io.kamax.mxisd.profile.JsonProfileRequest; import io.kamax.mxisd.profile.JsonProfileResult; import io.kamax.mxisd.profile.ProfileProvider; import org.apache.commons.io.IOUtils; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.utils.URIBuilder; @@ -49,7 +49,7 @@ import java.util.function.Function; public class RestProfileProvider extends RestProvider implements ProfileProvider { - private transient final Logger log = LoggerFactory.getLogger(RestProfileProvider.class); + private static final Logger log = LoggerFactory.getLogger(RestProfileProvider.class); public RestProfileProvider(RestBackendConfig cfg) { super(cfg); @@ -60,64 +60,71 @@ public class RestProfileProvider extends RestProvider implements ProfileProvider Function> endpoint, Function> value ) { - return cfg.getEndpoints().getProfile() - // We get the endpoint - .flatMap(endpoint) - // We only continue if there is a value - .filter(StringUtils::isNotBlank) - // We use the endpoint - .flatMap(url -> { - try { - URIBuilder builder = new URIBuilder(url); - HttpPost req = new HttpPost(builder.build()); - req.setEntity(new StringEntity(GsonUtil.get().toJson(new JsonProfileRequest(userId)), ContentType.APPLICATION_JSON)); - try (CloseableHttpResponse res = client.execute(req)) { - int sc = res.getStatusLine().getStatusCode(); - if (sc == 404) { - log.info("Got 404 - No result found"); - return Optional.empty(); - } + Optional url = endpoint.apply(cfg.getEndpoints().getProfile()); + if (!url.isPresent()) { + return Optional.empty(); + } - if (sc != 200) { - throw new InternalServerError("Unexpected backed status code: " + sc); - } + try { + URIBuilder builder = new URIBuilder(url.get()); + HttpPost req = new HttpPost(builder.build()); + req.setEntity(new StringEntity(GsonUtil.get().toJson(new JsonProfileRequest(userId)), ContentType.APPLICATION_JSON)); + try (CloseableHttpResponse res = client.execute(req)) { + int sc = res.getStatusLine().getStatusCode(); + if (sc == 404) { + log.info("Got 404 - No result found"); + return Optional.empty(); + } - String body = IOUtils.toString(res.getEntity().getContent(), StandardCharsets.UTF_8); - if (StringUtils.isBlank(body)) { - log.warn("Backend response body is empty/blank, expected JSON object with profile key"); - return Optional.empty(); - } + if (sc != 200) { + throw new InternalServerError("Unexpected backed status code: " + sc); + } - Optional pJson = GsonUtil.findObj(GsonUtil.parseObj(body), "profile"); - if (!pJson.isPresent()) { - log.warn("Backend response body is invalid, expected JSON object with profile key"); - return Optional.empty(); - } + String body = IOUtils.toString(res.getEntity().getContent(), StandardCharsets.UTF_8); + if (StringUtils.isBlank(body)) { + log.warn("Backend response body is empty/blank, expected JSON object with profile key"); + return Optional.empty(); + } - JsonProfileResult profile = gson.fromJson(pJson.get(), JsonProfileResult.class); - return value.apply(profile); - } - } catch (JsonSyntaxException | InvalidJsonException e) { - log.error("Unable to parse backend response as JSON", e); - throw new InternalServerError(e); - } catch (URISyntaxException e) { - log.error("Unable to build a valid request URL", e); - throw new InternalServerError(e); - } catch (IOException e) { - log.error("I/O Error during backend request", e); - throw new InternalServerError(); - } - }); + Optional pJson = GsonUtil.findObj(GsonUtil.parseObj(body), "profile"); + if (!pJson.isPresent()) { + log.warn("Backend response body is invalid, expected JSON object with profile key"); + return Optional.empty(); + } + + JsonProfileResult profile = gson.fromJson(pJson.get(), JsonProfileResult.class); + return value.apply(profile); + } + } catch (JsonSyntaxException | InvalidJsonException e) { + log.error("Unable to parse backend response as JSON", e); + throw new InternalServerError(e); + } catch (URISyntaxException e) { + log.error("Unable to build a valid request URL", e); + throw new InternalServerError(e); + } catch (IOException e) { + log.error("I/O Error during backend request", e); + throw new InternalServerError(); + } } @Override public Optional getDisplayName(_MatrixID userId) { - return doRequest(userId, p -> Optional.ofNullable(p.getDisplayName()), profile -> Optional.ofNullable(profile.getDisplayName())); + return doRequest(userId, p -> { + if (StringUtils.isBlank(p.getDisplayName())) { + return Optional.empty(); + } + return Optional.ofNullable(p.getDisplayName()); + }, profile -> Optional.ofNullable(profile.getDisplayName())); } @Override public List<_ThreePid> getThreepids(_MatrixID userId) { - return doRequest(userId, p -> Optional.ofNullable(p.getThreepids()), profile -> { + return doRequest(userId, p -> { + if (StringUtils.isBlank(p.getThreepids())) { + return Optional.empty(); + } + return Optional.ofNullable(p.getThreepids()); + }, profile -> { List<_ThreePid> t = new ArrayList<>(); if (Objects.nonNull(profile.getThreepids())) { t.addAll(profile.getThreepids()); @@ -128,7 +135,12 @@ public class RestProfileProvider extends RestProvider implements ProfileProvider @Override public List getRoles(_MatrixID userId) { - return doRequest(userId, p -> Optional.ofNullable(p.getRoles()), profile -> { + return doRequest(userId, p -> { + if (StringUtils.isBlank(p.getRoles())) { + return Optional.empty(); + } + return Optional.ofNullable(p.getRoles()); + }, profile -> { List t = new ArrayList<>(); if (Objects.nonNull(profile.getRoles())) { t.addAll(profile.getRoles()); diff --git a/src/main/java/io/kamax/mxisd/config/YamlConfigLoader.java b/src/main/java/io/kamax/mxisd/config/YamlConfigLoader.java index 55f07bd..5877f1a 100644 --- a/src/main/java/io/kamax/mxisd/config/YamlConfigLoader.java +++ b/src/main/java/io/kamax/mxisd/config/YamlConfigLoader.java @@ -43,9 +43,14 @@ public class YamlConfigLoader { rep.getPropertyUtils().setSkipMissingProperties(true); Yaml yaml = new Yaml(new Constructor(MxisdConfig.class), rep); try (FileInputStream is = new FileInputStream(path)) { - Object o = yaml.load(is); + MxisdConfig raw = yaml.load(is); log.debug("Read config in memory from {}", path); - MxisdConfig cfg = GsonUtil.get().fromJson(GsonUtil.get().toJson(o), MxisdConfig.class); + + // SnakeYaml set objects to null when there is no value set in the config, even a full sub-tree. + // This is problematic for default config values and objects, to avoid NPEs. + // Therefore, we'll use Gson to re-parse the data in a way that avoids us checking the whole config for nulls. + MxisdConfig cfg = GsonUtil.get().fromJson(GsonUtil.get().toJson(raw), MxisdConfig.class); + log.info("Loaded config from {}", path); return cfg; } diff --git a/src/main/java/io/kamax/mxisd/config/rest/RestBackendConfig.java b/src/main/java/io/kamax/mxisd/config/rest/RestBackendConfig.java index 5abe098..1f1a7b8 100644 --- a/src/main/java/io/kamax/mxisd/config/rest/RestBackendConfig.java +++ b/src/main/java/io/kamax/mxisd/config/rest/RestBackendConfig.java @@ -28,7 +28,6 @@ import org.slf4j.LoggerFactory; import java.net.MalformedURLException; import java.net.URL; import java.util.Objects; -import java.util.Optional; public class RestBackendConfig { @@ -118,8 +117,8 @@ public class RestBackendConfig { this.identity = identity; } - public Optional getProfile() { - return Optional.ofNullable(profile); + public ProfileEndpoints getProfile() { + return profile; } public void setProfile(ProfileEndpoints profile) { @@ -128,7 +127,7 @@ public class RestBackendConfig { } - private transient final Logger log = LoggerFactory.getLogger(RestBackendConfig.class); + private static final Logger log = LoggerFactory.getLogger(RestBackendConfig.class); private boolean enabled; private String host; @@ -197,6 +196,11 @@ public class RestBackendConfig { log.info("Directory endpoint: {}", endpoints.getDirectory()); log.info("Identity Single endpoint: {}", endpoints.identity.getSingle()); log.info("Identity Bulk endpoint: {}", endpoints.identity.getBulk()); + + log.info("Profile endpoints:"); + log.info(" - Display name: {}", getEndpoints().getProfile().getDisplayName()); + log.info(" - 3PIDs: {}", getEndpoints().getProfile().getThreepids()); + log.info(" - Roles: {}", getEndpoints().getProfile().getRoles()); } } diff --git a/src/test/java/io/kamax/mxisd/test/backend/rest/RestProfileProviderTest.java b/src/test/java/io/kamax/mxisd/test/backend/rest/RestProfileProviderTest.java index 1cbd74e..1118937 100644 --- a/src/test/java/io/kamax/mxisd/test/backend/rest/RestProfileProviderTest.java +++ b/src/test/java/io/kamax/mxisd/test/backend/rest/RestProfileProviderTest.java @@ -23,6 +23,7 @@ package io.kamax.mxisd.test.backend.rest; import com.github.tomakehurst.wiremock.junit.WireMockRule; import io.kamax.matrix.MatrixID; import io.kamax.matrix._MatrixID; +import io.kamax.matrix._ThreePid; import io.kamax.matrix.json.GsonUtil; import io.kamax.mxisd.backend.rest.RestProfileProvider; import io.kamax.mxisd.config.rest.RestBackendConfig; @@ -34,6 +35,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import java.util.List; import java.util.Optional; import static com.github.tomakehurst.wiremock.client.WireMock.*; @@ -42,27 +44,40 @@ import static org.junit.Assert.*; public class RestProfileProviderTest { + private static final int MockHttpPort = 65000; + private static final String MockHttpHost = "localhost"; + @Rule - public WireMockRule wireMockRule = new WireMockRule(65000); + public WireMockRule wireMockRule = new WireMockRule(MockHttpPort); private final String displayNameEndpoint = "/displayName"; - private final _MatrixID userId = MatrixID.from("john", "matrix.localhost").valid(); + private final _MatrixID userId = MatrixID.from("john", "matrix." + MockHttpHost).valid(); private RestProfileProvider p; - @Before - public void before() { - ProfileEndpoints endpoints = new ProfileEndpoints(); - endpoints.setDisplayName(displayNameEndpoint); - + private RestBackendConfig getCfg(RestBackendConfig.Endpoints endpoints) { RestBackendConfig cfg = new RestBackendConfig(); cfg.setEnabled(true); - cfg.setHost("http://localhost:65000"); - cfg.getEndpoints().setProfile(endpoints); + cfg.setHost("http://" + MockHttpHost + ":" + MockHttpPort); + cfg.setEndpoints(endpoints); cfg.build(); - p = new RestProfileProvider(cfg); + return cfg; + } + + private RestProfileProvider get(RestBackendConfig cfg) { + return new RestProfileProvider(cfg); + } + + @Before + public void before() { + ProfileEndpoints pEndpoints = new ProfileEndpoints(); + pEndpoints.setDisplayName(displayNameEndpoint); + RestBackendConfig.Endpoints endpoints = new RestBackendConfig.Endpoints(); + endpoints.setProfile(pEndpoints); + + p = get(getCfg(endpoints)); } @Test @@ -144,4 +159,26 @@ public class RestProfileProviderTest { } } + @Test + public void forEmptyEndpoints() { + ProfileEndpoints pEndpoints = new ProfileEndpoints(); + pEndpoints.setDisplayName(""); + pEndpoints.setThreepids(""); + pEndpoints.setRoles(""); + + RestBackendConfig.Endpoints endpoints = new RestBackendConfig.Endpoints(); + endpoints.setProfile(pEndpoints); + + RestProfileProvider p = get(getCfg(endpoints)); + + Optional dn = p.getDisplayName(userId); + assertFalse(dn.isPresent()); + + List roles = p.getRoles(userId); + assertTrue(roles.isEmpty()); + + List<_ThreePid> tpids = p.getThreepids(userId); + assertTrue(tpids.isEmpty()); + } + }