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
This commit is contained in:
Daniel Fesenmeyer 2021-04-27 16:19:37 +02:00 committed by Stian Thorgersen
parent d92bb7df65
commit 0a2f8f5b63
8 changed files with 57 additions and 18 deletions

View file

@ -23,6 +23,7 @@ import java.util.Map;
import javax.ws.rs.Consumes; import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE; import javax.ws.rs.DELETE;
import javax.ws.rs.GET; import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.PUT; import javax.ws.rs.PUT;
import javax.ws.rs.Path; import javax.ws.rs.Path;
import javax.ws.rs.PathParam; import javax.ws.rs.PathParam;
@ -59,4 +60,9 @@ public interface RealmLocalizationResource {
@PUT @PUT
@Consumes(MediaType.TEXT_PLAIN) @Consumes(MediaType.TEXT_PLAIN)
void saveRealmLocalizationText(@PathParam("locale") String locale, @PathParam("key") String key, String text); 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<String, String> localizationTexts);
} }

View file

@ -1698,9 +1698,9 @@ public class RealmAdapter implements CachedRealmModel {
} }
@Override @Override
public void patchRealmLocalizationTexts(String locale, Map<String, String> localizationTexts) { public void createOrUpdateRealmLocalizationTexts(String locale, Map<String, String> localizationTexts) {
getDelegateForUpdate(); getDelegateForUpdate();
updated.patchRealmLocalizationTexts(locale, localizationTexts); updated.createOrUpdateRealmLocalizationTexts(locale, localizationTexts);
} }
@Override @Override

View file

@ -2173,11 +2173,13 @@ public class RealmAdapter implements RealmModel, JpaModel<RealmEntity> {
} }
@Override @Override
public void patchRealmLocalizationTexts(String locale, Map<String, String> localizationTexts) { public void createOrUpdateRealmLocalizationTexts(String locale, Map<String, String> localizationTexts) {
Map<String, RealmLocalizationTextsEntity> currentLocalizationTexts = realm.getRealmLocalizationTexts(); Map<String, RealmLocalizationTextsEntity> currentLocalizationTexts = realm.getRealmLocalizationTexts();
if(currentLocalizationTexts.containsKey(locale)) { if(currentLocalizationTexts.containsKey(locale)) {
RealmLocalizationTextsEntity localizationTextsEntity = currentLocalizationTexts.get(locale); RealmLocalizationTextsEntity localizationTextsEntity = currentLocalizationTexts.get(locale);
localizationTextsEntity.getTexts().putAll(localizationTexts); Map<String, String> updatedTexts = new HashMap<>(localizationTextsEntity.getTexts());
updatedTexts.putAll(localizationTexts);
localizationTextsEntity.setTexts(updatedTexts);
em.persist(localizationTextsEntity); em.persist(localizationTextsEntity);
} }

View file

@ -1294,7 +1294,7 @@ public class MapRealmAdapter extends AbstractRealmModel<MapRealmEntity> implemen
} }
@Override @Override
public void patchRealmLocalizationTexts(String locale, Map<String, String> localizationTexts) { public void createOrUpdateRealmLocalizationTexts(String locale, Map<String, String> localizationTexts) {
Map<String, Map<String, String>> realmLocalizationTexts = entity.getLocalizationTexts(); Map<String, Map<String, String>> realmLocalizationTexts = entity.getLocalizationTexts();
if (realmLocalizationTexts.containsKey(locale)) { if (realmLocalizationTexts.containsKey(locale)) {

View file

@ -175,7 +175,7 @@ public class MapRealmProvider implements RealmProvider {
if (! updateLocalizationText(realm, locale, key, text)) { if (! updateLocalizationText(realm, locale, key, text)) {
Map<String, String> texts = new HashMap<>(); Map<String, String> texts = new HashMap<>();
texts.put(key, text); texts.put(key, text);
realm.patchRealmLocalizationTexts(locale, texts); realm.createOrUpdateRealmLocalizationTexts(locale, texts);
} }
} }
@ -183,7 +183,7 @@ public class MapRealmProvider implements RealmProvider {
@Override @Override
public void saveLocalizationTexts(RealmModel realm, String locale, Map<String, String> localizationTexts) { public void saveLocalizationTexts(RealmModel realm, String locale, Map<String, String> localizationTexts) {
if (locale == null || localizationTexts == null) return; if (locale == null || localizationTexts == null) return;
realm.patchRealmLocalizationTexts(locale, localizationTexts); realm.createOrUpdateRealmLocalizationTexts(locale, localizationTexts);
} }
//TODO move the following method to adapter //TODO move the following method to adapter
@ -192,7 +192,7 @@ public class MapRealmProvider implements RealmProvider {
if (locale == null || key == null || text == null || (! realm.getRealmLocalizationTextsByLocale(locale).containsKey(key))) return false; if (locale == null || key == null || text == null || (! realm.getRealmLocalizationTextsByLocale(locale).containsKey(key))) return false;
Map<String, String> texts = new HashMap<>(realm.getRealmLocalizationTextsByLocale(locale)); Map<String, String> texts = new HashMap<>(realm.getRealmLocalizationTextsByLocale(locale));
texts.replace(key, text); texts.replace(key, text);
realm.patchRealmLocalizationTexts(locale, texts); realm.createOrUpdateRealmLocalizationTexts(locale, texts);
return true; return true;
} }
@ -210,7 +210,7 @@ public class MapRealmProvider implements RealmProvider {
Map<String, String> texts = new HashMap<>(realm.getRealmLocalizationTextsByLocale(locale)); Map<String, String> texts = new HashMap<>(realm.getRealmLocalizationTextsByLocale(locale));
texts.remove(key); texts.remove(key);
realm.removeRealmLocalizationTexts(locale); realm.removeRealmLocalizationTexts(locale);
realm.patchRealmLocalizationTexts(locale, texts); realm.createOrUpdateRealmLocalizationTexts(locale, texts);
return true; return true;
} }

View file

@ -992,10 +992,11 @@ public interface RealmModel extends RoleContainerModel {
void removeDefaultClientScope(ClientScopeModel clientScope); 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. * It updates texts, which are already stored or create new ones if the key does not exist yet.
*/ */
void patchRealmLocalizationTexts(String locale, Map<String, String> localizationTexts); void createOrUpdateRealmLocalizationTexts(String locale, Map<String, String> localizationTexts);
boolean removeRealmLocalizationTexts(String locale); boolean removeRealmLocalizationTexts(String locale);
Map<String, Map<String, String>> getRealmLocalizationTexts(); Map<String, Map<String, String>> getRealmLocalizationTexts();
Map<String, String> getRealmLocalizationTextsByLocale(String locale); Map<String, String> getRealmLocalizationTextsByLocale(String locale);

View file

@ -84,8 +84,8 @@ public class RealmLocalizationResource {
@POST @POST
@Path("{locale}") @Path("{locale}")
@Consumes(MediaType.MULTIPART_FORM_DATA) @Consumes(MediaType.MULTIPART_FORM_DATA)
public void patchRealmLocalizationTextsFromFile(@PathParam("locale") String locale, MultipartFormDataInput input) public void createOrUpdateRealmLocalizationTextsFromFile(@PathParam("locale") String locale,
throws IOException { MultipartFormDataInput input) {
this.auth.realm().requireManageRealm(); this.auth.realm().requireManageRealm();
Map<String, List<InputPart>> formDataMap = input.getFormDataMap(); Map<String, List<InputPart>> formDataMap = input.getFormDataMap();
@ -97,18 +97,19 @@ public class RealmLocalizationResource {
TypeReference<HashMap<String, String>> typeRef = new TypeReference<HashMap<String, String>>() { TypeReference<HashMap<String, String>> typeRef = new TypeReference<HashMap<String, String>>() {
}; };
Map<String, String> rep = JsonSerialization.readValue(inputStream, typeRef); Map<String, String> rep = JsonSerialization.readValue(inputStream, typeRef);
realm.patchRealmLocalizationTexts(locale, rep); realm.createOrUpdateRealmLocalizationTexts(locale, rep);
} catch (IOException e) { } catch (IOException e) {
throw new BadRequestException("Could not read file."); throw new BadRequestException("Could not read file.");
} }
} }
@PATCH @POST
@Path("{locale}") @Path("{locale}")
@Consumes(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON)
public void patchRealmLocalizationTexts(@PathParam("locale") String locale, Map<String, String> loclizationTexts) { public void createOrUpdateRealmLocalizationTexts(@PathParam("locale") String locale,
Map<String, String> localizationTexts) {
this.auth.realm().requireManageRealm(); this.auth.realm().requireManageRealm();
realm.patchRealmLocalizationTexts(locale, loclizationTexts); realm.createOrUpdateRealmLocalizationTexts(locale, localizationTexts);
} }
@Path("{locale}") @Path("{locale}")

View file

@ -17,15 +17,16 @@
package org.keycloak.testsuite.admin; package org.keycloak.testsuite.admin;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import org.hamcrest.CoreMatchers; import org.hamcrest.CoreMatchers;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.keycloak.admin.client.resource.RealmLocalizationResource; import org.keycloak.admin.client.resource.RealmLocalizationResource;
import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
@ -44,6 +45,7 @@ public class RealmLocalizationResourceTest extends AbstractAdminTest {
getCleanup().addLocalization("en"); getCleanup().addLocalization("en");
getCleanup().addLocalization("de"); getCleanup().addLocalization("de");
getCleanup().addLocalization("es");
resource = adminClient.realm(REALM_NAME).localization(); resource = adminClient.realm(REALM_NAME).localization();
} }
@ -127,4 +129,31 @@ public class RealmLocalizationResourceTest extends AbstractAdminTest {
assertThat(localizations, CoreMatchers.hasItems("de")); assertThat(localizations, CoreMatchers.hasItems("de"));
} }
@Test
public void createOrUpdateRealmLocalizationWhenLocaleDoesNotYetExist() {
final Map<String, String> newLocalizationTexts = new HashMap<>();
newLocalizationTexts.put("key-a", "text-a_es");
newLocalizationTexts.put("key-b", "text-b_es");
resource.createOrUpdateRealmLocalizationTexts("es", newLocalizationTexts);
final Map<String, String> persistedLocalizationTexts = resource.getRealmLocalizationTexts("es");
assertEquals(newLocalizationTexts, persistedLocalizationTexts);
}
@Test
public void createOrUpdateRealmLocalizationWhenLocaleAlreadyExists() {
final Map<String, String> newLocalizationTexts = new HashMap<>();
newLocalizationTexts.put("key-b", "text-b_changed_en");
newLocalizationTexts.put("key-c", "text-c_en");
resource.createOrUpdateRealmLocalizationTexts("en", newLocalizationTexts);
final Map<String, String> expectedLocalizationTexts = new HashMap<>();
expectedLocalizationTexts.put("key-a", "text-a_en");
expectedLocalizationTexts.putAll(newLocalizationTexts);
final Map<String, String> persistedLocalizationTexts = resource.getRealmLocalizationTexts("en");
assertEquals(expectedLocalizationTexts, persistedLocalizationTexts);
}
} }