From 9cc005015c02fff28eb68ed8c35632050606b37f Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Thu, 14 Feb 2019 16:26:09 -0700 Subject: [PATCH] [SHIBUI-1058][SHIBUI-1231] Fixed a bug where we were persisting boolean relying party overrides improperly. We now only persist when the value from the UI is true. Note that we still persist based on how the override is defined in the yaml. In other words, the UI controls whether or not to persist and the yaml controls what is actually persisted. --- .../ui/service/JPAEntityServiceImpl.java | 29 ++++++++++--------- .../admin/ui/util/TestHelpers.groovy | 4 +-- .../admin/ui/util/TestObjectGenerator.groovy | 7 +---- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java index 9e32d371c..7d9e9388c 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java @@ -104,22 +104,23 @@ public List getAttributeListFromRelyingPartyOverridesRepresentation(M RelyingPartyOverrideProperty overrideProperty = overridePropertyList.stream().filter(op -> op.getName().equals(key)).findFirst().get(); switch (ModelRepresentationConversions.AttributeTypes.valueOf(overrideProperty.getDisplayType().toUpperCase())) { case BOOLEAN: - if (overrideProperty.getPersistType() != null && - !overrideProperty.getPersistType().equalsIgnoreCase("boolean") && - ((entry.getValue() instanceof Boolean && (Boolean)entry.getValue()) || - ((entry.getValue() instanceof String) && Boolean.valueOf((String)entry.getValue())))) { - list.add(attributeUtility.createAttributeWithStringValues(overrideProperty.getAttributeName(), - overrideProperty.getAttributeFriendlyName(), - overrideProperty.getPersistValue())); - } else { - if (entry.getValue() instanceof String) { - list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(), + if ((entry.getValue() instanceof Boolean && (Boolean)entry.getValue()) || + ((entry.getValue() instanceof String) && Boolean.valueOf((String)entry.getValue()))) { + if (overrideProperty.getPersistType() != null && + !overrideProperty.getPersistType().equalsIgnoreCase("boolean")) { + list.add(attributeUtility.createAttributeWithStringValues(overrideProperty.getAttributeName(), overrideProperty.getAttributeFriendlyName(), - Boolean.valueOf((String) entry.getValue()))); + overrideProperty.getPersistValue())); } else { - list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(), - overrideProperty.getAttributeFriendlyName(), - (Boolean) entry.getValue())); + if (entry.getValue() instanceof String) { + list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(), + overrideProperty.getAttributeFriendlyName(), + Boolean.valueOf((String) entry.getValue()))); + } else { + list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(), + overrideProperty.getAttributeFriendlyName(), + (Boolean) entry.getValue())); + } } } break; diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestHelpers.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestHelpers.groovy index 672618b30..0b175c5ad 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestHelpers.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestHelpers.groovy @@ -20,8 +20,8 @@ class TestHelpers { count += ((Collection)entry.value).size() != 0 ? 1 : 0 } else if (entry.value instanceof String) { count += StringUtils.isNotBlank((String)entry.value) ? 1 : 0 - } else { - count++ + } else if (entry.value instanceof Boolean) { + count += Boolean.valueOf((Boolean)entry.value) ? 1 : 0 } } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy index 8eb8426f4..11b0d7b36 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy @@ -348,12 +348,7 @@ class TestObjectGenerator { customPropertiesConfiguration.getOverrides().each { override -> switch (ModelRepresentationConversions.AttributeTypes.valueOf(override.getDisplayType().toUpperCase())) { case ModelRepresentationConversions.AttributeTypes.BOOLEAN: - if (override.getPersistType() != null && - override.getPersistType() != override.getDisplayType()) { - representation.put(override.getName(), generator.randomString(30)) - } else { - representation.put(override.getName(), generator.randomBoolean()) - } + representation.put(override.getName(), generator.randomBoolean()) break case ModelRepresentationConversions.AttributeTypes.INTEGER: representation.put(override.getName(), generator.randomInt(0, 999999))