Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail to read property due to IllegalArgumentException when using it on withX methods #1601

Open
aviadsTaboola opened this issue Feb 26, 2024 · 4 comments
Assignees
Labels
Milestone

Comments

@aviadsTaboola
Copy link

aviadsTaboola commented Feb 26, 2024

Hi, I am using lombok in java and make immutable object using the With annotation.
tried to use byte-buddy Advice to recognize a change of property in a class , but since it is not a setter it throws an exception:
withStatus does not follow Java bean naming conventions
maybe java bean naming convention is too strict for this...

It will be great to be able to use @Advice.Origin("#p") String propertyName such that it will work with any method that contains the field name even if someone will call it updateX().

if this bug/feat does not seem proper to you I will be glad for a reference for what I can do it otherwise. my goal is to track any field change, so the best way I saw doing it is by assuming this naming convention + checking that the value has changed by returning a value on @Advice.OnMethodEnter and checking the value on exist is different than the value on enter with @Advice.Enter Object value.

thanks!
full code usage:


static class FieldAssignmentInterceptor {
        final static Set<Field> dirtyFields = new HashSet<>();

        @Advice.OnMethodEnter
        public static Object enter(@Advice.Origin Class<?> clazz, // The class that contains the method
                                   @Advice.Origin("#m") String methodName, // The method that is being executed
                                   @Advice.Origin("#p") String propertyName, // The property that is being assigned
                                   @Advice.This Object target // The object that is being modified
        ) {
            
                try {
                    Field field = clazz.getDeclaredField(propertyName);
                    if (shouldTrackMethod(field, methodName, propertyName)) {
                        field.setAccessible(true);
                        return field.get(target);
                    }
                } catch (NoSuchFieldException | IllegalAccessException e) {
                  
                }
            
            return new NoValueMarker();
        }

        @Advice.OnMethodExit
        public static void exit(@Advice.Origin Class<?> clazz, // The class that contains the method
                                @Advice.Origin("#m") String methodName, // The method that is being executed
                                @Advice.Origin("#p") String propertyName, // The property that is being assigned
                                @Advice.Enter Object value, // The value before the assignment
                                @Advice.This Object target // The object that is being modified
        ) {
            if (value instanceof NoValueMarker) {
                return;
            }
          
                try {
                    Field field = clazz.getDeclaredField(propertyName);
                    field.setAccessible(true);
                    Object newValue = field.get(target);
                    if (shouldTrackMethod(field, methodName, propertyName) && valueChanged(value, newValue)) {
                        dirtyFields.add(field);
                    }
                } catch (NoSuchFieldException | IllegalAccessException e) {
                 
                }
            
        }

        private static boolean shouldTrackMethod(Field field, String methodName, String propertyName) {
            return field.isAnnotationPresent(UpdateOnlyOnExplicitAssignment.class) && isAccessorAdhereToNamingConvention(methodName, propertyName);
        }

        private static boolean valueChanged(Object originValue, Object newValue) {
            return !Objects.equals(originValue, newValue);
        }

        private static boolean isAccessorAdhereToNamingConvention(String methodName, String propertyName) {
            return methodName.toLowerCase().contains(propertyName.toLowerCase());
        }
    }
    
    
    ```
@raphw
Copy link
Owner

raphw commented Feb 26, 2024

You can creste your own annotation and bind it to a Field as you want. Check the javadoc for further details.

@raphw raphw self-assigned this Feb 26, 2024
@raphw raphw added the question label Feb 26, 2024
@raphw raphw added this to the 1.14.12 milestone Feb 26, 2024
@aviadsTaboola
Copy link
Author

aviadsTaboola commented Feb 26, 2024

Hi,
I need to track down fields that have changed, I already added annotation for it, I still need to find any method that might cause an assignment.
to over come this blocker I extract property name from method name myself with:

private static Optional<String> extractPropertyName(String methodName) {
            // Extract property name from method name using regex
            // Skip initial lowercase letters until reaching an uppercase letter, then take the rest of the string
            Pattern pattern = Pattern.compile("[a-z]+([A-Z].*)");
            Matcher matcher = pattern.matcher(methodName);
            if (matcher.find()) {
                String propertyName = matcher.group(1);
                // Ensure the first letter is lowercase
                return Optional.of(Character.toLowerCase(propertyName.charAt(0)) + propertyName.substring(1));
            }
            return Optional.empty();
        }```

@raphw
Copy link
Owner

raphw commented Feb 26, 2024

So you want to access if a method changes a property? I was thinking that you derive that from the method name.

@aviadsTaboola
Copy link
Author

aviadsTaboola commented Feb 28, 2024

I am deriving this from method name, I want to be able to do it by using @Advice.Origin("#p") only without adding code of my own. currently, the code is like so:

public class DirtyFieldTracker {

    public static void trackFields(Set<Field> fields) {
        Set<Class<?>> classes = getClasses(fields);
        trackClasses(classes);
    }

    public static boolean isDirty(Field field, Object instance) {
        return AssignmentInterceptors.isDirty(field, instance);
    }

    private static void trackClasses(Set<Class<?>> classes) {
        for (Class<?> clazz : classes) {
            instrumentClass(clazz);
        }
    }

    private static Set<Class<?>> getClasses(Set<Field> potentialFields) {
        Set<Class<?>> classes = new HashSet<>();
        for (Field field : potentialFields) {
            classes.add(field.getDeclaringClass());
        }
        return classes;
    }

    private static void instrumentClass(Class<?> clazz) {
        try (val unloadedType = new ByteBuddy()
                .redefine(clazz)
                .visit(Advice.to(AssignmentInterceptors.SetInterceptor.class).on(isSetMethod()))
                .visit(Advice.to(AssignmentInterceptors.WithInterceptor.class).on(isWithMethod()))
                .make()) {

            final byte[] transformed = unloadedType.getBytes();
            // Get the current instrumentation
            Instrumentation instrumentation = ByteBuddyAgent.install();
            // Redefine the class using instrumentation
            ClassDefinition classDefinition = new ClassDefinition(clazz, transformed);
            instrumentation.redefineClasses(classDefinition);
            //unloadedType.load(clazz.getClassLoader(), ClassLoadingStrategy.Default.WRAPPER));
        } catch (IOException | UnmodifiableClassException | ClassNotFoundException e) {
            throw new RuntimeException(e);
        }
    }

    private static ElementMatcher.Junction<MethodDescription> isWithMethod() {
        return isMethod()
                .and(isPublic())
                .and(not(isStatic()))
                .and(not(isAbstract()))
                .and(nameStartsWith("with"));
    }

    static class AssignmentInterceptors {

        private final static Set<TrackedField> dirtyFields = new ConcurrentHashSet<>();

        public static boolean isDirty(Field field, Object instance) {
            return dirtyFields.contains(TrackedField.of(field, instance));
        }

        static class SetInterceptor {
            @Advice.OnMethodEnter
            public static PropertyData enter(@Advice.Origin Class<?> clazz, // The class that contains the method
                                             @Advice.Origin("#m") String methodName, // The method that is being executed
                                             @Advice.This Object target // The object that is being modified
            ) {
                return derivePropertyData(clazz, methodName, target);
            }

            @Advice.OnMethodExit
            public static void exit(@Advice.This Object target, // The object that is being modified
                                    @Advice.Enter PropertyData propertyData) { // The value returned by the enter method
                if (propertyData == null) {
                    return;
                }
                addWhenDirty(propertyData, target);
            }
        }

        static class WithInterceptor {
            @Advice.OnMethodEnter
            public static PropertyData enter(@Advice.Origin Class<?> clazz, // The class that contains the method
                                             @Advice.Origin("#m") String methodName, // The method that is being executed
                                             @Advice.This Object target // The object that is being modified
            ) {
                return derivePropertyData(clazz, methodName, target);
            }

            @Advice.OnMethodExit
            public static void exit(@Advice.Enter PropertyData propertyData,
                                    @Advice.Return(typing = Assigner.Typing.DYNAMIC) Object returnValue) { // The value returned by the enter method
                if (propertyData == null) {
                    return;
                }
                addWhenDirty(propertyData, returnValue);
            }
        }

        private static PropertyData derivePropertyData(Class<?> clazz, String methodName, Object target) {
            return extractPropertyName(methodName)
                    .map(propertyName -> buildPropertyData(clazz, target, propertyName))
                    .orElse(null);
        }

        private static void addWhenDirty(PropertyData propertyData, Object instance) {
            Field field = propertyData.getField();
            if (propertyData.hasNewAssignment(instance)) {
                dirtyFields.add(TrackedField.of(field, instance));
            }
        }

        private static PropertyData buildPropertyData(Class<?> clazz, Object target, String propertyName) {
            try {
                Field field = clazz.getDeclaredField(propertyName);
                field.setAccessible(true);
                return new PropertyData(propertyName, field, field.get(target));
            } catch (NoSuchFieldException | IllegalAccessException e) {
 
            }
            return null;
        }

        private static Optional<String> extractPropertyName(String methodName) {
            // Extract property name from method name using regex
            // Skip initial lowercase letters until reaching an uppercase letter, then take the rest of the string
            Pattern pattern = Pattern.compile("[a-z]+([A-Z].*)");
            Matcher matcher = pattern.matcher(methodName);
            if (matcher.find()) {
                String propertyName = matcher.group(1);
                // Ensure the first letter is lowercase
                return Optional.of(Character.toLowerCase(propertyName.charAt(0)) + propertyName.substring(1));
            }
            return Optional.empty();
        }

        @Value(staticConstructor = "of")
        static class TrackedField {
            Field field;
            Object target;
        }

        @Value(staticConstructor = "of")
        private static class PropertyData {
            String propertyName;
            Field field;
            Object value;

            boolean hasNewAssignment(Object instance) {
                try {
                    final Object newValue = field.get(instance);
                    return !Objects.equals(value, newValue);
                } catch (IllegalAccessException e) {
                    
                    return false;
                }
            }
        }

    }
}

I also had to create seperate interceptors since @Advice.Return can not work with method that return void as Object type for setters. I would expect it to kind of cast it to Void type and not fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants