Skip to content

Commit

Permalink
Support for immutable and private fields/properties
Browse files Browse the repository at this point in the history
In addition to better encapsulation and increased safety,
this allows LoganSquare to support Kotlin data classes.

For each fields, marked for parsing from JSON, there must be a
matching constructor parameter in the corresponding position. The names
of parameters are not validated, instead the number and types of parameters
are relied on for safety.

Each model uses either setter/field injection (old behavior) or constructor
injection (new behavior); mixing setters and constructor parameters within
same model is not supported. Calling `parseField` on JsonMappers of models
with constructor injection produces UnsupportedOperationException.

The impact on existing code should be minimal — the new parsing strategy
gets used only when previous version of LoganSquare would produce an error:
e.g. if there are final final or private fields without setters.

See pull request for full description of feature
  • Loading branch information
Alexander-- committed Jul 6, 2017
1 parent a063552 commit e300421
Show file tree
Hide file tree
Showing 12 changed files with 535 additions and 66 deletions.
Original file line number Diff line number Diff line change
@@ -1,25 +1,28 @@
package com.bluelinelabs.logansquare.processor;

import com.bluelinelabs.logansquare.annotation.JsonField;
import com.bluelinelabs.logansquare.processor.processor.Processor;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.ParameterizedTypeName;
import com.squareup.javapoet.TypeName;

import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.io.Writer;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.*;

import javax.annotation.processing.AbstractProcessor;
import javax.annotation.processing.Filer;
import javax.annotation.processing.ProcessingEnvironment;
import javax.annotation.processing.RoundEnvironment;
import javax.lang.model.SourceVersion;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.util.ElementFilter;
import javax.lang.model.util.Elements;
import javax.lang.model.util.Types;
import javax.tools.Diagnostic;
import javax.tools.JavaFileObject;

import static javax.tools.Diagnostic.Kind.ERROR;
Expand Down Expand Up @@ -70,6 +73,18 @@ public boolean process(Set<? extends TypeElement> elements, RoundEnvironment env
if (!jsonObjectHolder.fileCreated) {
jsonObjectHolder.fileCreated = true;

if (jsonObjectHolder.hasParentClass() && hasUnsupportedInheritance(jsonObjectHolder)) {
// The interactions between models with constructor injection and
// and their subclasses can get really weird. For now let's simply
// disallow inheritance from them
continue;
}

if (jsonObjectHolder.needConstructorInjection) {
// our constructor injection requires proper argument order, ensure it here
sortAccordingToFieldOrder(jsonObjectHolder);
}

try {
JavaFileObject jfo = mFiler.createSourceFile(fqcn);
Writer writer = jfo.openWriter();
Expand All @@ -91,6 +106,64 @@ public boolean process(Set<? extends TypeElement> elements, RoundEnvironment env
}
}

@SuppressWarnings("all")
private void sortAccordingToFieldOrder(JsonObjectHolder jsonObjectHolder) {
TypeName jsonModel = jsonObjectHolder.objectTypeName;

TypeName jsonModelType = jsonModel instanceof ClassName
? (ClassName) jsonModel
: ((ParameterizedTypeName) jsonModel).rawType;

TypeElement jsonModelElement = mElementUtils.getTypeElement(jsonModelType.toString());

final List<VariableElement> fields = ElementFilter.fieldsIn(jsonModelElement.getEnclosedElements());

for (int i = 0; i < fields.size(); ++i) {
((List) fields).set(i, fields.get(i).getSimpleName().toString());
}

final Map<String, JsonFieldHolder> oldMap = jsonObjectHolder.fieldMap;

final TreeMap<String, JsonFieldHolder> newMap = new TreeMap<>(new Comparator<String>() {
@Override
public int compare(String o1, String o2) {
return Integer.compare(fields.indexOf(o1), fields.indexOf(o2));
}
});

newMap.putAll(oldMap);

jsonObjectHolder.fieldMap = newMap;
}

private boolean hasUnsupportedInheritance(JsonObjectHolder objectHolder) {
TypeName parentType = objectHolder.parentTypeName;

ClassName parentClass = parentType instanceof ClassName
? (ClassName) parentType
: ((ParameterizedTypeName) parentType).rawType;

JsonObjectHolder parentHolder =
mJsonObjectMap.get(TypeUtils.getInjectedFQCN(parentClass));

if (parentHolder == null || !parentHolder.needConstructorInjection) {
return false;
}

TypeName badType = objectHolder.objectTypeName;

TypeName perpetratorType = badType instanceof ClassName
? (ClassName) badType
: ((ParameterizedTypeName) badType).rawType;

TypeElement badTypeElement = mElementUtils.getTypeElement(perpetratorType.toString());

processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR,
"Subclassing from models with constructor injection is not supported", badTypeElement);

return true;
}

private void error(String message, Object... args) {
processingEnv.getMessager().printMessage(ERROR, String.format(message, args));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,37 @@

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

import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.ElementFilter;
import javax.lang.model.util.Elements;
import javax.lang.model.util.Types;

import static javax.lang.model.element.Modifier.FINAL;
import static javax.lang.model.element.Modifier.PRIVATE;

public class JsonFieldHolder {

public String[] fieldName;
public String setterMethod;
public String getterMethod;
public boolean shouldParse;
public boolean shouldSerialize;
public boolean needConstructorArg;
public Type type;

public String fill(Element element, Elements elements, Types types, String[] fieldNames, TypeMirror typeConverterType, JsonObjectHolder objectHolder, boolean shouldParse, boolean shouldSerialize) {
if (shouldParse && hasNoMatchingSetter(element, elements)) {
this.needConstructorArg = true;
objectHolder.needConstructorInjection = true;
}

if (fieldNames == null || fieldNames.length == 0) {
String defaultFieldName = element.getSimpleName().toString();

Expand All @@ -49,6 +60,12 @@ public String fill(Element element, Elements elements, Types types, String[] fie
return ensureValidType(type, element);
}

private static boolean hasNoMatchingSetter(Element element, Elements elements) {
final Set<Modifier> modifiers = element.getModifiers();

return modifiers.contains(FINAL) || modifiers.contains(PRIVATE) && TextUtils.isEmpty(JsonFieldHolder.getSetter(element, elements));
}

private String ensureValidType(Type type, Element element) {
if (type == null) {
return "Type could not be determined for " + element.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@ public class JsonObjectHolder {
public String onCompleteCallback;
public String preSerializeCallback;

public boolean needConstructorInjection;

// Using a TreeMap now to keep the entries sorted. This ensures that code is
// always written the exact same way, no matter which JDK you're using.
public final Map<String, JsonFieldHolder> fieldMap = new TreeMap<>();
public Map<String, JsonFieldHolder> fieldMap = new TreeMap<>();

public boolean fileCreated;

public boolean hasParentClass() {
Expand Down
Loading

0 comments on commit e300421

Please sign in to comment.