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

FISH-391 OutOfMemoryError exception caused by OpenApi refactor #4865

Merged
merged 21 commits into from
Sep 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5b1841d
FISH-391 Discard HK2 Types after OpenAPI document creation
jGauravGupta Aug 31, 2020
39c2fd5
FISH-391 store AnnotationInfo in OpenAPIContext
jGauravGupta Sep 1, 2020
25ad9e1
FISH-391 polish
jGauravGupta Sep 1, 2020
be2be17
FISH-391 synchronize buildDocument function
jGauravGupta Sep 1, 2020
bbbc09c
FISH-391 set application classloader to current context
jGauravGupta Sep 1, 2020
a3955f5
Revert "FISH-31 Upgrade Grizzly to 2.4.4.payara-p3"
MattGill98 Sep 2, 2020
5ffa918
FISH-391 Lazily initialize openapi document
jGauravGupta Sep 3, 2020
b3831ce
FISH-391 Remove types from transient metadata of application
jGauravGupta Sep 4, 2020
a9a0fee
FISH-391 Downgrade grizzly to 2.4.4.payara-p2
jGauravGupta Sep 4, 2020
eece586
Merge pull request #4871 from MattGill98/tck-fix
Pandrex247 Sep 4, 2020
ee3758c
Disable ServerLogTest until Grizzly upgrade
MattGill98 Sep 4, 2020
ee70114
Merge pull request #4872 from MattGill98/serverlogtest-disable
MarkWareham Sep 4, 2020
6c7908c
FISH-391 Discard HK2 Types after OpenAPI document creation
jGauravGupta Aug 31, 2020
ea20059
FISH-391 store AnnotationInfo in OpenAPIContext
jGauravGupta Sep 1, 2020
bd08789
FISH-391 polish
jGauravGupta Sep 1, 2020
c650412
FISH-391 synchronize buildDocument function
jGauravGupta Sep 1, 2020
51ee9bb
FISH-391 set application classloader to current context
jGauravGupta Sep 1, 2020
7bf03cf
FISH-391 Lazily initialize openapi document
jGauravGupta Sep 3, 2020
66e9aa9
FISH-391 Remove types from transient metadata of application
jGauravGupta Sep 4, 2020
c4fa3c9
Merge branch 'FISH-391' of https://github.com/jgauravgupta/Payara int…
jGauravGupta Sep 4, 2020
5b961c0
FISH-391 Revert cleaning of ApplicationInfo transient metadata
jGauravGupta Sep 4, 2020
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
Expand Up @@ -39,8 +39,10 @@
*/
package fish.payara.microprofile.openapi.api.visitor;

import fish.payara.microprofile.openapi.impl.visitor.AnnotationInfo;
import org.eclipse.microprofile.openapi.models.OpenAPI;
import org.eclipse.microprofile.openapi.models.Operation;
import org.glassfish.hk2.classmodel.reflect.ExtensibleType;
import org.glassfish.hk2.classmodel.reflect.Type;

/**
Expand Down Expand Up @@ -90,4 +92,12 @@ public interface ApiContext {
* @return the application class loader
*/
ClassLoader getApplicationClassLoader();

/**
*
* @param type
* @return the aggregated annotation info of type
*/
public AnnotationInfo getAnnotationInfo(ExtensibleType<? extends ExtensibleType> type);

}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
*/
package fish.payara.microprofile.openapi.impl;

import com.sun.enterprise.v3.server.ApplicationLifecycle;
import com.sun.enterprise.v3.services.impl.GrizzlyService;
import fish.payara.microprofile.openapi.api.OpenAPIBuildException;
import fish.payara.microprofile.openapi.impl.admin.OpenApiServiceConfiguration;
Expand Down Expand Up @@ -92,7 +93,10 @@
import static java.util.logging.Level.WARNING;
import static java.util.stream.Collectors.toSet;
import org.glassfish.api.deployment.archive.ReadableArchive;
import org.glassfish.hk2.classmodel.reflect.Parser;
import org.glassfish.hk2.classmodel.reflect.Type;
import org.glassfish.hk2.classmodel.reflect.Types;
import org.glassfish.internal.deployment.analysis.StructuredDeploymentTracing;

@Service(name = "microprofile-openapi-service")
@RunLevel(StartupRunLevel.VAL)
Expand All @@ -116,6 +120,9 @@ public class OpenApiService implements PostConstruct, PreDestroy, EventListener,
@Inject
private ServiceLocator habitat;

@Inject
private ApplicationLifecycle applicationLifecycle;

@Override
public void postConstruct() {
mappings = new ConcurrentLinkedDeque<>();
Expand Down Expand Up @@ -173,7 +180,8 @@ public void event(Event<?> event) {
// Create all the relevant resources
if (isValidApp(appInfo)) {
// Store the application mapping in the list
mappings.add(new OpenApiMapping(appInfo));
OpenApiMapping mapping = new OpenApiMapping(appInfo);
mappings.add(mapping);
allDocuments = null;
}
} else if (event.is(Deployment.APPLICATION_UNLOADED)) {
Expand All @@ -192,8 +200,9 @@ public void event(Event<?> event) {
* @return the document If multiple application deployed then merge all the
* documents. Creates one if it hasn't already been created.
* @throws OpenAPIBuildException if creating the document failed.
* @throws java.io.IOException if source archive not accessible
*/
public OpenAPI getDocument() throws OpenAPIBuildException {
public OpenAPI getDocument() throws OpenAPIBuildException, IOException {
if (mappings.isEmpty() || !isEnabled()) {
return null;
}
Expand Down Expand Up @@ -260,58 +269,74 @@ ApplicationInfo getAppInfo() {
return appInfo;
}

private synchronized OpenAPI getDocument() throws OpenAPIBuildException {
private OpenAPI getDocument() throws OpenAPIBuildException {
return document;
}

private OpenAPI buildDocument() throws OpenAPIBuildException {
document = new OpenAPIImpl();
private synchronized OpenAPI buildDocument() throws OpenAPIBuildException, IOException {
if (this.document != null) {
return this.document;
}

Parser parser = applicationLifecycle.getDeployableParser(
appInfo.getSource(),
true,
true,
StructuredDeploymentTracing.create(appInfo.getName()),
LOGGER
);
Types types = parser.getContext().getTypes();

OpenAPI doc = new OpenAPIImpl();
try {
String contextRoot = getContextRoot(appInfo);
List<URL> baseURLs = getServerURL(contextRoot);

document = new ModelReaderProcessor().process(document, appConfig);
document = new FileProcessor(appInfo.getAppClassLoader()).process(document, appConfig);
document = new ApplicationProcessor(
appInfo.getTypes(),
filterTypes(appInfo, appConfig),
doc = new ModelReaderProcessor().process(doc, appConfig);
doc = new FileProcessor(appInfo.getAppClassLoader()).process(doc, appConfig);
doc = new ApplicationProcessor(
types,
filterTypes(appInfo, appConfig, types),
appInfo.getAppClassLoader()
).process(document, appConfig);
document = new BaseProcessor(baseURLs).process(document, appConfig);
document = new FilterProcessor().process(document, appConfig);
).process(doc, appConfig);
doc = new BaseProcessor(baseURLs).process(doc, appConfig);
doc = new FilterProcessor().process(doc, appConfig);
} catch (Throwable t) {
throw new OpenAPIBuildException(t);
} finally {
this.document = doc;
}

LOGGER.info("OpenAPI document created.");
return document;
return this.document;
}

}

/**
* @return a list of all classes in the archive.
*/
private Set<Type> filterTypes(ApplicationInfo appInfo, OpenApiConfiguration config) {
private Set<Type> filterTypes(ApplicationInfo appInfo, OpenApiConfiguration config, Types hk2Types) {
ReadableArchive archive = appInfo.getSource();
Set<Type> types = new HashSet<>(filterLibTypes(appInfo, config, archive));
Set<Type> types = new HashSet<>(filterLibTypes(config, hk2Types, archive));
types.addAll(
Collections.list(archive.entries()).stream()
// Only use the classes
.filter(clazz -> clazz.endsWith(".class"))
// Remove the WEB-INF/classes and return the proper class name format
.map(clazz -> clazz.replaceAll("WEB-INF/classes/", "").replace("/", ".").replace(".class", ""))
// Fetch class type
.map(clazz -> appInfo.getTypes().getBy(clazz))
.map(clazz -> hk2Types.getBy(clazz))
// Don't return null classes
.filter(Objects::nonNull)
.collect(toSet())
);
return config == null ? types : config.getValidClasses(types);
}

private Set<Type> filterLibTypes(ApplicationInfo appInfo, OpenApiConfiguration config, ReadableArchive archive) {
private Set<Type> filterLibTypes(
OpenApiConfiguration config,
Types hk2Types,
ReadableArchive archive) {
Set<Type> types = new HashSet<>();
if (config != null && config.getScanLib()) {
Enumeration<String> subArchiveItr = archive.entries();
Expand All @@ -328,7 +353,7 @@ private Set<Type> filterLibTypes(ApplicationInfo appInfo, OpenApiConfiguration c
// return the proper class name format
.map(clazz -> clazz.replace("/", ".").replace(".class", ""))
// Fetch class type
.map(clazz -> appInfo.getTypes().getBy(clazz))
.map(clazz -> hk2Types.getBy(clazz))
// Don't return null classes
.filter(Objects::nonNull)
.collect(toSet())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import fish.payara.microprofile.openapi.api.visitor.ApiContext;
import fish.payara.microprofile.openapi.impl.model.ExtensibleImpl;
import fish.payara.microprofile.openapi.impl.model.ExternalDocumentationImpl;
import fish.payara.microprofile.openapi.impl.model.util.AnnotationInfo;
import fish.payara.microprofile.openapi.impl.visitor.AnnotationInfo;
import fish.payara.microprofile.openapi.impl.model.util.ModelUtils;
import static fish.payara.microprofile.openapi.impl.model.util.ModelUtils.applyReference;
import static fish.payara.microprofile.openapi.impl.model.util.ModelUtils.mergeProperty;
Expand Down Expand Up @@ -833,7 +833,7 @@ public static void merge(Schema from, Schema to,
String schemaName = null;
if (type instanceof ExtensibleType) {
ExtensibleType implementationType = (ExtensibleType) type;
AnnotationInfo annotationInfo = AnnotationInfo.valueOf(implementationType);
AnnotationInfo annotationInfo = context.getAnnotationInfo(implementationType);
AnnotationModel annotation = annotationInfo.getAnnotation(org.eclipse.microprofile.openapi.annotations.media.Schema.class);
// Get the schema name
if (annotation != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@
*/
package fish.payara.microprofile.openapi.impl.model.util;

import fish.payara.microprofile.openapi.impl.visitor.AnnotationInfo;
import fish.payara.microprofile.openapi.api.visitor.ApiContext;
import fish.payara.microprofile.openapi.impl.model.OperationImpl;
import fish.payara.microprofile.openapi.impl.visitor.OpenApiContext;
import java.lang.annotation.Annotation;
import java.lang.reflect.Array;
import java.lang.reflect.Field;
Expand Down Expand Up @@ -128,12 +130,13 @@ public static String normaliseUrl(String path) {
}

/**
* @param context
* @param method the method to analyse.
* @return the {@link HttpMethod} applied to this method, or null if there is
* none.
*/
public static HttpMethod getHttpMethod(MethodModel method) {
AnnotationInfo annotations = AnnotationInfo.valueOf(method.getDeclaringType());
public static HttpMethod getHttpMethod(OpenApiContext context, MethodModel method) {
AnnotationInfo annotations = context.getAnnotationInfo(method.getDeclaringType());
if (annotations.isAnnotationPresent(GET.class, method)) {
return HttpMethod.GET;
}
Expand Down Expand Up @@ -227,10 +230,10 @@ public static Operation getOrCreateOperation(PathItem pathItem, HttpMethod httpM
return operation;
}

public static Operation findOperation(OpenAPI api, MethodModel method, String path) {
public static Operation findOperation(OpenApiContext context, OpenAPI api, MethodModel method, String path) {
Operation foundOperation = null;
try {
return api.getPaths().getPathItem(path).getOperations().get(getHttpMethod(method));
return api.getPaths().getPathItem(path).getOperations().get(getHttpMethod(context, method));
} catch (NullPointerException ex) {
// Operation not found
}
Expand Down Expand Up @@ -360,8 +363,8 @@ public static SchemaType getParentSchemaType(SchemaType type1, SchemaType type2)
return type1;
}

public static boolean isRequestBody(org.glassfish.hk2.classmodel.reflect.Parameter parameter) {
AnnotationInfo annotations = AnnotationInfo.valueOf(parameter.getMethod().getDeclaringType());
public static boolean isRequestBody(ApiContext context, org.glassfish.hk2.classmodel.reflect.Parameter parameter) {
AnnotationInfo annotations = context.getAnnotationInfo(parameter.getMethod().getDeclaringType());
if (annotations.getAnnotationCount(parameter) == 0) {
return true;
}
Expand All @@ -372,8 +375,8 @@ public static boolean isRequestBody(org.glassfish.hk2.classmodel.reflect.Paramet
);
}

public static In getParameterType(org.glassfish.hk2.classmodel.reflect.Parameter parameter) {
AnnotationInfo annotations = AnnotationInfo.valueOf(parameter.getMethod().getDeclaringType());
public static In getParameterType(ApiContext context, org.glassfish.hk2.classmodel.reflect.Parameter parameter) {
AnnotationInfo annotations = context.getAnnotationInfo(parameter.getMethod().getDeclaringType());
if (annotations.isAnnotationPresent(PathParam.class, parameter)) {
return In.PATH;
}
Expand All @@ -389,8 +392,8 @@ public static In getParameterType(org.glassfish.hk2.classmodel.reflect.Parameter
return null;
}

public static String getParameterName(org.glassfish.hk2.classmodel.reflect.Parameter parameter) {
AnnotationInfo annotations = AnnotationInfo.valueOf(parameter.getMethod().getDeclaringType());
public static String getParameterName(ApiContext context, org.glassfish.hk2.classmodel.reflect.Parameter parameter) {
AnnotationInfo annotations = context.getAnnotationInfo(parameter.getMethod().getDeclaringType());
if (annotations.isAnnotationPresent(PathParam.class, parameter)) {
return annotations.getAnnotationValue(PathParam.class, parameter);
} else if (annotations.isAnnotationPresent(QueryParam.class, parameter)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
import fish.payara.microprofile.openapi.impl.model.security.SecuritySchemeImpl;
import fish.payara.microprofile.openapi.impl.model.servers.ServerImpl;
import fish.payara.microprofile.openapi.impl.model.tags.TagImpl;
import fish.payara.microprofile.openapi.impl.model.util.AnnotationInfo;
import fish.payara.microprofile.openapi.impl.visitor.AnnotationInfo;
import fish.payara.microprofile.openapi.impl.model.util.ModelUtils;
import fish.payara.microprofile.openapi.impl.visitor.OpenApiWalker;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -555,7 +555,7 @@ private Schema visitSchemaClass(
if (superClass != null) {

// Get the parent schema annotation
AnnotationModel parentSchemAnnotation = AnnotationInfo.valueOf(superClass)
AnnotationModel parentSchemAnnotation = context.getAnnotationInfo(superClass)
.getAnnotation(org.eclipse.microprofile.openapi.annotations.media.Schema.class);

ParameterizedInterfaceModel parameterizedInterface = clazz.getParameterizedInterface(superClass);
Expand Down Expand Up @@ -589,7 +589,7 @@ public void visitSchemaField(AnnotationModel schemaAnnotation, FieldModel field,

// Get the parent schema object name
String parentName = null;
AnnotationModel classSchemaAnnotation = AnnotationInfo.valueOf(field.getDeclaringType())
AnnotationModel classSchemaAnnotation = context.getAnnotationInfo(field.getDeclaringType())
.getAnnotation(org.eclipse.microprofile.openapi.annotations.media.Schema.class);
if (classSchemaAnnotation != null) {
parentName = classSchemaAnnotation.getValue("name", String.class);
Expand Down Expand Up @@ -617,7 +617,7 @@ private static void visitSchemaParameter(AnnotationModel schemaAnnotation, org.g
return;
}
// Check if it's a request body
if (ModelUtils.isRequestBody(parameter)) {
if (ModelUtils.isRequestBody(context, parameter)) {
if (context.getWorkingOperation().getRequestBody() == null) {
context.getWorkingOperation().setRequestBody(new RequestBodyImpl());
}
Expand All @@ -629,10 +629,10 @@ private static void visitSchemaParameter(AnnotationModel schemaAnnotation, org.g
if (schema.getRef() != null && !schema.getRef().isEmpty()) {
mediaType.setSchema(new SchemaImpl().ref(schema.getRef()));
}
} else if (ModelUtils.getParameterType(parameter) != null) {
} else if (ModelUtils.getParameterType(context, parameter) != null) {
for (Parameter param : context.getWorkingOperation()
.getParameters()) {
if (param.getName().equals(ModelUtils.getParameterName(parameter))) {
if (param.getName().equals(ModelUtils.getParameterName(context, parameter))) {
Schema schema = SchemaImpl.createInstance(schemaAnnotation, context);
SchemaImpl.merge(schema, param.getSchema(), true, context);
if (schema.getRef() != null && !schema.getRef().isEmpty()) {
Expand Down Expand Up @@ -794,20 +794,20 @@ private static Parameter findOperationParameterFor(
// Get all parameters with the same name
List<org.glassfish.hk2.classmodel.reflect.Parameter> matchingMethodParameters = annotated.getParameters()
.stream()
.filter(x -> name.equals(ModelUtils.getParameterName(x)))
.filter(x -> name.equals(ModelUtils.getParameterName(context, x)))
.collect(Collectors.toList());
// If there is more than one match, filter it further
In in = parameter.getIn();
if (matchingMethodParameters.size() > 1 && in != null) {
// Remove all parameters of the wrong input type
matchingMethodParameters
.removeIf(x -> ModelUtils.getParameterType(x) != In.valueOf(in.name()));
.removeIf(x -> ModelUtils.getParameterType(context, x) != In.valueOf(in.name()));
}
if (matchingMethodParameters.isEmpty()) {
return null;
}
// If there's only one matching parameter, handle it immediately
String matchingMethodParamName = ModelUtils.getParameterName(matchingMethodParameters.get(0));
String matchingMethodParamName = ModelUtils.getParameterName(context, matchingMethodParameters.get(0));
// Find the matching operation parameter
for (Parameter operationParam : context
.getWorkingOperation().getParameters()) {
Expand All @@ -824,7 +824,7 @@ private static Parameter findOperationParameterFor(
*/
private static Parameter findOperationParameterFor(
org.glassfish.hk2.classmodel.reflect.Parameter annotated, ApiContext context) {
String actualName = ModelUtils.getParameterName(annotated);
String actualName = ModelUtils.getParameterName(context, annotated);
if (actualName == null) {
return null;
}
Expand Down Expand Up @@ -949,7 +949,7 @@ private RequestBody insertDefaultRequestBody(ApiContext context,
// Get the request body type of the method
org.glassfish.hk2.classmodel.reflect.ParameterizedType bodyType = null;
for (org.glassfish.hk2.classmodel.reflect.Parameter methodParam : method.getParameters()) {
if (ModelUtils.isRequestBody(methodParam)) {
if (ModelUtils.isRequestBody(context, methodParam)) {
bodyType = methodParam;
break;
}
Expand Down Expand Up @@ -1173,7 +1173,7 @@ private boolean insertObjectReference(ApiContext context, Reference<?> referee,

if (referenceClass != null && referenceClass instanceof ExtensibleType) {
ExtensibleType referenceClassType = (ExtensibleType) referenceClass;
final AnnotationModel schemaAnnotation = AnnotationInfo.valueOf(referenceClassType)
final AnnotationModel schemaAnnotation = context.getAnnotationInfo(referenceClassType)
.getAnnotation(org.eclipse.microprofile.openapi.annotations.media.Schema.class);
String schemaName = null;
if (schemaAnnotation != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public Response getResponse(@Context HttpServletRequest request, @Context HttpSe
OpenAPI document = null;
try {
document = openApiService.getDocument();
} catch (OpenAPIBuildException ex) {
} catch (OpenAPIBuildException | IOException ex) {
LOGGER.log(WARNING, "OpenAPI document creation failed.", ex);
}

Expand Down
Loading