From aa6db8d06e59bb91630be6d7f75da195d39d3190 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 10 Apr 2026 23:44:17 +0000 Subject: [PATCH] 8382022: jpackage: enhance CompositeProxy Reviewed-by: almatvee --- .../internal/util/CompositeProxy.java | 737 +++++++----- .../internal/util/CompositeProxyTest.java | 1062 +++++++++++++++-- .../tools/jdk/jpackage/test/JUnitUtils.java | 14 + 3 files changed, 1464 insertions(+), 349 deletions(-) diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/CompositeProxy.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/CompositeProxy.java index 39a9d319468..e4257c87306 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/CompositeProxy.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/CompositeProxy.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2025, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -31,17 +31,17 @@ import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.lang.reflect.Proxy; +import java.util.ArrayList; import java.util.Arrays; +import java.util.BitSet; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; -import java.util.function.BinaryOperator; +import java.util.Optional; import java.util.function.Predicate; +import java.util.stream.Collectors; import java.util.stream.Stream; /** @@ -95,11 +95,9 @@ import java.util.stream.Stream; * } * }; * - * Sloop sloop = CompositeProxy.create(Sloop.class, new Sailboat() { - * }, withMain, withJib); + * Sloop sloop = CompositeProxy.create(Sloop.class, withMain, withJib); * - * Catboat catboat = CompositeProxy.create(Catboat.class, new Sailboat() { - * }, withMain); + * Catboat catboat = CompositeProxy.create(Catboat.class, withMain); * * sloop.trimSails(); * catboat.trimSails(); @@ -137,20 +135,60 @@ public final class CompositeProxy { * dispatching the interface method invocations to the given handlers */ public T create(Class interfaceType, Object... slices) { - return CompositeProxy.createCompositeProxy(interfaceType, conflictResolver, invokeTunnel, slices); + return CompositeProxy.createCompositeProxy( + interfaceType, + Optional.ofNullable(methodConflictResolver).orElse(JPACKAGE_METHOD_CONFLICT_RESOLVER), + Optional.ofNullable(objectConflictResolver).orElse(JPACKAGE_OBJECT_CONFLICT_RESOLVER), + invokeTunnel, + allowUnreferencedSlices, + slices); } /** * Sets the method dispatch conflict resolver for this builder. The conflict * resolver is used by composite proxy to select a method call handler from - * multiple candidates. + * several candidates. * - * @param v the conflict resolver for this builder or null if the - * default conflict resolver should be used + * @param v the method conflict resolver for this builder or null + * if the default conflict resolver should be used * @return this */ - public Builder conflictResolver(BinaryOperator v) { - conflictResolver = v; + public Builder methodConflictResolver(MethodConflictResolver v) { + methodConflictResolver = v; + return this; + } + + /** + * Sets the object dispatch conflict resolver for this builder. The conflict + * resolver is used by the composite proxy to select an object from several + * candidates. + * + * @param v the object conflict resolver for this builder or null + * if the default conflict resolver should be used + * @return this + */ + public Builder objectConflictResolver(ObjectConflictResolver v) { + objectConflictResolver = v; + return this; + } + + /** + * Configures if this builder allows unreferenced slices in the + * {@link #create(Class, Object...)}. + *

+ * By default, if the builder happens to create such a composite proxy that one + * or more slices passed in the {@link #create(Class, Object...)} method happen + * to be unreferenced, it will throw {@code IllegalArgumentException}. Passing + * true disables this throw cause. + * + * @param v true to disable throwing of + * {@code IllegalArgumentException} from + * {@link #create(Class, Object...)} if some of the passed in slices + * happen to be unreferenced and false otherwise + * @return this + */ + public Builder allowUnreferencedSlices(boolean v) { + allowUnreferencedSlices = v; return this; } @@ -168,8 +206,65 @@ public final class CompositeProxy { private Builder() {} - private BinaryOperator conflictResolver = STANDARD_CONFLICT_RESOLVER; + private MethodConflictResolver methodConflictResolver; + private ObjectConflictResolver objectConflictResolver; private InvokeTunnel invokeTunnel; + private boolean allowUnreferencedSlices; + } + + /** + * Method conflict resolver. Used when the composite proxy needs to decide if + * the default method of the interface it implements should be overridden by an + * implementing object. + */ + @FunctionalInterface + public interface MethodConflictResolver { + + /** + * Returns {@code true} if the composite proxy should override the default + * method {@code method} in {@code interfaceType} type with the corresponding + * method form the {@code obj}. + * + * @param interfaceType the interface type composite proxy instance should + * implement + * @param slices all objects passed to the calling composite proxy. The + * value is a copy of the last parameter passed in the + * {@link Builder#create(Class, Object...)} + * @param method default method in {@code interfaceType} type + * @param obj object providing a usable method with the same signature + * (the name and parameter types) as the signature of the + * {@code method} method + */ + boolean isOverrideDefault(Class interfaceType, Object[] slices, Method method, Object obj); + } + + /** + * Object conflict resolver. Used when several objects have methods that are + * candidates to implement some method in an interface and the composite proxy + * needs to choose one of these objects. + */ + @FunctionalInterface + public interface ObjectConflictResolver { + + /** + * Returns the object that should be used in a composite proxy to implement + * abstract method {@code method}. + * + * @param interfaceType the interface type composite proxy instance should + * implement + * @param slices all objects passed to the calling composite proxy. The + * value is a copy of the last parameter passed in the + * {@link Builder#create(Class, Object...)} + * @param method abstract method + * @param candidates objects with a method with the same signature (the name + * and parameter types) as the signature of the + * {@code method} method. The array is unordered, doesn't + * contain duplicates, and is a subset of the + * {@code slices} array + * @return either one of items from the {@code candidates} or {@code null} if + * can't choose one + */ + Object choose(Class interfaceType, Object[] slices, Method method, Object[] candidates); } /** @@ -263,233 +358,231 @@ public final class CompositeProxy { private CompositeProxy() { } - private static T createCompositeProxy(Class interfaceType, BinaryOperator conflictResolver, - InvokeTunnel invokeTunnel, Object... slices) { + private static T createCompositeProxy( + Class interfaceType, + MethodConflictResolver methodConflictResolver, + ObjectConflictResolver objectConflictResolver, + InvokeTunnel invokeTunnel, + boolean allowUnreferencedSlices, + Object... slices) { - validateTypeIsInterface(interfaceType); + Objects.requireNonNull(interfaceType); + Objects.requireNonNull(methodConflictResolver); + Objects.requireNonNull(objectConflictResolver); + Stream.of(slices).forEach(Objects::requireNonNull); - final var interfaces = interfaceType.getInterfaces(); - List.of(interfaces).forEach(CompositeProxy::validateTypeIsInterface); - - if (interfaces.length != slices.length) { - throw new IllegalArgumentException( - String.format("type %s must extend %d interfaces", interfaceType.getName(), slices.length)); + if (!interfaceType.isInterface()) { + throw new IllegalArgumentException(String.format("Type %s must be an interface", interfaceType.getName())); } - final Map, Object> interfaceDispatch = createInterfaceDispatch(interfaces, slices); + final var uniqueSlices = Stream.of(slices).map(IdentityWrapper::new).collect(toSet()); + + final var unreferencedSlicesBuilder = SetBuilder.>build().emptyAllowed(true); + + if (!allowUnreferencedSlices) { + unreferencedSlicesBuilder.add(uniqueSlices); + } final Map methodDispatch = getProxyableMethods(interfaceType).map(method -> { - var handler = createHandler(interfaceType, method, interfaceDispatch, conflictResolver, invokeTunnel); - if (handler != null) { - return Map.entry(method, handler); - } else { - return null; + return Map.entry(method, uniqueSlices.stream().flatMap(slice -> { + var sliceMethods = getImplementerMethods(slice.value()).filter(sliceMethod -> { + return signatureEquals(sliceMethod, method); + }).toList(); + + if (sliceMethods.size() > 1) { + throw new AssertionError(); + } + + return sliceMethods.stream().findFirst().map(sliceMethod -> { + return Map.entry(slice, sliceMethod); + }).stream(); + }).toList()); + }).flatMap(e -> { + final Method method = e.getKey(); + final List, Method>> slicesWithMethods = e.getValue(); + + final Map.Entry, Method> sliceWithMethods; + switch (slicesWithMethods.size()) { + case 0 -> { + if (!method.isDefault()) { + throw new IllegalArgumentException(String.format("None of the slices can handle %s", method)); + } else { + return Optional.ofNullable(createHandlerForDefaultMethod(method, invokeTunnel)).map(handler -> { + return Map.entry(method, handler); + }).stream(); + } + } + case 1 -> { + sliceWithMethods = slicesWithMethods.getFirst(); + } + default -> { + var candidates = slicesWithMethods.stream().map(sliceEntry -> { + return sliceEntry.getKey().value(); + }).toList(); + + var candidate = objectConflictResolver.choose( + interfaceType, Arrays.copyOf(slices, slices.length), method, candidates.toArray()); + if (candidate == null) { + throw new IllegalArgumentException(String.format( + "Ambiguous choice between %s for %s", candidates, method)); + } + + var candidateIdentity = IdentityWrapper.wrapIdentity(candidate); + + if (candidates.stream().map(IdentityWrapper::new).noneMatch(Predicate.isEqual(candidateIdentity))) { + throw new UnsupportedOperationException(); + } + + sliceWithMethods = slicesWithMethods.stream().filter(v -> { + return candidateIdentity.equals(v.getKey()); + }).findFirst().orElseThrow(); + } } - }).filter(Objects::nonNull).collect(toMap(Map.Entry::getKey, Map.Entry::getValue)); + + final var slice = sliceWithMethods.getKey().value(); + final var sliceMethod = sliceWithMethods.getValue(); + final Handler handler; + if (!method.isDefault() + || (method.equals(sliceMethod) + && getUnfilteredImplementerMethods(slice) + .map(FullMethodSignature::new) + .anyMatch(Predicate.isEqual(new FullMethodSignature(sliceMethod)))) + || ( method.getReturnType().equals(sliceMethod.getReturnType()) + && !sliceMethod.isDefault() + && methodConflictResolver.isOverrideDefault(interfaceType, Arrays.copyOf(slices, slices.length), method, slice))) { + // Use implementation from the slice if one of the statements is "true": + // - The target method is abstract (not default) + // - The target method is default and it is the same method in the slice which overrides it. + // This is a special case when default method must not be invoked via InvocationHandler.invokeDefault(). + // - The target method is default and the matching slice method has the same return type, + // is not default, and the method conflict resolver approves the use of the slice method + if (!allowUnreferencedSlices) { + unreferencedSlicesBuilder.remove(sliceWithMethods.getKey()); + } + handler = createHandlerForMethod(slice, sliceMethod, invokeTunnel); + } else { + handler = createHandlerForDefaultMethod(method, invokeTunnel); + } + + return Optional.ofNullable(handler).map(h -> { + return Map.entry(method, h); + }).stream(); + + }).collect(toMap(Map.Entry::getKey, Map.Entry::getValue)); + + if (!allowUnreferencedSlices) { + var unreferencedSlices = unreferencedSlicesBuilder.create().stream().map(IdentityWrapper::value).toList(); + if (!unreferencedSlices.isEmpty()) { + throw new IllegalArgumentException(String.format("Unreferenced slices: %s", unreferencedSlices)); + } + } @SuppressWarnings("unchecked") T proxy = (T) Proxy.newProxyInstance(interfaceType.getClassLoader(), new Class[] { interfaceType }, - new CompositeProxyInvocationHandler(methodDispatch)); + new CompositeProxyInvocationHandler(Collections.unmodifiableMap(methodDispatch))); return proxy; } - private record InterfaceDispatchBuilder(Set> interfaces, Collection slices) { - - InterfaceDispatchBuilder { - Objects.requireNonNull(interfaces); - Objects.requireNonNull(slices); - - if (interfaces.isEmpty()) { - throw new IllegalArgumentException("No interfaces to dispatch"); - } - - if (slices.isEmpty()) { - throw createInterfaceNotImplementedException(interfaces); - } - } - - InterfaceDispatchBuilder(Result result) { - this(result.unservedInterfaces(), result.unusedSlices()); - } - - Map, List> createDispatchGroups() { - return interfaces.stream().collect(toMap(x -> x, iface -> { - return slices.stream().filter(obj -> { - return Stream.of(obj.getClass().getInterfaces()).flatMap(sliceIface -> { - return unfoldInterface(sliceIface); - }).anyMatch(Predicate.isEqual(iface)); - }).toList(); - })); - } - - Result createDispatch() { - var groups = createDispatchGroups(); - - var dispatch = groups.entrySet().stream().filter(e -> { - return e.getValue().size() == 1; - }).collect(toMap(Map.Entry::getKey, e -> { - return e.getValue().getFirst(); - })); - - var unservedInterfaces = groups.entrySet().stream().filter(e -> { - return e.getValue().size() != 1; - }).map(Map.Entry::getKey).collect(toSet()); - - var usedSliceIdentities = dispatch.values().stream() - .map(IdentityWrapper::new) - .collect(toSet()); - - var unusedSliceIdentities = new HashSet<>(toIdentitySet(slices)); - unusedSliceIdentities.removeAll(usedSliceIdentities); - - return new Result(dispatch, unservedInterfaces, unusedSliceIdentities.stream().map(IdentityWrapper::value).toList()); - } - - private record Result(Map, Object> dispatch, Set> unservedInterfaces, Collection unusedSlices) { - - Result { - Objects.requireNonNull(dispatch); - Objects.requireNonNull(unservedInterfaces); - Objects.requireNonNull(unusedSlices); - - if (!Collections.disjoint(dispatch.keySet(), unservedInterfaces)) { - throw new IllegalArgumentException(); - } - - if (!Collections.disjoint(toIdentitySet(dispatch.values()), toIdentitySet(unusedSlices))) { - throw new IllegalArgumentException(); - } - } - } - - private static Collection> toIdentitySet(Collection v) { - return v.stream().map(IdentityWrapper::new).collect(toSet()); - } - } - - private static Map, Object> createInterfaceDispatch(Class[] interfaces, Object[] slices) { - - if (interfaces.length == 0) { - return Collections.emptyMap(); - } - - Map, Object> dispatch = new HashMap<>(); - - var builder = new InterfaceDispatchBuilder(Set.of(interfaces), List.of(slices)); - for (;;) { - var result = builder.createDispatch(); - if (result.dispatch().isEmpty()) { - var unserved = builder.createDispatchGroups(); - for (var e : unserved.entrySet()) { - var iface = e.getKey(); - var ifaceSlices = e.getValue(); - if (ifaceSlices.size() > 1) { - throw new IllegalArgumentException( - String.format("multiple slices %s implement %s", ifaceSlices, iface)); - } - } - - var unservedInterfaces = unserved.entrySet().stream().filter(e -> { - return e.getValue().isEmpty(); - }).map(Map.Entry::getKey).toList(); - throw createInterfaceNotImplementedException(unservedInterfaces); - } else { - dispatch.putAll(result.dispatch()); - if (result.unservedInterfaces().isEmpty()) { - break; - } - } - - builder = new InterfaceDispatchBuilder(result); - } - - return dispatch.keySet().stream().flatMap(iface -> { - return unfoldInterface(iface).map(unfoldedIface -> { - return Map.entry(unfoldedIface, dispatch.get(iface)); - }); - }).collect(toMap(Map.Entry::getKey, Map.Entry::getValue)); - } - private static Stream> unfoldInterface(Class interfaceType) { - return Stream.concat(Stream.of(interfaceType), - Stream.of(interfaceType.getInterfaces()).flatMap(CompositeProxy::unfoldInterface)); + return Stream.concat( + Stream.of(interfaceType), + Stream.of(interfaceType.getInterfaces() + ).flatMap(CompositeProxy::unfoldInterface)); } - private static IllegalArgumentException createInterfaceNotImplementedException( - Collection> missingInterfaces) { - return new IllegalArgumentException(String.format("none of the slices implement %s", missingInterfaces)); - } + private static List> getSuperclasses(Class type) { + List> superclasses = new ArrayList<>(); - private static void validateTypeIsInterface(Class type) { - if (!type.isInterface()) { - throw new IllegalArgumentException(String.format("type %s must be an interface", type.getName())); + var current = type.getSuperclass(); + + while (current != null) { + superclasses.add(current); + current = current.getSuperclass(); } + + return superclasses; } - private static Handler createHandler(Class interfaceType, Method method, Map, Object> interfaceDispatch, - BinaryOperator conflictResolver, InvokeTunnel invokeTunnel) { - - final var methodDeclaringClass = method.getDeclaringClass(); - - if (!methodDeclaringClass.equals(interfaceType)) { - // The method is declared in one of the superinterfaces. - final var slice = interfaceDispatch.get(methodDeclaringClass); - - if (isInvokeDefault(method, slice)) { - return createHandlerForDefaultMethod(method, invokeTunnel); - } else { - return createHandlerForMethod(slice, method, invokeTunnel); - } - } else if (method.isDefault()) { - return createHandlerForDefaultMethod(method, invokeTunnel); - } else { - // Find a slice handling the method. - var handler = interfaceDispatch.entrySet().stream().map(e -> { - try { - Class iface = e.getKey(); - Object slice = e.getValue(); - return createHandlerForMethod(slice, iface.getMethod(method.getName(), method.getParameterTypes()), - invokeTunnel); - } catch (NoSuchMethodException ex) { - return null; - } - }).filter(Objects::nonNull).reduce(new ConflictResolverAdapter(conflictResolver)).orElseThrow(() -> { - return new IllegalArgumentException(String.format("none of the slices can handle %s", method)); - }); - - return handler; - } + private static Stream getUnfilteredProxyableMethods(Class interfaceType) { + return unfoldInterface(interfaceType).flatMap(type -> { + return Stream.of(type.getMethods()); + }).filter(method -> { + return !Modifier.isStatic(method.getModifiers()) + && !method.isBridge(); + }); } private static Stream getProxyableMethods(Class interfaceType) { - return Stream.of(interfaceType.getMethods()).filter(method -> !Modifier.isStatic(method.getModifiers())); + return removeRedundancy(getUnfilteredProxyableMethods(interfaceType)); } - private static boolean isInvokeDefault(Method method, Object slice) { - if (!method.isDefault()) { - return false; - } + private static Stream getUnfilteredImplementerMethods(Object slice) { + var sliceType = slice.getClass(); - // The "method" is default. - // See if is overridden by any non-abstract method in the "slice". - // If it is, InvocationHandler.invokeDefault() should not be used to call it. + return Stream.of( + Stream.of(sliceType), + getSuperclasses(sliceType).stream() + ).flatMap(x -> x).flatMap(type -> { + return Stream.of(type.getMethods()); + }).filter(method -> { + return !Modifier.isStatic(method.getModifiers()) + && !method.isBridge() + && !method.isDefault() + && !Modifier.isPrivate(method.getModifiers()); + }); + } - final var sliceClass = slice.getClass(); + private static Stream getImplementerMethods(Object slice) { + var sliceType = slice.getClass(); - final var methodOverriden = Stream.of(sliceClass.getMethods()).filter(Predicate.not(Predicate.isEqual(method))) - .filter(sliceMethod -> !Modifier.isAbstract(sliceMethod.getModifiers())) - .anyMatch(sliceMethod -> signatureEquals(sliceMethod, method)); + var proxyableMethods = Stream.of( + Stream.of(sliceType), + getSuperclasses(sliceType).stream() + ).flatMap(x -> x) + .map(Class::getInterfaces) + .flatMap(Stream::of) + .flatMap(CompositeProxy::unfoldInterface) + .flatMap(CompositeProxy::getUnfilteredProxyableMethods) + .toList(); - return !methodOverriden; + var proxyableMethodSignatures = proxyableMethods.stream() + .map(FullMethodSignature::new) + .collect(toSet()); + + var methods = getUnfilteredImplementerMethods(slice).filter(method -> { + return !proxyableMethodSignatures.contains(new FullMethodSignature(method)); + }); + + return removeRedundancy(Stream.concat(methods, proxyableMethods.stream())); + } + + private static Stream removeRedundancy(Stream methods) { + var groups = methods.distinct().collect(Collectors.groupingBy(MethodSignature::new)).values(); + return groups.stream().map(group -> { + // All but a single method should be filtered out from the group. + return group.stream().reduce((a, b) -> { + var ac = a.getDeclaringClass(); + var bc = b.getDeclaringClass(); + if (ac.equals(bc)) { + // Both methods don't fit: they are declared in the same class and have the same signatures. + // That is possible only with code generation bypassing compiler checks. + throw new AssertionError(); + } else if (ac.isAssignableFrom(bc)) { + return b; + } else if (bc.isAssignableFrom(ac)) { + return a; + } else if (a.isDefault()) { + return b; + } else { + return a; + } + }).orElseThrow(); + }); } private static boolean signatureEquals(Method a, Method b) { - if (!Objects.equals(a.getName(), b.getName()) || !Arrays.equals(a.getParameterTypes(), b.getParameterTypes())) { - return false; - } - - return Objects.equals(a.getReturnType(), b.getReturnType()); + return Objects.equals(new MethodSignature(a), new MethodSignature(b)); } private record CompositeProxyInvocationHandler(Map dispatch) implements InvocationHandler { @@ -515,22 +608,14 @@ public final class CompositeProxy { return obj.getClass().getName() + '@' + Integer.toHexString(System.identityHashCode(obj)); } - private static boolean objectEquals(Object obj, Object other) { + private static boolean objectIsSame(Object obj, Object other) { return obj == other; } - private static Method getMethod(Class type, String methodName, Class...paramaterTypes) { - try { - return type.getDeclaredMethod(methodName, paramaterTypes); - } catch (NoSuchMethodException|SecurityException ex) { - throw new InternalError(ex); - } - } + private record ObjectMethodHandler(Method method) implements Handler { - static class ObjectMethodHandler extends HandlerOfMethod { - - ObjectMethodHandler(Method method) { - super(method); + ObjectMethodHandler { + Objects.requireNonNull(method); } @Override @@ -546,43 +631,47 @@ public final class CompositeProxy { } } - private static final Map OBJECT_METHOD_DISPATCH = Map.of( - getMethod(Object.class, "toString"), - new ObjectMethodHandler(getMethod(CompositeProxyInvocationHandler.class, "objectToString", Object.class)), - getMethod(Object.class, "equals", Object.class), - new ObjectMethodHandler(getMethod(CompositeProxyInvocationHandler.class, "objectEquals", Object.class, Object.class)), - getMethod(Object.class, "hashCode"), - new ObjectMethodHandler(getMethod(System.class, "identityHashCode", Object.class)) - ); + private static final Map OBJECT_METHOD_DISPATCH; + + static { + try { + OBJECT_METHOD_DISPATCH = Map.of( + Object.class.getMethod("toString"), + new ObjectMethodHandler(CompositeProxyInvocationHandler.class.getDeclaredMethod("objectToString", Object.class)), + + Object.class.getMethod("equals", Object.class), + new ObjectMethodHandler(CompositeProxyInvocationHandler.class.getDeclaredMethod("objectIsSame", Object.class, Object.class)), + + Object.class.getMethod("hashCode"), + new ObjectMethodHandler(System.class.getMethod("identityHashCode", Object.class)) + ); + } catch (NoSuchMethodException | SecurityException ex) { + throw new InternalError(ex); + } + } } - private static HandlerOfMethod createHandlerForDefaultMethod(Method method, InvokeTunnel invokeTunnel) { + private static Handler createHandlerForDefaultMethod(Method method, InvokeTunnel invokeTunnel) { + Objects.requireNonNull(method); if (invokeTunnel != null) { - return new HandlerOfMethod(method) { - @Override - public Object invoke(Object proxy, Object[] args) throws Throwable { - return invokeTunnel.invokeDefault(proxy, this.method, args); - } + return (proxy, args) -> { + return invokeTunnel.invokeDefault(proxy, method, args); }; } else { return null; } } - private static HandlerOfMethod createHandlerForMethod(Object obj, Method method, InvokeTunnel invokeTunnel) { + private static Handler createHandlerForMethod(Object obj, Method method, InvokeTunnel invokeTunnel) { + Objects.requireNonNull(obj); + Objects.requireNonNull(method); if (invokeTunnel != null) { - return new HandlerOfMethod(method) { - @Override - public Object invoke(Object proxy, Object[] args) throws Throwable { - return invokeTunnel.invoke(obj, this.method, args); - } + return (proxy, args) -> { + return invokeTunnel.invoke(obj, method, args); }; } else { - return new HandlerOfMethod(method) { - @Override - public Object invoke(Object proxy, Object[] args) throws Throwable { - return this.method.invoke(obj, args); - } + return (proxy, args) -> { + return method.invoke(obj, args); }; } } @@ -593,37 +682,141 @@ public final class CompositeProxy { Object invoke(Object proxy, Object[] args) throws Throwable; } - private abstract static class HandlerOfMethod implements Handler { - HandlerOfMethod(Method method) { - this.method = method; + private record MethodSignature(String name, List> parameterTypes) { + MethodSignature { + Objects.requireNonNull(name); + parameterTypes.forEach(Objects::requireNonNull); } - protected final Method method; + MethodSignature(Method m) { + this(m.getName(), List.of(m.getParameterTypes())); + } } - private record ConflictResolverAdapter(BinaryOperator conflictResolver) - implements BinaryOperator { + private record FullMethodSignature(MethodSignature signature, Class returnType) { + FullMethodSignature { + Objects.requireNonNull(signature); + Objects.requireNonNull(returnType); + } - @Override - public HandlerOfMethod apply(HandlerOfMethod a, HandlerOfMethod b) { - var m = conflictResolver.apply(a.method, b.method); - if (m == a.method) { - return a; - } else if (m == b.method) { - return b; - } else { - throw new UnsupportedOperationException(); + FullMethodSignature(Method m) { + this(new MethodSignature(m), m.getReturnType()); + } + } + + /** + * Returns the standard jpackage configuration if the values of + * {@code interfaceType} and {@code slices} parameters comprise such or an empty + * {@code Optional} otherwise. + *

+ * Standard jpackage configuration is: + *

    + *
  • The proxy implements an interface comprised of two direct + * superinterfaces. + *
  • The superinterfaces are distinct, i.e. they are not superinterfaces of + * each other. + *
  • Each supplied slice implements one of the superinterfaces. + *
+ * + * @param interfaceType the interface type composite proxy instance should + * implement + * @param slices all objects passed to the calling composite proxy. The + * value is a copy of the last parameter passed in the + * {@link Builder#create(Class, Object...)} + */ + static Optional, Class>> detectJPackageConfiguration(Class interfaceType, Object... slices) { + var interfaces = interfaceType.getInterfaces(); + + if (interfaces.length != 2) { + return Optional.empty(); + } + + if (interfaces[0].isAssignableFrom(interfaces[1]) || interfaces[1].isAssignableFrom(interfaces[0])) { + return Optional.empty(); + } + + var uniqueSlices = Stream.of(slices).map(IdentityWrapper::new).distinct().toList(); + if (uniqueSlices.size() != interfaces.length) { + return Optional.empty(); + } + + Map, List>> dispatch = Stream.of(interfaces).collect(toMap(x -> x, iface -> { + return uniqueSlices.stream().filter(slice -> { + return iface.isInstance(slice.value()); + }).toList(); + })); + + return dispatch.values().stream().filter(v -> { + return v.size() == 1; + }).findFirst().map(anambiguous -> { + return dispatch.entrySet().stream().collect(toMap(e -> { + var ifaceSlices = e.getValue(); + if (ifaceSlices.size() == 1) { + return ifaceSlices.getFirst(); + } else { + if (anambiguous.size() != 1) { + throw new AssertionError(); + } + return ifaceSlices.stream().filter(Predicate.isEqual(anambiguous.getFirst()).negate()).findFirst().orElseThrow(); + } + }, Map.Entry::getKey)); + }); + } + + // jpackage-specific object conflict resolver + private static final ObjectConflictResolver JPACKAGE_OBJECT_CONFLICT_RESOLVER = (interfaceType, slices, method, candidates) -> { + return detectJPackageConfiguration(interfaceType, slices).map(dispatch -> { + // In this configuration, if one slice contains matching default method and + // another contains matching implemented method, + // the latter slice is selected as a supplier of this method for the composite proxy. + + var nonDefaultImplementations = new BitSet(candidates.length); + var defaultImplementations = new BitSet(candidates.length); + for (int i = 0; i != candidates.length; i++) { + var slice = candidates[i]; + + var limitSignatures = new Predicate() { + + @Override + public boolean test(Method m) { + return limitSignatures.contains(new MethodSignature(m)); + } + + private final Collection limitSignatures = + getProxyableMethods(dispatch.get(IdentityWrapper.wrapIdentity(slice))) + .map(MethodSignature::new) + .toList(); + }; + + int cur = i; + + getImplementerMethods(slice).filter(limitSignatures).filter(sliceMethod -> { + return signatureEquals(sliceMethod, method); + }).findFirst().ifPresent(sliceMethod -> { + if (!sliceMethod.isDefault() || + getUnfilteredImplementerMethods(slice) + .filter(limitSignatures) + .map(FullMethodSignature::new) + .anyMatch(Predicate.isEqual(new FullMethodSignature(sliceMethod)))) { + nonDefaultImplementations.set(cur); + } else { + defaultImplementations.set(cur); + } + }); } - } - } - private static final BinaryOperator STANDARD_CONFLICT_RESOLVER = (a, b) -> { - if (a.isDefault() == b.isDefault()) { - throw new IllegalArgumentException(String.format("ambiguous choice between %s and %s", a, b)); - } else if (!a.isDefault()) { - return a; - } else { - return b; - } + if (nonDefaultImplementations.cardinality() == 1) { + return candidates[nonDefaultImplementations.nextSetBit(0)]; + } else if (nonDefaultImplementations.cardinality() == 0 && defaultImplementations.cardinality() == 1) { + return candidates[defaultImplementations.nextSetBit(0)]; + } else { + throw new AssertionError(); + } + }).orElse(null); + }; + + // jpackage-specific method conflict resolver + private static final MethodConflictResolver JPACKAGE_METHOD_CONFLICT_RESOLVER = (interfaceType, slices, method, obj) -> { + return false; }; } diff --git a/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/CompositeProxyTest.java b/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/CompositeProxyTest.java index 1ece83be0a6..b8b1325529d 100644 --- a/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/CompositeProxyTest.java +++ b/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/CompositeProxyTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2025, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -22,17 +22,35 @@ */ package jdk.jpackage.internal.util; +import static jdk.jpackage.internal.util.PathUtils.mapNullablePath; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertThrowsExactly; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.function.Function; +import java.util.function.UnaryOperator; +import java.util.stream.Stream; +import jdk.jpackage.internal.util.CompositeProxy.InvokeTunnel; +import jdk.jpackage.test.JUnitUtils.StringArrayConverter; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.converter.ConvertWith; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; -public class CompositeProxyTest { +class CompositeProxyTest { static interface Smalltalk { @@ -86,24 +104,24 @@ public class CompositeProxyTest { } @Test - public void testSmalltalk() { + void testSmalltalk() { var convo = CompositeProxy.create(Smalltalk.class); assertEquals("Hello", convo.sayHello()); assertEquals("Bye", convo.sayBye()); } @Test - public void testConvo() { + void testConvo() { final var otherThings = "How is your day?"; var convo = CompositeProxy.create(Convo.class, - new Smalltalk() {}, new ConvoMixin.Stub(otherThings)); + new ConvoMixin.Stub(otherThings)); assertEquals("Hello", convo.sayHello()); assertEquals("Bye", convo.sayBye()); assertEquals(otherThings, convo.sayThings()); } @Test - public void testConvoWithDuke() { + void testConvoWithDuke() { final var otherThings = "How is your day?"; var convo = CompositeProxy.create(Convo.class, new Smalltalk() { @Override @@ -116,34 +134,47 @@ public class CompositeProxyTest { assertEquals(otherThings, convo.sayThings()); } - @Test - public void testConvoWithCustomSayBye() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testConvoWithCustomSayBye(boolean allowUnreferencedSlices) { var mixin = new ConvoMixinWithOverrideSayBye.Stub("How is your day?", "See you"); - var convo = CompositeProxy.create(ConvoWithOverrideSayBye.class, new Smalltalk() {}, mixin); + var smalltalk = new Smalltalk() {}; - var expectedConvo = new ConvoWithOverrideSayBye() { - @Override - public String sayBye() { - return mixin.sayBye; - } + var proxyBuilder = CompositeProxy.build().allowUnreferencedSlices(allowUnreferencedSlices); - @Override - public String sayThings() { - return mixin.sayThings; - } - }; + if (!allowUnreferencedSlices) { + var ex = assertThrowsExactly(IllegalArgumentException.class, () -> { + proxyBuilder.create(ConvoWithOverrideSayBye.class, smalltalk, mixin); + }); - assertEquals(expectedConvo.sayHello(), convo.sayHello()); - assertEquals(expectedConvo.sayBye(), convo.sayBye()); - assertEquals(expectedConvo.sayThings(), convo.sayThings()); + assertEquals(String.format("Unreferenced slices: %s", List.of(smalltalk)), ex.getMessage()); + } else { + var convo = proxyBuilder.create(ConvoWithOverrideSayBye.class, smalltalk, mixin); + + var expectedConvo = new ConvoWithOverrideSayBye() { + @Override + public String sayBye() { + return mixin.sayBye; + } + + @Override + public String sayThings() { + return mixin.sayThings; + } + }; + + assertEquals(expectedConvo.sayHello(), convo.sayHello()); + assertEquals(expectedConvo.sayBye(), convo.sayBye()); + assertEquals(expectedConvo.sayThings(), convo.sayThings()); + } } @Test - public void testConvoWithCustomSayHelloAndSayBye() { + void testConvoWithCustomSayHelloAndSayBye() { var mixin = new ConvoMixinWithOverrideSayBye.Stub("How is your day?", "See you"); - var convo = CompositeProxy.create(ConvoWithDefaultSayHelloWithOverrideSayBye.class, new Smalltalk() {}, mixin); + var convo = CompositeProxy.create(ConvoWithDefaultSayHelloWithOverrideSayBye.class, mixin); var expectedConvo = new ConvoWithDefaultSayHelloWithOverrideSayBye() { @Override @@ -164,7 +195,7 @@ public class CompositeProxyTest { } @Test - public void testInherited() { + void testInherited() { interface Base { String doSome(); } @@ -193,7 +224,7 @@ public class CompositeProxyTest { } @Test - public void testNestedProxy() { + void testNestedProxy() { interface AddM { String m(); } @@ -231,7 +262,7 @@ public class CompositeProxyTest { } @Test - public void testComposite() { + void testComposite() { interface A { String sayHello(); String sayBye(); @@ -262,54 +293,13 @@ public class CompositeProxyTest { assertEquals("ciao,bye", proxy.talk()); } - @ParameterizedTest - @ValueSource(booleans = {true, false}) - public void testBasicObjectMethods(boolean withOverrides) { - interface A { - default void foo() {} + @Test + void testBasicObjectMethods() { + interface Foo { } - interface B { - default void bar() {} - } - - interface C extends A, B { - } - - final A aImpl; - final B bImpl; - - if (withOverrides) { - aImpl = new A() { - @Override - public String toString() { - return "theA"; - } - - @Override - public boolean equals(Object other) { - return true; - } - - @Override - public int hashCode() { - return 7; - } - }; - - bImpl = new B() { - @Override - public String toString() { - return "theB"; - } - }; - } else { - aImpl = new A() {}; - bImpl = new B() {}; - } - - var proxy = CompositeProxy.create(C.class, aImpl, bImpl); - var proxy2 = CompositeProxy.create(C.class, aImpl, bImpl); + var proxy = CompositeProxy.create(Foo.class); + var proxy2 = CompositeProxy.create(Foo.class); assertNotEquals(proxy.toString(), proxy2.toString()); assertNotEquals(proxy.hashCode(), proxy2.hashCode()); @@ -320,7 +310,925 @@ public class CompositeProxyTest { } @Test - public void testJavadocExample() { + void testAutoMethodConflictResolver() { + + interface A { + String getString(); + } + + interface B { + String getString(); + } + + interface AB extends A, B { + } + + var foo = new Object() { + public String getString() { + return "foo"; + } + }; + + var proxy = CompositeProxy.create(AB.class, foo); + assertEquals("foo", proxy.getString()); + } + + @Test + void testAutoMethodConflictResolver2() { + + interface A { + String getString(); + } + + interface B { + String getString(); + } + + interface AB extends A, B { + String getString(); + } + + var foo = new Object() { + public String getString() { + return "foo"; + } + }; + + var proxy = CompositeProxy.create(AB.class, foo); + assertEquals("foo", proxy.getString()); + } + + @Test + void testUnreferencedSlices() { + + interface A { + String getString(); + } + + interface B { + String getString(); + } + + interface AB extends A, B { + default String getString() { + throw new AssertionError(); + } + } + + var foo = new Object() { + public String getString() { + throw new AssertionError(); + } + }; + + var ex = assertThrowsExactly(IllegalArgumentException.class, () -> { + CompositeProxy.create(AB.class, foo); + }); + + assertEquals(String.format("Unreferenced slices: %s", List.of(foo)), ex.getMessage()); + } + + @Test + void testAutoMethodConflictResolver4() { + + interface A { + String getString(); + } + + interface B { + String getString(); + } + + interface AB extends A, B { + default String getString() { + return "AB"; + } + } + + var proxy = CompositeProxy.create(AB.class); + assertEquals("AB", proxy.getString()); + } + + @Test + void testAutoMethodConflictResolver4_1() { + + interface A { + String foo(); + String bar(); + } + + interface B { + String foo(); + String bar(); + } + + interface AB extends A, B { + default String foo() { + return "AB.foo"; + } + } + + var proxy = CompositeProxy.create(AB.class, new AB() { + @Override + public String bar() { + return "Obj.bar"; + } + }); + assertEquals("AB.foo", proxy.foo()); + assertEquals("Obj.bar", proxy.bar()); + } + + @Test + void testAutoMethodConflictResolver5() { + + interface A { + default String getString() { + throw new AssertionError(); + } + } + + interface B { + String getString(); + } + + interface AB extends A, B { + String getString(); + } + + var foo = new Object() { + public String getString() { + return "foo"; + } + }; + + var proxy = CompositeProxy.create(AB.class, foo); + assertEquals("foo", proxy.getString()); + } + + @Test + void testAutoMethodConflictResolver6() { + + interface A { + default String getString() { + return "A"; + } + } + + interface B { + String getString(); + } + + interface AB extends A, B { + default String getString() { + return A.super.getString() + "!"; + } + } + + var proxy = CompositeProxy.create(AB.class); + assertEquals("A!", proxy.getString()); + } + + @Test + void testAutoMethodConflictResolver7() { + + interface A { + String getString(); + } + + interface B extends A { + default String getString() { + return "B"; + } + } + + interface AB extends A, B { + default String getString() { + return B.super.getString() + "!"; + } + } + + var proxy = CompositeProxy.create(AB.class); + assertEquals("B!", proxy.getString()); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testAutoMethodConflictResolver8(boolean override) { + + interface A { + String getString(); + } + + interface B extends A { + default String getString() { + return "B"; + } + } + + interface AB extends A, B { + } + + if (override) { + var foo = new Object() { + public String getString() { + return "foo"; + } + }; + + var proxy = CompositeProxy.build().methodConflictResolver((_, _, _, _) -> { + return true; + }).create(AB.class, foo); + assertEquals("foo", proxy.getString()); + } else { + var proxy = CompositeProxy.create(AB.class); + assertEquals("B", proxy.getString()); + } + } + + @Test + void testAutoMethodConflictResolver9() { + + interface A { + String getString(); + } + + interface B extends A { + default String getString() { + throw new AssertionError(); + } + } + + var foo = new Object() { + public String getString() { + return "foo"; + } + }; + + interface AB extends A, B { + String getString(); + } + + var ab = CompositeProxy.create(AB.class, foo); + assertEquals("foo", ab.getString()); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testAutoMethodConflictResolver10(boolean override) { + + interface A { + String getString(); + } + + interface B extends A { + default String getString() { + return "B"; + } + } + + interface AB extends A, B { + String getString(); + } + + if (override) { + var foo = new B() { + @Override + public String getString() { + return B.super.getString() + "!"; + } + }; + + var proxy = CompositeProxy.create(AB.class, foo); + assertEquals("B!", proxy.getString()); + } else { + var proxy = CompositeProxy.create(AB.class, new B() {}); + assertEquals("B", proxy.getString()); + } + } + + @Test + void testAutoMethodConflictResolver11() { + + interface A { + String getString(); + } + + class Foo implements A { + @Override + public String getString() { + throw new AssertionError(); + } + } + + class Bar extends Foo { + @Override + public String getString() { + throw new AssertionError(); + } + } + + class Buz extends Bar { + @Override + public String getString() { + return "buz"; + } + } + + var proxy = CompositeProxy.create(A.class, new Buz()); + assertEquals("buz", proxy.getString()); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testAutoMethodConflictResolver12(boolean override) { + + interface A { + String getString(); + } + + interface B { + default String getString() { + return "foo"; + } + } + + if (override) { + class BImpl implements B { + @Override + public String getString() { + return "bar"; + } + } + + var proxy = CompositeProxy.create(A.class, new BImpl() {}); + assertEquals("bar", proxy.getString()); + } else { + class BImpl implements B { + } + + var proxy = CompositeProxy.create(A.class, new BImpl() {}); + assertEquals("foo", proxy.getString()); + } + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testAutoMethodConflictResolver13(boolean override) { + + interface A { + String getString(); + } + + interface Foo { + default String getString() { + return "foo"; + } + } + + if (override) { + class B { + public String getString() { + return "B"; + } + } + + class C extends B implements Foo { + } + + for (var slice : List.of(new C(), new C() {})) { + var proxy = CompositeProxy.create(A.class, slice); + assertEquals("B", proxy.getString()); + } + + var proxy = CompositeProxy.create(A.class, new C() { + @Override + public String getString() { + return "C"; + } + }); + assertEquals("C", proxy.getString()); + } else { + class B { + } + + class C extends B implements Foo { + } + + for (var slice : List.of(new C(), new C() {})) { + var proxy = CompositeProxy.create(A.class, slice); + assertEquals("foo", proxy.getString()); + } + } + } + + @Test + void testAutoMethodConflictResolver14() { + + interface Launcher { + + String name(); + + Map extraAppImageFileData(); + + record Stub(String name, Map extraAppImageFileData) implements Launcher {} + } + + interface WinLauncherMixin { + + boolean shortcut(); + + record Stub(boolean shortcut) implements WinLauncherMixin {} + } + + interface WinLauncher extends Launcher, WinLauncherMixin { + + default Map extraAppImageFileData() { + return Map.of("shortcut", Boolean.toString(shortcut())); + } + } + + var proxy = CompositeProxy.create(WinLauncher.class, new Launcher.Stub("foo", Map.of()), new WinLauncherMixin.Stub(true)); + + assertEquals("foo", proxy.name()); + assertEquals(Map.of("shortcut", "true"), proxy.extraAppImageFileData()); + } + + @ParameterizedTest + @CsvSource({ + "a,b", + "b,a", + }) + void testObjectConflictResolver(String fooResolve, String barResolve) { + + interface I { + String foo(); + String bar(); + } + + var a = new I() { + @Override + public String foo() { + return "a-foo"; + } + + @Override + public String bar() { + return "a-bar"; + } + }; + + var b = new Object() { + public String foo() { + return "b-foo"; + } + + public String bar() { + return "b-bar"; + } + }; + + Function resolver = tag -> { + return switch (tag) { + case "a" -> a; + case "b" -> b; + default -> { + throw new AssertionError(); + } + }; + }; + + var proxy = CompositeProxy.build().objectConflictResolver((_, _, method, _) -> { + return switch (method.getName()) { + case "foo" -> resolver.apply(fooResolve); + case "bar" -> resolver.apply(barResolve); + default -> { + throw new AssertionError(); + } + }; + }).create(I.class, a, b); + + assertEquals(fooResolve + "-foo", proxy.foo()); + assertEquals(barResolve + "-bar", proxy.bar()); + } + + @Test + void testObjectConflictResolverInvalid() { + + interface I { + String foo(); + } + + var a = new I() { + @Override + public String foo() { + throw new AssertionError(); + } + }; + + var b = new Object() { + public String foo() { + throw new AssertionError(); + } + }; + + assertThrowsExactly(UnsupportedOperationException.class, () -> { + CompositeProxy.build().objectConflictResolver((_, _, _, _) -> { + return new Object(); + }).create(I.class, a, b); + }); + } + + @ParameterizedTest + @ValueSource( strings = { + "no-foo", + "private-foo", + "protected-foo", + "package-foo", + "static-foo", + "static-foo,private-foo,no-foo", + }) + void testMissingImplementer(@ConvertWith(StringArrayConverter.class) String[] slicesSpec) throws NoSuchMethodException, SecurityException { + + interface A { + void foo(); + } + + var slices = Stream.of(slicesSpec).map(slice -> { + return switch (slice) { + case "no-foo" -> new Object(); + case "private-foo" -> new Object() { + private void foo() { + throw new AssertionError(); + } + }; + case "protected-foo" -> new Object() { + protected void foo() { + throw new AssertionError(); + } + }; + case "package-foo" -> new Object() { + void foo() { + throw new AssertionError(); + } + }; + case "static-foo" -> new Object() { + public static void foo() { + throw new AssertionError(); + } + }; + default -> { throw new AssertionError(); } + }; + }).toList(); + + var ex = assertThrowsExactly(IllegalArgumentException.class, () -> { + CompositeProxy.create(A.class, slices.toArray()); + }); + + assertEquals(String.format("None of the slices can handle %s", A.class.getMethod("foo")), ex.getMessage()); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testUnusedSlice(boolean all) { + + interface A { + default void foo() { + throw new AssertionError(); + } + } + + A a = new A() {}; + var obj = new Object(); + + if (all) { + var messages = Set.of( + String.format("Unreferenced slices: %s", List.of(a, obj)), + String.format("Unreferenced slices: %s", List.of(obj, a)) + ); + + var ex = assertThrowsExactly(IllegalArgumentException.class, () -> { + CompositeProxy.create(A.class, a, obj); + }); + + assertTrue(messages.contains(ex.getMessage())); + } else { + interface B extends A { + void foo(); + } + + var ex = assertThrowsExactly(IllegalArgumentException.class, () -> { + CompositeProxy.create(B.class, a, obj); + }); + + assertEquals(String.format("Unreferenced slices: %s", List.of(obj)), ex.getMessage()); + } + } + + @ParameterizedTest + @CsvSource({ + "'a,b,a',false", + "'a,b,a',true", + "'a,b',true", + "'b,a',true", + "'a,b',false", + "'b,a',false", + }) + void testAmbiguousImplementers( + @ConvertWith(StringArrayConverter.class) String[] slicesSpec, + boolean withObjectConflictResolver) throws NoSuchMethodException, SecurityException { + + interface A { + String foo(); + String bar(); + } + + var a = new Object() { + public String foo() { + return "a-foo"; + } + public String bar() { + throw new AssertionError(); + } + }; + + var b = new Object() { + public String bar() { + return "b-bar"; + } + }; + + var ambiguousMethod = A.class.getMethod("bar"); + + var slices = Stream.of(slicesSpec).map(slice -> { + return switch (slice) { + case "a" -> a; + case "b" -> b; + default -> { throw new AssertionError(); } + }; + }).toArray(); + + if (withObjectConflictResolver) { + var proxy = CompositeProxy.build().objectConflictResolver((_, _, _, _) -> { + return b; + }).create(A.class, slices); + + assertEquals("a-foo", proxy.foo()); + assertEquals("b-bar", proxy.bar()); + } else { + var ex = assertThrowsExactly(IllegalArgumentException.class, () -> { + CompositeProxy.create(A.class, slices); + }); + + var messages = Set.of( + String.format("Ambiguous choice between %s for %s", List.of(a, b), ambiguousMethod), + String.format("Ambiguous choice between %s for %s", List.of(b, a), ambiguousMethod) + ); + + assertTrue(messages.contains(ex.getMessage())); + } + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testDifferentReturnTypes(boolean compatible) { + + interface A { + Number foo(); + } + + Object obj; + if (compatible) { + obj = new Object() { + public Integer foo() { + return 123; + } + }; + } else { + obj = new Object() { + public String foo() { + return "123"; + } + }; + } + + var proxy = CompositeProxy.create(A.class, obj); + + if (compatible) { + assertEquals(123, proxy.foo()); + } else { + assertThrows(ClassCastException.class, proxy::foo); + } + } + + @Test + void testCovariantReturnType() { + + interface A { + Number foo(); + } + + interface Mixin { + String bar(); + } + + interface AWithMixin extends A, Mixin { + Integer foo(); + } + + var proxy = CompositeProxy.create(AWithMixin.class, new A() { + @Override + public Number foo() { + return 123; + } + }, new Mixin() { + @Override + public String bar() { + return "bar"; + } + }); + + assertEquals(123, proxy.foo()); + assertEquals("bar", proxy.bar()); + } + + @Test + void testNotInterface() { + var ex = assertThrowsExactly(IllegalArgumentException.class, () -> { + CompositeProxy.create(Integer.class); + }); + + assertEquals(String.format("Type %s must be an interface", Integer.class.getName()), ex.getMessage()); + } + + @Test + void testExcessiveInterfaces() { + + interface Launcher { + String name(); + + default String executableResource() { + return "jpackageapplauncher"; + } + + record Stub(String name) implements Launcher { + } + } + + interface WinLauncherMixin { + String version(); + + record Stub(String version) implements WinLauncherMixin { + } + } + + interface WinLauncher extends Launcher, WinLauncherMixin { + + default String executableResource() { + return "jpackageapplauncher.exe"; + } + } + + var winLauncher = CompositeProxy.create(WinLauncher.class, new Launcher.Stub("foo"), new WinLauncherMixin.Stub("1.0")); + + var winLauncher2 = CompositeProxy.create(WinLauncher.class, new Launcher.Stub("bar"), winLauncher); + + assertEquals("foo", winLauncher.name()); + assertEquals("1.0", winLauncher.version()); + assertEquals("jpackageapplauncher.exe", winLauncher.executableResource()); + + assertEquals("bar", winLauncher2.name()); + assertEquals("1.0", winLauncher2.version()); + assertEquals("jpackageapplauncher.exe", winLauncher2.executableResource()); + } + + @Test + void testInvokeTunnel() { + + interface A { + default String foo() { + return "foo"; + } + String bar(); + } + + var obj = new Object() { + public String bar() { + return "bar"; + } + }; + + Slot invokeCalled = Slot.createEmpty(); + invokeCalled.set(false); + + Slot invokeDefaultCalled = Slot.createEmpty(); + invokeDefaultCalled.set(false); + + var proxy = CompositeProxy.build().invokeTunnel(new InvokeTunnel() { + + @Override + public Object invoke(Object obj, Method method, Object[] args) throws Throwable { + invokeCalled.set(true); + return method.invoke(obj, args); + } + + @Override + public Object invokeDefault(Object proxy, Method method, Object[] args) throws Throwable { + invokeDefaultCalled.set(true); + return InvocationHandler.invokeDefault(proxy, method, args); + } + + }).create(A.class, obj); + + assertFalse(invokeCalled.get()); + assertFalse(invokeDefaultCalled.get()); + assertEquals("foo", proxy.foo()); + assertFalse(invokeCalled.get()); + assertTrue(invokeDefaultCalled.get()); + + invokeDefaultCalled.set(false); + assertEquals("bar", proxy.bar()); + assertTrue(invokeCalled.get()); + assertFalse(invokeDefaultCalled.get()); + } + + @Test + void testDefaultOverride() { + + interface AppImageLayout { + + Path runtimeDirectory(); + + Path rootDirectory(); + + default boolean isResolved() { + return !rootDirectory().equals(Path.of("")); + } + + default AppImageLayout unresolve() { + if (isResolved()) { + final var root = rootDirectory(); + return map(root::relativize); + } else { + return this; + } + } + + AppImageLayout map(UnaryOperator mapper); + + record Stub(Path rootDirectory, Path runtimeDirectory) implements AppImageLayout { + + public Stub { + Objects.requireNonNull(rootDirectory); + } + + public Stub(Path runtimeDirectory) { + this(Path.of(""), runtimeDirectory); + } + + @Override + public AppImageLayout map(UnaryOperator mapper) { + return new Stub(mapNullablePath(mapper, rootDirectory), mapNullablePath(mapper, runtimeDirectory)); + } + } + } + + interface ApplicationLayoutMixin { + + Path appDirectory(); + + record Stub(Path appDirectory) implements ApplicationLayoutMixin { + } + } + + interface ApplicationLayout extends AppImageLayout, ApplicationLayoutMixin { + + @Override + default ApplicationLayout unresolve() { + return (ApplicationLayout)AppImageLayout.super.unresolve(); + } + + @Override + default ApplicationLayout map(UnaryOperator mapper) { + return CompositeProxy.create(ApplicationLayout.class, + new AppImageLayout.Stub(rootDirectory(), runtimeDirectory()).map(mapper), + new ApplicationLayoutMixin.Stub(mapper.apply(appDirectory()))); + } + } + + var proxy = CompositeProxy.create(ApplicationLayout.class, + new AppImageLayout.Stub(Path.of(""), Path.of("runtime")), + new ApplicationLayoutMixin.Stub(Path.of("app"))); + + assertSame(proxy, proxy.unresolve()); + + var mapped = proxy.map(Path.of("a")::resolve); + assertEquals(Path.of("a"), mapped.rootDirectory()); + assertEquals(Path.of("a/runtime"), mapped.runtimeDirectory()); + assertEquals(Path.of("a/app"), mapped.appDirectory()); + } + + @Test + void testJavadocExample() { interface Sailboat { default void trimSails() {} } @@ -364,9 +1272,9 @@ public class CompositeProxyTest { } }; - Sloop sloop = CompositeProxy.create(Sloop.class, new Sailboat() {}, withMain, withJib); + Sloop sloop = CompositeProxy.create(Sloop.class, withMain, withJib); - Catboat catboat = CompositeProxy.create(Catboat.class, new Sailboat() {}, withMain); + Catboat catboat = CompositeProxy.create(Catboat.class, withMain); sloop.trimSails(); catboat.trimSails(); diff --git a/test/jdk/tools/jpackage/junit/tools/jdk/jpackage/test/JUnitUtils.java b/test/jdk/tools/jpackage/junit/tools/jdk/jpackage/test/JUnitUtils.java index 530a0d5cb1f..97041ea1a0e 100644 --- a/test/jdk/tools/jpackage/junit/tools/jdk/jpackage/test/JUnitUtils.java +++ b/test/jdk/tools/jpackage/junit/tools/jdk/jpackage/test/JUnitUtils.java @@ -27,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import java.util.Map; import java.util.Objects; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.converter.SimpleArgumentConverter; public final class JUnitUtils { @@ -121,6 +122,19 @@ public final class JUnitUtils { } + public static class StringArrayConverter extends SimpleArgumentConverter { + + @Override + protected Object convert(Object source, Class targetType) { + if (source instanceof String && String[].class.isAssignableFrom(targetType)) { + return ((String) source).split("\\s*,\\s*"); + } else { + throw new IllegalArgumentException(); + } + } + } + + private static final class ExceptionCauseRemover extends Exception { ExceptionCauseRemover(Exception ex) {