Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.appsmith.server.dtos;

/**
* This class is used to store information about a field in a class. It stores the full path of the field and the type
* of the field.
* e.g. If we have a class with the following structure:
* class A {
* Field1 field1;
* Field2 field2;
* }
* class Field1 {
* String field3;
* Boolean field4;
* }
* This will result in the following FieldInfo objects:
* field3 => FieldInfo("field1.field3", String.class)
* field4 => FieldInfo("field1.field4", Boolean.class)
* @param fullPath
* @param type
*/
public record FieldInfo(String fullPath, Class<?> type) {}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@

import com.appsmith.server.dtos.ce.RecentlyUsedEntityCE_DTO;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import lombok.experimental.FieldNameConstants;

@Getter
@Setter
@NoArgsConstructor
@FieldNameConstants
public class RecentlyUsedEntityDTO extends RecentlyUsedEntityCE_DTO {
public static class Fields extends RecentlyUsedEntityCE_DTO.Fields {}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.appsmith.server.helpers;

public class AppsmithClassUtils {

/**
* This method checks if the given class is Appsmith defined projection class.
* @param clazz The class to be checked
* @return True if the class is a projection class, false otherwise
*/
public static boolean isAppsmithProjections(Class<?> clazz) {
return clazz.getPackageName().matches(".*appsmith.*projections");
}

/**
* This method checks if the given class is an Appsmith defined class.
* @param clazz The class to be checked
* @return True if the class is an Appsmith defined class, false otherwise
*/
public static boolean isAppsmithDefinedClass(Class<?> clazz) {
return clazz.getPackageName().startsWith("com.appsmith");
}
}
Original file line number Diff line number Diff line change
@@ -1,60 +1,190 @@
package com.appsmith.server.helpers.ce;

import com.appsmith.server.dtos.FieldInfo;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.springframework.util.CollectionUtils;

import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Map;

import static com.appsmith.external.helpers.StringUtils.dotted;
import static com.appsmith.server.helpers.AppsmithClassUtils.isAppsmithDefinedClass;
import static com.appsmith.server.helpers.AppsmithClassUtils.isAppsmithProjections;
import static org.modelmapper.internal.util.Primitives.isPrimitiveWrapper;

public class ReflectionHelpers {

private static final ObjectMapper objectMapper = new ObjectMapper();

/**
* Maps a tuple to an object of the given type using the constructor of the type. The order of the tuple should be
* the same as the order of the fields in the type constructor.
* Maps objects to the given type using the constructor of the type. The order of the objects should be the same as
* the order of the fields in the type constructor.
* @param objects The objects to be mapped to the type. This holds the values of the fields of type objectTypes
* @param type The type of the object to be created
* @param tuple The tuple to be mapped to the object
* @param tupleTypes The types of the tuple elements. If not provided, the types of the fields of the type are used.
* @param objectTypes The types of the objects elements. If not provided, the types of the fields of the type are
* used.
*
* @return The object of the given type
* @param <T> The type of the object to be created
*/
private static <T> T map(Object[] tuple, Class<T> type, List<Class<?>> tupleTypes) {
if (CollectionUtils.isEmpty(tupleTypes)) {
tupleTypes = new ArrayList<>();
for (Field field : type.getDeclaredFields()) {
tupleTypes.add(field.getType());
}
private static <T> T map(ArrayList<Object> objects, Class<T> type, List<Class<?>> objectTypes) {
if (CollectionUtils.isEmpty(objectTypes)) {
objectTypes = fetchAllFieldTypes(type);
}
try {
Constructor<T> constructor = type.getConstructor(tupleTypes.toArray(new Class<?>[tuple.length]));
return constructor.newInstance(tuple);
// Create a deep copy of the objects
ArrayList<Object> modified = new ArrayList<>(objects.size());
for (Class<?> objectType : objectTypes) {
// In case of Appsmith based projection loop through each field to avoid mapping all the fields from
// the entity class
// e.g. class EntityClass {
// private String field1;
// private String field2;
// }
// class ProjectionClass {
// private String field1;
// }
// In the above example, we only need to map field1 from EntityClass to ProjectionClass. This is
// because in the objects param we expect only field1 value to be present.
if (isAppsmithProjections(objectType)) {
modified.add(map(objects, objectType, null));
} else {
Object value = null;
if (!CollectionUtils.isEmpty(objects)) {
value = objects.get(0) != null
&& (isCollectionType(objectType) || isAppsmithDefinedClass(objectType))
? objectMapper.readValue(objectMapper.writeValueAsString(objects.get(0)), objectType)
: objects.get(0);
// Drop the first element from objects as it has been processed
objects.remove(0);
}
modified.add(value);
}
}
Constructor<T> constructor = type.getConstructor(objectTypes.toArray(new Class<?>[0]));
return constructor.newInstance(modified.toArray());
} catch (Exception e) {
throw new RuntimeException(e);
}
}

public static <T> T map(Object[] tuple, Class<T> type) {
return map(tuple, type, null);
/**
* Maps a row from the database to an object of the given type using the constructor of the type.
* @param row The row to be mapped to the object
* @param type The type of the object to be created
*
* @return The object of the given type
* @param <T> The type of the object to be created
*/
public static <T> T map(Object[] row, Class<T> type) {
ArrayList<Object> update = new ArrayList<>(Arrays.asList(row));
return map(update, type, null);
}

/**
* Maps a list of tuples to a list of objects of the given type using the constructor of the type.
* @param type The type of the object to be created
* @param clazz The type of the object to be created
* @param records The list of tuples to be mapped to the objects
*
* @return The list of objects of the given type
* @param <T> The type of the object to be created
*/
public static <T> List<T> map(List<Object[]> records, Class<T> type) {
public static <T> List<T> map(List<Object[]> records, Class<T> clazz) {
List<T> result = new ArrayList<>();
List<Class<?>> tupleTypes = new ArrayList<>();
for (Field field : type.getDeclaredFields()) {
tupleTypes.add(field.getType());
}
// In case of multiple records avoid fetching the field types for each record
List<Class<?>> fieldTypes = fetchAllFieldTypes(clazz);
for (Object[] record : records) {
result.add(map(record, type, tupleTypes));
ArrayList<Object> update = new ArrayList<>(Arrays.asList(record));
result.add(map(update, clazz, fieldTypes));
}
return result;
}

/**
* Fetches all the field types of a class and its superclasses.
* @param clazz The class whose field types are to be fetched
*
* @return The list of field types
*/
private static List<Class<?>> fetchAllFieldTypes(Class<?> clazz) {
List<Class<?>> tupleTypes = new ArrayList<>();

// Traverse the class hierarchy to get fields from the class and its superclasses
while (clazz != null) {
// Get declared fields from the current class
for (Field field : clazz.getDeclaredFields()) {
// Ensure access to private fields
field.setAccessible(true);
tupleTypes.add(field.getType());
}
// Move to the superclass
clazz = clazz.getSuperclass();
}

return tupleTypes;
}

/**
* Check if the class is a Java container class e.g. List, Set, Map etc.
* @param clazz The class to be checked
*
* @return True if the class is a container class, false otherwise
*/
private static boolean isCollectionType(Class<?> clazz) {
// Check if the class is a subtype of Collection or Map
return Collection.class.isAssignableFrom(clazz) || Map.class.isAssignableFrom(clazz);
}

/**
* Extracts all the field paths along-with the field type of the projection class.
* @param projectionClass The projection class whose field paths are to be extracted
*
* @return The list of field paths
*/
public static List<FieldInfo> extractFieldPaths(Class<?> projectionClass) {
List<FieldInfo> fieldPaths = new ArrayList<>();
List<Class<?>> visitedClasses = new ArrayList<>();
extractFieldPathsRecursively(projectionClass, "", fieldPaths, visitedClasses);
return fieldPaths;
}

private static void extractFieldPathsRecursively(
Class<?> clazz, String parentPath, List<FieldInfo> fieldPaths, List<Class<?>> visitedClasses) {
// Check if the class has already been visited to prevent cyclic dependencies
if (visitedClasses.contains(clazz)) {
String cyclicChain = String.join(
" -> ", visitedClasses.stream().map(Class::getName).toArray(String[]::new));
throw new RuntimeException("Cyclical dependency detected for: " + cyclicChain);
}
// Process the class and its superclasses
while (clazz != null && clazz != Object.class) {
for (Field field : clazz.getDeclaredFields()) {
field.setAccessible(true); // Ensure access to private fields
String fieldName = field.getName();
String fullPath = parentPath.isEmpty() ? fieldName : dotted(parentPath, fieldName);
Class<?> fieldType = field.getType();

if (isAppsmithProjections(fieldType)) {
visitedClasses.add(fieldType);
extractFieldPathsRecursively(field.getType(), fullPath, fieldPaths, visitedClasses);
} else {
// Check if the field type is part of JdbcType.getDdlTypeCode if not assign the Object as the type
if (isPrimitiveWrapper(field.getType()) || String.class.equals(field.getType())) {
fieldPaths.add(new FieldInfo(fullPath, field.getType()));
} else {
fieldPaths.add(new FieldInfo(fullPath, Object.class));
}
}
}
// Move to superclass (if any)
clazz = clazz.getSuperclass();
}
// Remove the class from the visited set to allow revisiting in different branches
visitedClasses.remove(clazz);
}
Comment on lines +149 to +189
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider more graceful handling of cyclic dependencies

While the cyclic dependency detection is good, throwing a runtime exception might be too severe. Consider logging a warning and skipping the cyclic path instead.

Suggested modification:

-            throw new RuntimeException("Cyclical dependency detected for: " + cyclicChain);
+            System.err.println("Warning: Cyclical dependency detected for: " + cyclicChain);
+            return;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static List<FieldInfo> extractFieldPaths(Class<?> projectionClass) {
List<FieldInfo> fieldPaths = new ArrayList<>();
List<Class<?>> visitedClasses = new ArrayList<>();
extractFieldPathsRecursively(projectionClass, "", fieldPaths, visitedClasses);
return fieldPaths;
}
private static void extractFieldPathsRecursively(
Class<?> clazz, String parentPath, List<FieldInfo> fieldPaths, List<Class<?>> visitedClasses) {
// Check if the class has already been visited to prevent cyclic dependencies
if (visitedClasses.contains(clazz)) {
String cyclicChain = String.join(
" -> ", visitedClasses.stream().map(Class::getName).toArray(String[]::new));
throw new RuntimeException("Cyclical dependency detected for: " + cyclicChain);
}
// Process the class and its superclasses
while (clazz != null && clazz != Object.class) {
for (Field field : clazz.getDeclaredFields()) {
field.setAccessible(true); // Ensure access to private fields
String fieldName = field.getName();
String fullPath = parentPath.isEmpty() ? fieldName : dotted(parentPath, fieldName);
Class<?> fieldType = field.getType();
if (isAppsmithProjections(fieldType)) {
visitedClasses.add(fieldType);
extractFieldPathsRecursively(field.getType(), fullPath, fieldPaths, visitedClasses);
} else {
// Check if the field type is part of JdbcType.getDdlTypeCode if not assign the Object as the type
if (isPrimitiveWrapper(field.getType()) || String.class.equals(field.getType())) {
fieldPaths.add(new FieldInfo(fullPath, field.getType()));
} else {
fieldPaths.add(new FieldInfo(fullPath, Object.class));
}
}
}
// Move to superclass (if any)
clazz = clazz.getSuperclass();
}
// Remove the class from the visited set to allow revisiting in different branches
visitedClasses.remove(clazz);
}
public static List<FieldInfo> extractFieldPaths(Class<?> projectionClass) {
List<FieldInfo> fieldPaths = new ArrayList<>();
List<Class<?>> visitedClasses = new ArrayList<>();
extractFieldPathsRecursively(projectionClass, "", fieldPaths, visitedClasses);
return fieldPaths;
}
private static void extractFieldPathsRecursively(
Class<?> clazz, String parentPath, List<FieldInfo> fieldPaths, List<Class<?>> visitedClasses) {
// Check if the class has already been visited to prevent cyclic dependencies
if (visitedClasses.contains(clazz)) {
String cyclicChain = String.join(
" -> ", visitedClasses.stream().map(Class::getName).toArray(String[]::new));
System.err.println("Warning: Cyclical dependency detected for: " + cyclicChain);
return;
}
// Process the class and its superclasses
while (clazz != null && clazz != Object.class) {
for (Field field : clazz.getDeclaredFields()) {
field.setAccessible(true); // Ensure access to private fields
String fieldName = field.getName();
String fullPath = parentPath.isEmpty() ? fieldName : dotted(parentPath, fieldName);
Class<?> fieldType = field.getType();
if (isAppsmithProjections(fieldType)) {
visitedClasses.add(fieldType);
extractFieldPathsRecursively(field.getType(), fullPath, fieldPaths, visitedClasses);
} else {
// Check if the field type is part of JdbcType.getDdlTypeCode if not assign the Object as the type
if (isPrimitiveWrapper(field.getType()) || String.class.equals(field.getType())) {
fieldPaths.add(new FieldInfo(fullPath, field.getType()));
} else {
fieldPaths.add(new FieldInfo(fullPath, Object.class));
}
}
}
// Move to superclass (if any)
clazz = clazz.getSuperclass();
}
// Remove the class from the visited set to allow revisiting in different branches
visitedClasses.remove(clazz);
}

Copy link
Contributor Author

@abhvsn abhvsn Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to fail early in such scenario so that devs can come up with better datastructure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abhvsn, understood. Failing early in such scenarios makes sense to encourage better data structures. Thanks for clarifying.

(_/)
( -_-) thinking...


✏️ Learnings added
Learnt from: abhvsn
PR: appsmithorg/appsmith#36988
File: app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/ReflectionHelpers.java:149-189
Timestamp: 2024-10-22T03:15:41.280Z
Learning: In methods like `extractFieldPathsRecursively` in `ReflectionHelpers.java`, when detecting cyclic dependencies, we prefer to fail early by throwing a `RuntimeException` to encourage developers to design better data structures.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

}
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public BridgeQuery<T> jsonIn(@NonNull String needle, @NonNull String key) {
return this;
}

private static <R> Expression<R> keyToExpression(
public static <R> Expression<R> keyToExpression(
@NonNull Class<R> type, @NonNull Root<?> root, @NonNull CriteriaBuilder cb, @NonNull String key) {
if (key.contains(".")) {
final List<String> parts = List.of(key.split("\\."));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,24 @@
package com.appsmith.server.projections;

import com.appsmith.server.dtos.RecentlyUsedEntityDTO;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.Getter;

import java.util.ArrayList;
import java.util.List;

public record UserRecentlyUsedEntitiesProjection(List<RecentlyUsedEntityDTO> recentlyUsedEntityIds) {}
@Getter
public class UserRecentlyUsedEntitiesProjection {
List<RecentlyUsedEntityDTO> recentlyUsedEntityIds = new ArrayList<>();

public UserRecentlyUsedEntitiesProjection(List<Object> recentlyUsedEntityIds) {
if (recentlyUsedEntityIds == null) {
return;
}
// TODO Abhijeet: This is a temporary fix to convert the list of Object to list of RecentlyUsedEntityDTO
recentlyUsedEntityIds.forEach(recentlyUsedEntityId -> {
this.recentlyUsedEntityIds.add(
new ObjectMapper().convertValue(recentlyUsedEntityId, RecentlyUsedEntityDTO.class));
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.appsmith.server.acl.AclPermission;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.domains.User;
import com.appsmith.server.dtos.FieldInfo;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.ce.bridge.Bridge;
Expand Down Expand Up @@ -62,7 +63,9 @@

import static com.appsmith.external.helpers.ReflectionHelpers.getAllFields;
import static com.appsmith.server.constants.FieldName.PERMISSION_GROUPS;
import static com.appsmith.server.helpers.ce.ReflectionHelpers.extractFieldPaths;
import static com.appsmith.server.helpers.ce.ReflectionHelpers.map;
import static com.appsmith.server.helpers.ce.bridge.BridgeQuery.keyToExpression;

/**
* In case you are wondering why we have two different repository implementation classes i.e.
Expand Down Expand Up @@ -297,8 +300,13 @@ public <P> List<P> queryAllExecute(QueryAllParams<T> params, Class<P> projection

if (!projectionClass.getSimpleName().equals(genericDomain.getSimpleName())) {
List<Selection<?>> projectionFields = new ArrayList<>();
// TODO: Nested fields are not supported yet.
getAllFields(projectionClass).forEach(f -> projectionFields.add(root.get(f.getName())));
// Extract all field paths dynamically from the projection class
// Map of projection field path to the class type
List<FieldInfo> fieldPaths = extractFieldPaths(projectionClass);

for (FieldInfo fieldInfo : fieldPaths) {
projectionFields.add(keyToExpression(fieldInfo.type(), root, cb, fieldInfo.fullPath()));
}
cq.multiselect(projectionFields);
}

Expand All @@ -309,11 +317,11 @@ public <P> List<P> queryAllExecute(QueryAllParams<T> params, Class<P> projection
}

return Mono.fromSupplier(query::getResultList)
.map(tuple -> {
.map(rows -> {
if (genericDomain.getSimpleName().equals(projectionClass.getSimpleName())) {
return (List<P>) tuple;
return (List<P>) rows;
}
return map((List<Object[]>) tuple, projectionClass);
return map((List<Object[]>) rows, projectionClass);
})
.onErrorResume(NoResultException.class, e -> Mono.just(Collections.emptyList()));
})
Expand Down Expand Up @@ -371,11 +379,11 @@ public <P> Optional<P> queryOneExecute(QueryAllParams<T> params, Class<P> projec
}

return Mono.fromSupplier(entityManager.createQuery(cq)::getSingleResult)
.map(tuple -> {
.map(row -> {
if (genericDomain.getSimpleName().equals(projectionClass.getSimpleName())) {
return (P) tuple;
return (P) row;
}
return map((Object[]) tuple, projectionClass);
return map((Object[]) row, projectionClass);
})
.onErrorResume(NoResultException.class, e -> Mono.empty());
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public Optional<String> fetchMostRecentlyUsedWorkspaceId(String userId) {
.criteria(Bridge.equal(UserData.Fields.userId, userId))
.one(UserRecentlyUsedEntitiesProjection.class)
.map(userData -> {
final List<RecentlyUsedEntityDTO> recentlyUsedWorkspaceIds = userData.recentlyUsedEntityIds();
final List<RecentlyUsedEntityDTO> recentlyUsedWorkspaceIds = userData.getRecentlyUsedEntityIds();
return CollectionUtils.isEmpty(recentlyUsedWorkspaceIds)
? ""
: recentlyUsedWorkspaceIds.get(0).getWorkspaceId();
Expand Down