From 94003c0a45d89a8fd673b30e0795c13d8f1be940 Mon Sep 17 00:00:00 2001 From: georgweiss Date: Wed, 20 Nov 2024 10:30:39 +0100 Subject: [PATCH] Validation of property must also consider attributes --- .../org/phoebus/olog/LogTemplateResource.java | 8 +- src/main/java/org/phoebus/olog/TextUtil.java | 2 +- .../phoebus/olog/LogTemplateResourceTest.java | 95 +++++++++++++++++++ 3 files changed, 100 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/phoebus/olog/LogTemplateResource.java b/src/main/java/org/phoebus/olog/LogTemplateResource.java index f841095..5dec49c 100644 --- a/src/main/java/org/phoebus/olog/LogTemplateResource.java +++ b/src/main/java/org/phoebus/olog/LogTemplateResource.java @@ -102,12 +102,12 @@ public LogTemplate createLogTemplate(@RequestBody LogTemplate logTemplate, } } + // Check that template contains valid properties and attributes. Take advantage of Property#equals(). Set properties = logTemplate.getProperties(); if(properties != null && !properties.isEmpty()){ - Set propertyNames = properties.stream().map(Property::getName).collect(Collectors.toSet()); - Set persistedProperties = new HashSet<>(); - propertyRepository.findAll().forEach(p -> persistedProperties.add(p.getName())); - if (!CollectionUtils.containsAll(persistedProperties, propertyNames)) { + Set persistedProperties = new HashSet<>(); + propertyRepository.findAll().forEach(p -> persistedProperties.add(p)); + if (!CollectionUtils.containsAll(persistedProperties, properties)) { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, TextUtil.LOG_INVALID_PROPERTIES); } } diff --git a/src/main/java/org/phoebus/olog/TextUtil.java b/src/main/java/org/phoebus/olog/TextUtil.java index e0c3678..337f2ad 100644 --- a/src/main/java/org/phoebus/olog/TextUtil.java +++ b/src/main/java/org/phoebus/olog/TextUtil.java @@ -94,7 +94,7 @@ public class TextUtil { public static final String LOG_ID_NOT_FOUND = "Log id {0} not found"; public static final String LOG_INVALID_LOGBOOKS = "One or more invalid logbook name(s)"; public static final String LOG_INVALID_TAGS = "One or more invalid tag name(s)"; - public static final String LOG_INVALID_PROPERTIES = "One or more invalid property name(s)"; + public static final String LOG_INVALID_PROPERTIES = "One or more invalid property name(s), or invalid attribute list"; public static final String LOG_MUST_HAVE_LOGBOOK = "A log entry must specify at least one logbook"; public static final String LOG_MUST_HAVE_TITLE = "A log entry must specify a title"; public static final String LOG_NOT_ARCHIVED = "Failed to archive log with id {0}"; diff --git a/src/test/java/org/phoebus/olog/LogTemplateResourceTest.java b/src/test/java/org/phoebus/olog/LogTemplateResourceTest.java index 4bde7cc..44053f1 100644 --- a/src/test/java/org/phoebus/olog/LogTemplateResourceTest.java +++ b/src/test/java/org/phoebus/olog/LogTemplateResourceTest.java @@ -44,6 +44,7 @@ import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import org.springframework.web.server.ResponseStatusException; +import org.testcontainers.shaded.org.checkerframework.checker.units.qual.A; import java.util.Arrays; import java.util.Collections; @@ -250,6 +251,100 @@ void testCreateLogTemplateBadProperties() throws Exception { reset(logTemplateRepository); } + @Test + void testCreateLogTemplateBadPropertyAttributes() throws Exception { + LogTemplate logTemplate = new LogTemplate(); + logTemplate.setName("name"); + logTemplate.setId(UUID.randomUUID().toString()); + logTemplate.setOwner("user"); + logTemplate.setTitle("title"); + logTemplate.setSource("description"); + logTemplate.setLevel("Urgent"); + Property property = new Property("validPropName"); + Attribute attribute1 = new Attribute("validAttributeName"); + Attribute attribute2 = new Attribute("invalidAttributeName"); + property.setAttributes(Set.of(attribute1, attribute2)); + logTemplate.setProperties(Set.of(property)); + + Attribute attribute3 = new Attribute("validAttribute2"); + Property persistedProperty = new Property("validPropName"); + persistedProperty.setAttributes(Set.of(attribute1, attribute3)); + + + when(propertyRepository.findAll()).thenReturn(Collections.singletonList(persistedProperty)); + when(logTemplateRepository.save(argThat(new LogTemplateMatcher(logTemplate)))).thenReturn(logTemplate); + MockHttpServletRequestBuilder request = put("/" + OlogResourceDescriptors.LOG_TEMPLATE_RESOURCE_URI) + .content(objectMapper.writeValueAsString(logTemplate)) + .header(HttpHeaders.AUTHORIZATION, AUTHORIZATION) + .contentType(JSON); + mockMvc.perform(request).andExpect(status().isBadRequest()); + reset(propertyRepository); + reset(logTemplateRepository); + } + + /** + * Test with fewer attributes than persisted property + * @throws Exception + */ + @Test + void testCreateLogTemplateBadPropertyAttributes2() throws Exception { + LogTemplate logTemplate = new LogTemplate(); + logTemplate.setName("name"); + logTemplate.setId(UUID.randomUUID().toString()); + logTemplate.setOwner("user"); + logTemplate.setTitle("title"); + logTemplate.setSource("description"); + logTemplate.setLevel("Urgent"); + Property property = new Property("validPropName"); + Attribute attribute1 = new Attribute("validAttributeName"); + property.setAttributes(Set.of(attribute1)); + logTemplate.setProperties(Set.of(property)); + + Attribute attribute3 = new Attribute("validAttribute2"); + Property persistedProperty = new Property("validPropName"); + persistedProperty.setAttributes(Set.of(attribute1, attribute3)); + + + when(propertyRepository.findAll()).thenReturn(Collections.singletonList(persistedProperty)); + when(logTemplateRepository.save(argThat(new LogTemplateMatcher(logTemplate)))).thenReturn(logTemplate); + MockHttpServletRequestBuilder request = put("/" + OlogResourceDescriptors.LOG_TEMPLATE_RESOURCE_URI) + .content(objectMapper.writeValueAsString(logTemplate)) + .header(HttpHeaders.AUTHORIZATION, AUTHORIZATION) + .contentType(JSON); + mockMvc.perform(request).andExpect(status().isBadRequest()); + reset(propertyRepository); + reset(logTemplateRepository); + } + + @Test + void testCreateLogTemplatePropertyAttributes() throws Exception { + LogTemplate logTemplate = new LogTemplate(); + logTemplate.setName("name"); + logTemplate.setId(UUID.randomUUID().toString()); + logTemplate.setOwner("user"); + logTemplate.setTitle("title"); + logTemplate.setSource("description"); + logTemplate.setLevel("Urgent"); + Property property = new Property("validPropName"); + Attribute attribute1 = new Attribute("validAttributeName"); + Attribute attribute2 = new Attribute("validAttributeName2"); + property.setAttributes(Set.of(attribute1, attribute2)); + logTemplate.setProperties(Set.of(property)); + + Property persistedProperty = new Property("validPropName"); + persistedProperty.setAttributes(Set.of(attribute1, attribute2)); + + when(propertyRepository.findAll()).thenReturn(Collections.singletonList(persistedProperty)); + when(logTemplateRepository.save(argThat(new LogTemplateMatcher(logTemplate)))).thenReturn(logTemplate); + MockHttpServletRequestBuilder request = put("/" + OlogResourceDescriptors.LOG_TEMPLATE_RESOURCE_URI) + .content(objectMapper.writeValueAsString(logTemplate)) + .header(HttpHeaders.AUTHORIZATION, AUTHORIZATION) + .contentType(JSON); + mockMvc.perform(request).andExpect(status().isOk()); + reset(propertyRepository); + reset(logTemplateRepository); + } + @Test void testCreateLogTemplateDuplicateName() throws Exception { LogTemplate existing = new LogTemplate();