From 76a3d9d01b490b5986c3094a6468e35f8a265e5d Mon Sep 17 00:00:00 2001 From: baixinsui Date: Wed, 26 Feb 2025 18:46:00 +0800 Subject: [PATCH] check name set for all columns --- modules/database/pom.xml | 9 +- .../ServiceChangeRequestEntity.java | 2 +- .../ServiceConfigurationEntity.java | 2 +- .../servicepolicy/ServicePolicyEntity.java | 2 +- .../database/userpolicy/UserPolicyEntity.java | 4 +- .../database/DatabaseArchitectureTest.java | 99 --------- .../database/DatabaseValidationTest.java | 198 ++++++++++++++++++ pom.xml | 1 + 8 files changed, 211 insertions(+), 106 deletions(-) delete mode 100644 modules/database/src/test/java/org/eclipse/xpanse/modules/database/DatabaseArchitectureTest.java create mode 100644 modules/database/src/test/java/org/eclipse/xpanse/modules/database/DatabaseValidationTest.java diff --git a/modules/database/pom.xml b/modules/database/pom.xml index 91a90cbe6..e2754b529 100644 --- a/modules/database/pom.xml +++ b/modules/database/pom.xml @@ -4,8 +4,8 @@ ~ SPDX-FileCopyrightText: Huawei Inc. ~ --> - 4.0.0 @@ -55,6 +55,11 @@ jackson-datatype-jsr310 ${jackson.datatype.jsr310.version} + + org.reflections + reflections + ${org.reflections.version} + diff --git a/modules/database/src/main/java/org/eclipse/xpanse/modules/database/servicechange/ServiceChangeRequestEntity.java b/modules/database/src/main/java/org/eclipse/xpanse/modules/database/servicechange/ServiceChangeRequestEntity.java index 3fe06795a..341efc700 100644 --- a/modules/database/src/main/java/org/eclipse/xpanse/modules/database/servicechange/ServiceChangeRequestEntity.java +++ b/modules/database/src/main/java/org/eclipse/xpanse/modules/database/servicechange/ServiceChangeRequestEntity.java @@ -59,7 +59,7 @@ public class ServiceChangeRequestEntity extends CreatedModifiedTime implements S private String resourceName; - @Column(nullable = false) + @Column(name = "CHANGE_HANDLER", nullable = false) private String changeHandler; private String resultMessage; diff --git a/modules/database/src/main/java/org/eclipse/xpanse/modules/database/serviceconfiguration/ServiceConfigurationEntity.java b/modules/database/src/main/java/org/eclipse/xpanse/modules/database/serviceconfiguration/ServiceConfigurationEntity.java index 7a555f481..850915ec6 100644 --- a/modules/database/src/main/java/org/eclipse/xpanse/modules/database/serviceconfiguration/ServiceConfigurationEntity.java +++ b/modules/database/src/main/java/org/eclipse/xpanse/modules/database/serviceconfiguration/ServiceConfigurationEntity.java @@ -43,7 +43,7 @@ public class ServiceConfigurationEntity { @OnDelete(action = OnDeleteAction.CASCADE) private ServiceDeploymentEntity serviceDeploymentEntity; - @Column(columnDefinition = "json", nullable = false) + @Column(name = "CONFIGURATION", columnDefinition = "json", nullable = false) @Type(value = JsonType.class) @Convert(converter = ObjectJsonConverter.class) private Map configuration; diff --git a/modules/database/src/main/java/org/eclipse/xpanse/modules/database/servicepolicy/ServicePolicyEntity.java b/modules/database/src/main/java/org/eclipse/xpanse/modules/database/servicepolicy/ServicePolicyEntity.java index 182920579..40f665251 100644 --- a/modules/database/src/main/java/org/eclipse/xpanse/modules/database/servicepolicy/ServicePolicyEntity.java +++ b/modules/database/src/main/java/org/eclipse/xpanse/modules/database/servicepolicy/ServicePolicyEntity.java @@ -54,6 +54,6 @@ public class ServicePolicyEntity extends CreatedModifiedTime { private String flavorNames; /** Is the policy enabled. */ - @Column(columnDefinition = "boolean default true") + @Column(name = "ENABLED", nullable = false, columnDefinition = "boolean default true") private Boolean enabled; } diff --git a/modules/database/src/main/java/org/eclipse/xpanse/modules/database/userpolicy/UserPolicyEntity.java b/modules/database/src/main/java/org/eclipse/xpanse/modules/database/userpolicy/UserPolicyEntity.java index d785e170f..5df7087b6 100644 --- a/modules/database/src/main/java/org/eclipse/xpanse/modules/database/userpolicy/UserPolicyEntity.java +++ b/modules/database/src/main/java/org/eclipse/xpanse/modules/database/userpolicy/UserPolicyEntity.java @@ -25,7 +25,7 @@ @Data @Table( name = "USER_POLICY", - uniqueConstraints = {@UniqueConstraint(columnNames = {"USERID", "POLICY", "CSP"})}) + uniqueConstraints = {@UniqueConstraint(columnNames = {"USER_ID", "POLICY", "CSP"})}) @Entity @EqualsAndHashCode(callSuper = true) public class UserPolicyEntity extends CreatedModifiedTime { @@ -50,6 +50,6 @@ public class UserPolicyEntity extends CreatedModifiedTime { private Csp csp; /** Is the policy enabled. */ - @Column(columnDefinition = "boolean default true", nullable = false) + @Column(name = "ENABLED", columnDefinition = "boolean default true", nullable = false) private Boolean enabled; } diff --git a/modules/database/src/test/java/org/eclipse/xpanse/modules/database/DatabaseArchitectureTest.java b/modules/database/src/test/java/org/eclipse/xpanse/modules/database/DatabaseArchitectureTest.java deleted file mode 100644 index 76db73917..000000000 --- a/modules/database/src/test/java/org/eclipse/xpanse/modules/database/DatabaseArchitectureTest.java +++ /dev/null @@ -1,99 +0,0 @@ -package org.eclipse.xpanse.modules.database; - -import java.io.File; -import java.io.IOException; -import java.net.URL; -import java.util.Enumeration; -import lombok.extern.slf4j.Slf4j; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; - -@Slf4j -public class DatabaseArchitectureTest { - - private static final String DATABASE_PACKAGE_NAME = "org.eclipse.xpanse.modules.database"; - - /** - * Check if any class in the repositories have the method findAll(). - * - * @throws IOException if the classloader cannot find the resource. - */ - @Test - void checkMethodFindAllInRepositories() throws IOException { - ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); - String path = DATABASE_PACKAGE_NAME.replace('.', '/'); - Enumeration resources = classLoader.getResources(path); - while (resources.hasMoreElements()) { - URL resource = resources.nextElement(); - File file = new File(resource.getFile()); - if (file.getPath().contains("test-classes")) { - continue; - } - if (resource.getProtocol().equals("file")) { - scanDirectoryForClasses(new File(resource.getFile()), DATABASE_PACKAGE_NAME); - } - } - } - - void scanDirectoryForClasses(File directory, String packageName) { - if (!directory.exists() || !directory.isDirectory()) { - return; - } - File[] files = directory.listFiles(); - if (files != null) { - for (File file : files) { - if (file.isDirectory()) { - scanDirectoryForClasses(file, packageName + "." + file.getName()); - } else if (file.getName().endsWith(".class")) { - String className = - packageName - + '.' - + file.getName().substring(0, file.getName().length() - 6); - try { - Class clazz = Class.forName(className); - checkClazzHasFindAllMethod(clazz, className); - } catch (ClassNotFoundException e) { - log.error("Failed to load class {}", className, e); - } - } - } - } - } - - void checkClazzHasFindAllMethod(Class clazz, String subClassName) { - if (hasMethodFindAll(clazz)) { - Class[] interfaces = clazz.getInterfaces(); - for (Class face : interfaces) { - if (hasMethodFindAll(face)) { - Assertions.fail( - "Class " - + subClassName - + " should not implement the interface " - + face.getName() - + " which declare the method findAll()."); - } - } - Class superclass = clazz.getSuperclass(); - if (superclass != null && superclass != Object.class) { - if (hasMethodFindAll(superclass)) { - Assertions.fail( - "Class " - + subClassName - + " should not extend the class " - + clazz.getName() - + " which declare the method findAll()."); - } - } - Assertions.fail("Class " + subClassName + " should not declare method findAll()."); - } - } - - boolean hasMethodFindAll(Class clazz) { - try { - clazz.getMethod("findAll"); - return true; - } catch (NoSuchMethodException e) { - return false; - } - } -} diff --git a/modules/database/src/test/java/org/eclipse/xpanse/modules/database/DatabaseValidationTest.java b/modules/database/src/test/java/org/eclipse/xpanse/modules/database/DatabaseValidationTest.java new file mode 100644 index 000000000..ad00f1d69 --- /dev/null +++ b/modules/database/src/test/java/org/eclipse/xpanse/modules/database/DatabaseValidationTest.java @@ -0,0 +1,198 @@ +package org.eclipse.xpanse.modules.database; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import java.lang.annotation.Annotation; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.regex.Pattern; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.reflections.Reflections; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider; +import org.springframework.core.type.filter.AnnotationTypeFilter; +import org.springframework.stereotype.Repository; +import org.springframework.util.CollectionUtils; + +@Slf4j +public class DatabaseValidationTest { + + private static final String DATABASE_BASE_PACKAGE = "org.eclipse.xpanse.modules.database"; + + private static final Pattern COLUMN_NAME_PATTERN = Pattern.compile("^[A-Z]+(_[A-Z]+)*$"); + + /** Test to check if any repository class has the method named findAll() in its hierarchy. */ + @Test + void checkMethodFindAllInRepositories() { + List allErrors = new ArrayList<>(); + Set> repositoryClasses = scanClassesWithAnnotation(Repository.class); + repositoryClasses.forEach( + clazz -> { + if (hasFindAllInHierarchy(clazz)) { + List errors = new ArrayList<>(); + errors.add( + String.format( + "Class %s inherits method findAll() from hierarchy", + clazz.getName())); + checkClassHasMethodFindAllInheritance(clazz, errors); + allErrors.addAll(errors); + } + }); + if (!CollectionUtils.isEmpty(allErrors)) { + Assertions.fail(formatErrorMessages(allErrors)); + } + } + + /** Test to check if any entity class has invalid column names. */ + @Test + void validateColumnNamesInEntityClasses() { + List allErrors = new ArrayList<>(); + Set> entityClasses = scanClassesWithAnnotation(Entity.class); + entityClasses.forEach( + entity -> { + List errors = new ArrayList<>(); + validateColumnNamesInEntity(entity, errors); + allErrors.addAll(errors); + }); + if (!CollectionUtils.isEmpty(allErrors)) { + Assertions.fail(formatErrorMessages(allErrors)); + } + } + + private Set> scanClassesWithAnnotation(Class annotation) { + Set> classes = new HashSet<>(); + ClassPathScanningCandidateComponentProvider scanner = + new ClassPathScanningCandidateComponentProvider(false); + scanner.addIncludeFilter(new AnnotationTypeFilter(annotation, true, true)); + Set beanDefinitions = + scanner.findCandidateComponents(DATABASE_BASE_PACKAGE); + log.info( + "Found {} beans with annotation @{} from candidate components.", + beanDefinitions.size(), + annotation.getSimpleName()); + beanDefinitions.forEach( + beanDefinition -> { + try { + classes.add(Class.forName(beanDefinition.getBeanClassName())); + } catch (ClassNotFoundException e) { + log.error( + "Failed to load class by bean class name {}", + beanDefinition.getBeanClassName(), + e); + } + }); + + // if no classes found by beans components, try to load classes from reflections. + if (CollectionUtils.isEmpty(classes)) { + Set> classesWithAnnotation = + new Reflections(DATABASE_BASE_PACKAGE).getTypesAnnotatedWith(annotation); + log.info( + "Found {} classes with annotation @{} from classes reflections.", + classesWithAnnotation.size(), + annotation.getSimpleName()); + classes.addAll(classesWithAnnotation); + } + log.info( + "Found {} classes with annotation @{}.", + classes.size(), + annotation.getSimpleName()); + return classes; + } + + void checkClassHasMethodFindAllInheritance(Class clazz, List errors) { + if (hasDeclaredMethodFindAll(clazz)) { + errors.add( + String.format("Class %s could not declare method findAll()", clazz.getName())); + return; + } + Class superClass = clazz.getSuperclass(); + if (superClass != null && superClass != Object.class) { + if (hasFindAllInHierarchy(superClass)) { + errors.add( + String.format( + "Class %s could not extend class %s has declared method findAll()", + clazz.getName(), superClass.getSimpleName())); + } + } + for (Class interfaceClass : clazz.getInterfaces()) { + if (hasFindAllInHierarchy(interfaceClass)) { + errors.add( + String.format( + "Class %s cloud not implements interface %s has declared method" + + " findAll()", + clazz.getName(), interfaceClass.getSimpleName())); + } + } + } + + boolean hasFindAllInHierarchy(Class clazz) { + try { + Method method = clazz.getMethod("findAll"); + return Modifier.isPublic(method.getModifiers()); + } catch (NoSuchMethodException e) { + return false; + } + } + + boolean hasDeclaredMethodFindAll(Class clazz) { + try { + Method method = clazz.getDeclaredMethod("findAll"); + return Modifier.isPublic(method.getModifiers()); + } catch (NoSuchMethodException e) { + return false; + } + } + + private boolean isEntityClass(Class clazz) { + Optional classHasEntityAnnotation = + Arrays.stream(clazz.getAnnotations()) + .filter(annotation -> annotation.annotationType() == Entity.class) + .findAny(); + return classHasEntityAnnotation.isPresent(); + } + + private void validateColumnNamesInEntity(Class entityClass, List errors) { + + for (Field field : entityClass.getDeclaredFields()) { + if (java.lang.reflect.Modifier.isStatic(field.getModifiers()) + || java.lang.reflect.Modifier.isTransient(field.getModifiers())) { + continue; + } + Annotation[] annotations = field.getAnnotations(); + Optional columnAnnotation = + Arrays.stream(annotations) + .filter(annotation -> annotation.annotationType() == Column.class) + .findAny(); + if (columnAnnotation.isPresent()) { + Column column = (Column) columnAnnotation.get(); + if (StringUtils.isBlank(column.name())) { + errors.add( + String.format( + "%s.%s: name set in annotation @Column cloud not be empty.", + entityClass.getSimpleName(), field.getName())); + } + if (!COLUMN_NAME_PATTERN.matcher(column.name()).matches()) { + errors.add( + String.format( + "%s.%s: format of name set in annotation @Column is invalid.", + entityClass.getSimpleName(), field.getName())); + } + } + } + } + + private String formatErrorMessages(List errors) { + return String.format( + "Found %d validation error(s):\n• %s", errors.size(), String.join("\n• ", errors)); + } +} diff --git a/pom.xml b/pom.xml index 6118a4593..656bfa3d8 100644 --- a/pom.xml +++ b/pom.xml @@ -88,6 +88,7 @@ 2.18.2 2.44.3 4.31.1 + 0.10.2 scm:git:git@github.com:https://github.com/eclipse-xpanse/xpanse.git