Skip to content

Commit f8d5779

Browse files
vkryachkovkryachko
authored
Fix bugs in encoder codegen (#1038)
* Encoders for members of Collection<T> and Map<T,U> were not generated. * Enum encoders were generated but they should not be.
1 parent 282721c commit f8d5779

File tree

3 files changed

+161
-13
lines changed

3 files changed

+161
-13
lines changed

‎encoders/firebase-encoders-processor/src/main/java/com/google/firebase/encoders/processor/EncodableProcessor.java

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,14 @@
3030
import com.squareup.javapoet.TypeSpec;
3131
import com.squareup.javapoet.WildcardTypeName;
3232
import java.io.IOException;
33+
import java.util.Collections;
3334
import java.util.HashMap;
3435
import java.util.LinkedHashMap;
3536
import java.util.LinkedHashSet;
3637
import java.util.Map;
3738
import java.util.Optional;
3839
import java.util.Set;
40+
import java.util.stream.Collectors;
3941
import javax.annotation.processing.AbstractProcessor;
4042
import javax.annotation.processing.ProcessingEnvironment;
4143
import javax.annotation.processing.Processor;
@@ -254,9 +256,7 @@ public VisitResult<Encoder> visit(TypeMirror type) {
254256

255257
Set<TypeMirror> result = new LinkedHashSet<>();
256258
for (Getter getter : getterFactory.allGetters((DeclaredType) type)) {
257-
if (shouldCreateEncoderFor(getter.getUnderlyingType())) {
258-
result.add(getter.getUnderlyingType());
259-
}
259+
result.addAll(getTypesToVisit(getter.getUnderlyingType()));
260260
methodBuilder.addCode("ctx.add($S, value.$L);\n", getter.name(), getter.expression());
261261
}
262262

@@ -279,21 +279,30 @@ public VisitResult<Encoder> visit(TypeMirror type) {
279279
return VisitResult.of(result, Encoder.create(types.erasure(type), encoder));
280280
}
281281

282-
private boolean shouldCreateEncoderFor(TypeMirror type) {
283-
if (type.getKind().isPrimitive()) {
284-
return false;
282+
private Set<TypeMirror> getTypesToVisit(TypeMirror type) {
283+
TypeMirror date = elements.getTypeElement("java.util.Date").asType();
284+
TypeMirror enumType = types.erasure(elements.getTypeElement("java.lang.Enum").asType());
285+
if (type.getKind().isPrimitive()
286+
|| types.isAssignable(type, date)
287+
|| types.isAssignable(type, enumType)
288+
|| "java.lang".equals(Names.packageName(types.asElement(type)))) {
289+
return Collections.emptySet();
290+
}
291+
if (!(type instanceof DeclaredType)) {
292+
return Collections.emptySet();
285293
}
294+
295+
DeclaredType dType = (DeclaredType) type;
296+
286297
TypeMirror collection =
287298
types.erasure(elements.getTypeElement("java.util.Collection").asType());
288299
TypeMirror map = types.erasure(elements.getTypeElement("java.util.Map").asType());
289-
TypeMirror date = elements.getTypeElement("java.util.Date").asType();
290-
if (types.isAssignable(type, collection)
291-
|| types.isAssignable(type, map)
292-
|| types.isAssignable(type, date)
293-
|| "java.lang".equals(Names.packageName(types.asElement(type)))) {
294-
return false;
300+
if (types.isAssignable(dType, collection) || types.isAssignable(dType, map)) {
301+
return dType.getTypeArguments().stream()
302+
.flatMap(t -> getTypesToVisit(t).stream())
303+
.collect(Collectors.toSet());
295304
}
296-
return true;
305+
return Collections.singleton(type);
297306
}
298307
}
299308
}

‎encoders/firebase-encoders-processor/src/test/java/com/google/firebase/encoders/processor/EncodableProcessorTest.java

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,86 @@ public void compile_withAutoValueInDifferentPackage_shouldRegisterGeneratedSubcl
255255
.contains("Class<? extends Member> TYPE = AutoValue_Member.class");
256256
}
257257

258+
@Test
259+
public void compile_withListOfCustomType_shouldRegisterEncoderForThatType() {
260+
Compilation result =
261+
javac()
262+
.withProcessors(new AutoValueProcessor(), new EncodableProcessor())
263+
.compile(
264+
JavaFileObjects.forSourceLines(
265+
"TypeWithList",
266+
"import com.google.firebase.encoders.annotations.Encodable;",
267+
"import java.util.List;",
268+
"@Encodable class TypeWithList {",
269+
"public List<Member> getMember() { return null; }",
270+
"}"),
271+
JavaFileObjects.forSourceLines("Member", "class Member {}"));
272+
273+
assertThat(result)
274+
.generatedSourceFile("AutoTypeWithListEncoder")
275+
.hasSourceEquivalentTo(JavaFileObjects.forResource("ExpectedTypeWithListEncoder.java"));
276+
}
277+
278+
@Test
279+
public void compile_withSetOfSetOfCustomType_shouldRegisterEncoderForThatType() {
280+
Compilation result =
281+
javac()
282+
.withProcessors(new AutoValueProcessor(), new EncodableProcessor())
283+
.compile(
284+
JavaFileObjects.forSourceLines(
285+
"TypeWithList",
286+
"import com.google.firebase.encoders.annotations.Encodable;",
287+
"import java.util.Set;",
288+
"@Encodable class TypeWithList {",
289+
"public Set<Set<Member>> getMember() { return null; }",
290+
"}"),
291+
JavaFileObjects.forSourceLines("Member", "class Member {}"));
292+
293+
assertThat(result)
294+
.generatedSourceFile("AutoTypeWithListEncoder")
295+
.hasSourceEquivalentTo(JavaFileObjects.forResource("ExpectedTypeWithListEncoder.java"));
296+
}
297+
298+
@Test
299+
public void compile_withMapOfListOfCustomType_shouldRegisterEncoderForThatType() {
300+
Compilation result =
301+
javac()
302+
.withProcessors(new AutoValueProcessor(), new EncodableProcessor())
303+
.compile(
304+
JavaFileObjects.forSourceLines(
305+
"TypeWithList",
306+
"import com.google.firebase.encoders.annotations.Encodable;",
307+
"import java.util.List;",
308+
"import java.util.Map;",
309+
"@Encodable class TypeWithList {",
310+
"public Map<String, List<Member>> getMember() { return null; }",
311+
"}"),
312+
JavaFileObjects.forSourceLines("Member", "class Member {}"));
313+
314+
assertThat(result)
315+
.generatedSourceFile("AutoTypeWithListEncoder")
316+
.hasSourceEquivalentTo(JavaFileObjects.forResource("ExpectedTypeWithListEncoder.java"));
317+
}
318+
319+
@Test
320+
public void compile_withEnum_shouldNotGenerateEnumEncoder() {
321+
Compilation result =
322+
javac()
323+
.withProcessors(new AutoValueProcessor(), new EncodableProcessor())
324+
.compile(
325+
JavaFileObjects.forSourceLines(
326+
"TypeWithEnum",
327+
"import com.google.firebase.encoders.annotations.Encodable;",
328+
"@Encodable class TypeWithEnum {",
329+
"public MyEnum getEnum() { return null; }",
330+
"}"),
331+
JavaFileObjects.forSourceLines("MyEnum", "enum MyEnum {VALUE;}"));
332+
assertThat(result)
333+
.generatedSourceFile("AutoTypeWithEnumEncoder")
334+
.contentsAsUtf8String()
335+
.doesNotContain("cfg.registerEncoder(MyEnum.class");
336+
}
337+
258338
@Test
259339
public void packageNameToCamelCase_withDefaultPackage_shouldReturnEmptyString() {
260340
assertThat(EncodableProcessor.packageNameToCamelCase("")).isEqualTo("");

‎encoders/firebase-encoders-processor/src/test/resources/ExpectedTypeWithListEncoder.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Copyright 2019 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import com.google.firebase.encoders.EncodingException;
16+
import com.google.firebase.encoders.ObjectEncoder;
17+
import com.google.firebase.encoders.ObjectEncoderContext;
18+
import com.google.firebase.encoders.config.Configurator;
19+
import com.google.firebase.encoders.config.EncoderConfig;
20+
import java.io.IOException;
21+
import java.lang.Override;
22+
import javax.annotation.Generated;
23+
24+
/**
25+
* @hide */
26+
@Generated("com.google.firebase.encoders.processor.EncodableProcessor")
27+
public final class AutoTypeWithListEncoder implements Configurator {
28+
public static final int CODEGEN_VERSION = 1;
29+
30+
public static final Configurator CONFIG = new AutoTypeWithListEncoder();
31+
32+
private AutoTypeWithListEncoder() {
33+
}
34+
35+
@Override
36+
public void configure(EncoderConfig<?> cfg) {
37+
cfg.registerEncoder(TypeWithList.class, TypeWithListEncoder.INSTANCE);
38+
cfg.registerEncoder(Member.class, MemberEncoder.INSTANCE);
39+
}
40+
41+
private static final class TypeWithListEncoder implements ObjectEncoder<TypeWithList> {
42+
static final TypeWithListEncoder INSTANCE = new TypeWithListEncoder();
43+
44+
@Override
45+
public void encode(TypeWithList value, ObjectEncoderContext ctx) throws IOException,
46+
EncodingException {
47+
ctx.add("member", value.getMember());
48+
}
49+
}
50+
51+
private static final class MemberEncoder implements ObjectEncoder<Member> {
52+
static final MemberEncoder INSTANCE = new MemberEncoder();
53+
54+
@Override
55+
public void encode(Member value, ObjectEncoderContext ctx) throws IOException,
56+
EncodingException {
57+
}
58+
}
59+
}

0 commit comments

Comments
 (0)