From 0a2f8f5b63eec69645a951ca98cd91f0f09a7406 Mon Sep 17 00:00:00 2001 From: Daniel Fesenmeyer Date: Tue, 27 Apr 2021 16:19:37 +0200 Subject: [PATCH] KEYCLOAK-17887 fix endpoint for creating or updating realm localization texts for a given locale (UnsupportedOperation was thrown because RealmAdapter tried to change unmodifiable map): - fix RealmAdapter to create a new map instead of trying to change unmodifiable map - only provide POST endpoints for creating or updating the texts (to have the endpoints consistent with other Admin API endpoints) - add tests --- .../resource/RealmLocalizationResource.java | 6 ++++ .../models/cache/infinispan/RealmAdapter.java | 4 +-- .../org/keycloak/models/jpa/RealmAdapter.java | 6 ++-- .../models/map/realm/MapRealmAdapter.java | 2 +- .../models/map/realm/MapRealmProvider.java | 8 ++--- .../java/org/keycloak/models/RealmModel.java | 5 +-- .../admin/RealmLocalizationResource.java | 13 ++++---- .../admin/RealmLocalizationResourceTest.java | 31 ++++++++++++++++++- 8 files changed, 57 insertions(+), 18 deletions(-) diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/RealmLocalizationResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/RealmLocalizationResource.java index 51b84915c583..6e94da5cce39 100755 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/RealmLocalizationResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/RealmLocalizationResource.java @@ -23,6 +23,7 @@ import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.GET; +import javax.ws.rs.POST; import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.PathParam; @@ -59,4 +60,9 @@ public interface RealmLocalizationResource { @PUT @Consumes(MediaType.TEXT_PLAIN) void saveRealmLocalizationText(@PathParam("locale") String locale, @PathParam("key") String key, String text); + + @Path("{locale}") + @POST + @Consumes("application/json") + void createOrUpdateRealmLocalizationTexts(@PathParam("locale") String locale, Map localizationTexts); } diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java index af08724645d0..69508c3e8da9 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java @@ -1698,9 +1698,9 @@ public Map getAttributes() { } @Override - public void patchRealmLocalizationTexts(String locale, Map localizationTexts) { + public void createOrUpdateRealmLocalizationTexts(String locale, Map localizationTexts) { getDelegateForUpdate(); - updated.patchRealmLocalizationTexts(locale, localizationTexts); + updated.createOrUpdateRealmLocalizationTexts(locale, localizationTexts); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index 2c87393d100a..5beb63ec4d74 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -2173,11 +2173,13 @@ private ComponentEntity getComponentEntity(String id) { } @Override - public void patchRealmLocalizationTexts(String locale, Map localizationTexts) { + public void createOrUpdateRealmLocalizationTexts(String locale, Map localizationTexts) { Map currentLocalizationTexts = realm.getRealmLocalizationTexts(); if(currentLocalizationTexts.containsKey(locale)) { RealmLocalizationTextsEntity localizationTextsEntity = currentLocalizationTexts.get(locale); - localizationTextsEntity.getTexts().putAll(localizationTexts); + Map updatedTexts = new HashMap<>(localizationTextsEntity.getTexts()); + updatedTexts.putAll(localizationTexts); + localizationTextsEntity.setTexts(updatedTexts); em.persist(localizationTextsEntity); } diff --git a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmAdapter.java b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmAdapter.java index 4222bfeaa985..2b0fd3e24b5b 100644 --- a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmAdapter.java @@ -1294,7 +1294,7 @@ public Stream getDefaultClientScopesStream(boolean defaultScop } @Override - public void patchRealmLocalizationTexts(String locale, Map localizationTexts) { + public void createOrUpdateRealmLocalizationTexts(String locale, Map localizationTexts) { Map> realmLocalizationTexts = entity.getLocalizationTexts(); if (realmLocalizationTexts.containsKey(locale)) { diff --git a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProvider.java b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProvider.java index f42dc791db89..4def40064e90 100644 --- a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProvider.java @@ -175,7 +175,7 @@ public void saveLocalizationText(RealmModel realm, String locale, String key, St if (! updateLocalizationText(realm, locale, key, text)) { Map texts = new HashMap<>(); texts.put(key, text); - realm.patchRealmLocalizationTexts(locale, texts); + realm.createOrUpdateRealmLocalizationTexts(locale, texts); } } @@ -183,7 +183,7 @@ public void saveLocalizationText(RealmModel realm, String locale, String key, St @Override public void saveLocalizationTexts(RealmModel realm, String locale, Map localizationTexts) { if (locale == null || localizationTexts == null) return; - realm.patchRealmLocalizationTexts(locale, localizationTexts); + realm.createOrUpdateRealmLocalizationTexts(locale, localizationTexts); } //TODO move the following method to adapter @@ -192,7 +192,7 @@ public boolean updateLocalizationText(RealmModel realm, String locale, String ke if (locale == null || key == null || text == null || (! realm.getRealmLocalizationTextsByLocale(locale).containsKey(key))) return false; Map texts = new HashMap<>(realm.getRealmLocalizationTextsByLocale(locale)); texts.replace(key, text); - realm.patchRealmLocalizationTexts(locale, texts); + realm.createOrUpdateRealmLocalizationTexts(locale, texts); return true; } @@ -210,7 +210,7 @@ public boolean deleteLocalizationText(RealmModel realm, String locale, String ke Map texts = new HashMap<>(realm.getRealmLocalizationTextsByLocale(locale)); texts.remove(key); realm.removeRealmLocalizationTexts(locale); - realm.patchRealmLocalizationTexts(locale, texts); + realm.createOrUpdateRealmLocalizationTexts(locale, texts); return true; } diff --git a/server-spi/src/main/java/org/keycloak/models/RealmModel.java b/server-spi/src/main/java/org/keycloak/models/RealmModel.java index 4145a78e7171..087b14024c7e 100755 --- a/server-spi/src/main/java/org/keycloak/models/RealmModel.java +++ b/server-spi/src/main/java/org/keycloak/models/RealmModel.java @@ -992,10 +992,11 @@ default List getClientScopes() { void removeDefaultClientScope(ClientScopeModel clientScope); /** - * Patches the realm-specific localization texts. This method will not delete any text. + * Creates or updates the realm-specific localization texts for the given locale. + * This method will not delete any text. * It updates texts, which are already stored or create new ones if the key does not exist yet. */ - void patchRealmLocalizationTexts(String locale, Map localizationTexts); + void createOrUpdateRealmLocalizationTexts(String locale, Map localizationTexts); boolean removeRealmLocalizationTexts(String locale); Map> getRealmLocalizationTexts(); Map getRealmLocalizationTextsByLocale(String locale); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RealmLocalizationResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RealmLocalizationResource.java index 0ab2f99a6267..ad5a46d37bc0 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/RealmLocalizationResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RealmLocalizationResource.java @@ -84,8 +84,8 @@ public void saveRealmLocalizationText(@PathParam("locale") String locale, @PathP @POST @Path("{locale}") @Consumes(MediaType.MULTIPART_FORM_DATA) - public void patchRealmLocalizationTextsFromFile(@PathParam("locale") String locale, MultipartFormDataInput input) - throws IOException { + public void createOrUpdateRealmLocalizationTextsFromFile(@PathParam("locale") String locale, + MultipartFormDataInput input) { this.auth.realm().requireManageRealm(); Map> formDataMap = input.getFormDataMap(); @@ -97,18 +97,19 @@ public void patchRealmLocalizationTextsFromFile(@PathParam("locale") String loca TypeReference> typeRef = new TypeReference>() { }; Map rep = JsonSerialization.readValue(inputStream, typeRef); - realm.patchRealmLocalizationTexts(locale, rep); + realm.createOrUpdateRealmLocalizationTexts(locale, rep); } catch (IOException e) { throw new BadRequestException("Could not read file."); } } - @PATCH + @POST @Path("{locale}") @Consumes(MediaType.APPLICATION_JSON) - public void patchRealmLocalizationTexts(@PathParam("locale") String locale, Map loclizationTexts) { + public void createOrUpdateRealmLocalizationTexts(@PathParam("locale") String locale, + Map localizationTexts) { this.auth.realm().requireManageRealm(); - realm.patchRealmLocalizationTexts(locale, loclizationTexts); + realm.createOrUpdateRealmLocalizationTexts(locale, localizationTexts); } @Path("{locale}") diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/RealmLocalizationResourceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/RealmLocalizationResourceTest.java index 151082a089d0..b49c40ae432f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/RealmLocalizationResourceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/RealmLocalizationResourceTest.java @@ -17,15 +17,16 @@ package org.keycloak.testsuite.admin; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; import org.hamcrest.CoreMatchers; import org.junit.Before; import org.junit.Test; import org.keycloak.admin.client.resource.RealmLocalizationResource; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -44,6 +45,7 @@ public void before() { getCleanup().addLocalization("en"); getCleanup().addLocalization("de"); + getCleanup().addLocalization("es"); resource = adminClient.realm(REALM_NAME).localization(); } @@ -127,4 +129,31 @@ public void deleteRealmLocalizationTexts() { assertThat(localizations, CoreMatchers.hasItems("de")); } + + @Test + public void createOrUpdateRealmLocalizationWhenLocaleDoesNotYetExist() { + final Map newLocalizationTexts = new HashMap<>(); + newLocalizationTexts.put("key-a", "text-a_es"); + newLocalizationTexts.put("key-b", "text-b_es"); + + resource.createOrUpdateRealmLocalizationTexts("es", newLocalizationTexts); + + final Map persistedLocalizationTexts = resource.getRealmLocalizationTexts("es"); + assertEquals(newLocalizationTexts, persistedLocalizationTexts); + } + + @Test + public void createOrUpdateRealmLocalizationWhenLocaleAlreadyExists() { + final Map newLocalizationTexts = new HashMap<>(); + newLocalizationTexts.put("key-b", "text-b_changed_en"); + newLocalizationTexts.put("key-c", "text-c_en"); + + resource.createOrUpdateRealmLocalizationTexts("en", newLocalizationTexts); + + final Map expectedLocalizationTexts = new HashMap<>(); + expectedLocalizationTexts.put("key-a", "text-a_en"); + expectedLocalizationTexts.putAll(newLocalizationTexts); + final Map persistedLocalizationTexts = resource.getRealmLocalizationTexts("en"); + assertEquals(expectedLocalizationTexts, persistedLocalizationTexts); + } }