Skip to content

Commit

Permalink
Fixed ComponentDefinition
Browse files Browse the repository at this point in the history
- equals is not absolutely correct in the meaning of it's definitions,
  the desired mechanism is not equality, but if the method overrides
  another method. The order of processing of methods in the inheritance tree
  is given, that is why this work. After my refactoring it stopped working,
  because I tried to correctly implement equals (but it still wasn't symmetric).
- it should be probably replaced by different mechanism.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
  • Loading branch information
dmatej committed Sep 21, 2022
1 parent 445579b commit e192e5f
Show file tree
Hide file tree
Showing 3 changed files with 236 additions and 8 deletions.
10 changes: 10 additions & 0 deletions appserver/common/annotation-framework/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,19 @@
<version>${project.version}</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest</artifactId>
</dependency>
<dependency>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.lang.reflect.Constructor;
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.HashMap;
Expand All @@ -30,6 +29,10 @@

import org.glassfish.apf.ComponentInfo;

import static java.lang.reflect.Modifier.isPrivate;
import static java.lang.reflect.Modifier.isProtected;
import static java.lang.reflect.Modifier.isPublic;


/**
* This class represents the view of a class from annotation.
Expand Down Expand Up @@ -86,8 +89,6 @@ private void constructClassList() {
}
Class<?> parent = clazz;
while ((parent = parent.getSuperclass()) != null) {
// always check whether this class is in the class list
// for skipping annotation processing
if (!isExcludedFromAnnotationProcessing(parent)) {
classes.add(0, parent);
}
Expand Down Expand Up @@ -136,14 +137,15 @@ private void initializeMethods() {
*/
private boolean isExcludedFromAnnotationProcessing(Class<?> clazz) {
if (clazz.getPackage() == null) {
return false;
return true;
}
if (clazz.getPackage().getName().startsWith("java.lang")) {
return true;
}
return EXCLUDED_FROM_ANNOTATION_PROCESSING.contains(clazz.getCanonicalName());
}


/**
* MethodKey represents a method for the annotation's point of view.
* <p>
Expand Down Expand Up @@ -182,16 +184,19 @@ public boolean equals(Object o) {
if (!(o instanceof MethodKey)) {
return false;
}

MethodKey mk2 = (MethodKey) o;
Method m2 = mk2.m;
if (m.getName().equals(m2.getName()) && Arrays.equals(m.getParameterTypes(), m2.getParameterTypes())) {
int modifiers2 = m2.getModifiers();
boolean isSamePackage = hasSamePackage(mk2);
if (Modifier.isPrivate(m.getModifiers())) {
// need exact match
return Modifier.isPrivate(m2.getModifiers()) && isSamePackage && className.equals(mk2.className);
if (isPrivate(m.getModifiers())) {
return isPrivate(modifiers2) && isSamePackage && className.equals(mk2.className);
}
return isSamePackage && !Modifier.isPrivate(m2.getModifiers());
return isPublic(modifiers2) || isProtected(modifiers2)
|| (isPackageProtected(modifiers2) && isSamePackage);
}

return false;
}

Expand All @@ -204,5 +209,10 @@ private boolean hasSamePackage(MethodKey mk2) {
return classPackage != null && mk2.classPackage != null
&& classPackage.getName().equals(mk2.classPackage.getName());
}


private static boolean isPackageProtected(int modifiers) {
return !isPublic(modifiers) && !isProtected(modifiers) && !isPrivate(modifiers);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
/*
* Copyright (c) 2022 Eclipse Foundation and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
* version 2 with the GNU Classpath Exception, which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
*/

package org.glassfish.apf.impl;

import com.sun.enterprise.module.single.SingleModulesRegistry;

import jakarta.servlet.http.HttpServlet;

import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.stream.Stream;

import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.arrayWithSize;
import static org.hamcrest.Matchers.emptyArray;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertThrows;

/**
* Feel free to update numbers used in tests after you update some of used classes.
* <p>
* About greaterThan - be careful, reflection is quite poisonous thing; ie. JaCoCo, HK2, etc. add
* methods and fields to classes.
*
* @author David Matejcek
*/
public class ComponentDefinitionTest {

@Test
public void testPrimitives() {
ComponentDefinition component = new ComponentDefinition(int.class);
assertThat(component.getFields(), emptyArray());
assertThat(component.getConstructors(), emptyArray());
assertThat(component.getMethods(), emptyArray());
}


@Test
public void testNull() {
assertThrows(NullPointerException.class, () -> new ComponentDefinition(null));
}


@Test
public void testJavaLang() {
ComponentDefinition component = new ComponentDefinition(String.class);
assertThat(component.getFields(), emptyArray());
assertThat(component.getConstructors(), emptyArray());
assertThat(getMethodNames(component), emptyArray());
}


@Test
public void testServlet() {
ComponentDefinition component = new ComponentDefinition(HttpServlet.class);
assertThat(component.getFields(), emptyArray());
assertThat(component.getConstructors(), emptyArray());
assertThat(getMethodNames(component), emptyArray());
}


@Test
public void testComponentDefinitionTest() {
ComponentDefinition component = new ComponentDefinition(ComponentDefinitionTest.class);
assertAll(
() -> assertThat(component.getFields(), emptyArray()),
() -> assertThat(component.getConstructors(), arrayWithSize(1)),
() -> assertThat(component.getConstructors()[0].getName(), equalTo(ComponentDefinitionTest.class.getName())),
() -> assertThat(getMethodNames(component), arrayWithSize(equalTo(36)))
);
}


@Test
public void testHandlerProcessingResultImpl() {
ComponentDefinition component = new ComponentDefinition(HandlerProcessingResultImpl.class);
assertAll(
() -> assertThat(component.getFields(), arrayWithSize(2)),
() -> assertThat(component.getFields()[0].getName(), equalTo("results")),
() -> assertThat(component.getConstructors(), arrayWithSize(2)),
() -> assertThat(component.getConstructors()[0].getName(), equalTo(HandlerProcessingResultImpl.class.getName())),
() -> assertThat(getMethodNames(component), arrayWithSize(equalTo(5)))
);
}


@Test
public void testMethodKeyCollisions() {
ComponentDefinition component1 = new ComponentDefinition(GrandParent.class);
ComponentDefinition child1 = new ComponentDefinition(Parent.class);
ComponentDefinition collision2 = new ComponentDefinition(Child.class);
ComponentDefinition collision3 = new ComponentDefinition(GrandChild.class);
assertAll(
() -> assertThat(component1.getFields(), arrayWithSize(0)),
() -> assertThat(component1.getConstructors(), arrayWithSize(0)),
() -> assertThat(getMethodNames(component1),
arrayContaining("p1", "p2", "p3", "x", "x", "x", "y", "z")),
() -> assertThat(child1.getFields(), arrayWithSize(0)),
() -> assertThat(child1.getConstructors(), arrayWithSize(0)),
() -> assertThat(getMethodNames(child1),
arrayContaining("p1", "p1", "p2", "p3", "x", "x", "x", "x", "x", "y", "z")),
() -> assertThat(collision2.getFields(), arrayWithSize(0)),
() -> assertThat(collision2.getConstructors(), arrayWithSize(1)),
() -> assertThat(getMethodNames(collision2),
arrayContaining("p1", "p1", "p1", "p2", "p3", "x", "x", "x", "x", "x", "y", "z")),
() -> assertThat(collision3.getFields(), arrayWithSize(0)),
() -> assertThat(collision3.getConstructors(), arrayWithSize(2)),
() -> assertThat(getMethodNames(collision3),
arrayContaining("p1", "p1", "p1", "p2", "p3", "x", "x", "x", "x", "x", "y", "y", "z"))
);
}


@Test
public void testDifferentPackages() {
ComponentDefinition component1 = new ComponentDefinition(SingleModulesRegistry.class);
String[] methodNames = getMethodNames(component1);
assertAll(
() -> assertThat(component1.getFields(), arrayWithSize(greaterThan(11))),
() -> assertThat(component1.getConstructors(), arrayWithSize(3)),
() -> assertThat(Arrays.toString(methodNames), methodNames, arrayWithSize(equalTo(48)))
);
}


private String[] getMethodNames(ComponentDefinition component) {
return Stream.of(component.getMethods()).map(Method::getName).filter(m -> !"$jacocoInit".equals(m))
.toArray(String[]::new);
}

private static class GrandParent {
private static void p1() {
}
static void p2() {
}
public static void p3() {
}
public void x() {
}
void x(String c) {
}
private void x(Integer c) {
}
private int y() {
return 0;
}
int z() {
return 0;
}
}
private static class Parent extends GrandParent {
static void x(Object o) {
}
@Override
public void x() {
}
@Override
void x(String c) {
}
void x(Integer c) {
}
@Override
public int z() {
return super.z();
}
private void p1() {
}
}
public static class Child extends Parent {
static void x(Object x) {
}
@Override
public void x(Integer c) {
}
@Override
public void x(String c) {
}
void p1() {
}
}
public static class GrandChild extends Child {
private int y() {
return 0;
}
@Override
public void x(Integer c) {
}
}
}

0 comments on commit e192e5f

Please sign in to comment.