Skip to content
This repository has been archived by the owner on Jun 24, 2021. It is now read-only.

Commit

Permalink
fix: enable werror for dao-api. (linkedin#33)
Browse files Browse the repository at this point in the history
  • Loading branch information
John Plaisted committed Jan 14, 2021
1 parent 86925c7 commit 59bad03
Show file tree
Hide file tree
Showing 13 changed files with 116 additions and 75 deletions.
24 changes: 14 additions & 10 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,19 @@ project.ext.externalDependency = [
apply plugin: 'com.diffplug.spotless'
apply from: "./gradle/release.gradle"

// TODO expand this to all projects and then delete this allow list. This list is letting us fix errors over time rather
// than in one big change.
def wErrorProjects = [
project(':core-models'),
// project(':dao-api'),
// project(':dao-api-impl'),
// project(':restli-resources'),
project(':testing'),
project(':validators')
// TODO get this list to 0 and then delete this allow list. This list is letting us fix errors over time rather than in
// one big change. This is a block list so new modules automatically have wError enabled.
def skipWErrorProjects = [
project(':dao-impl:ebean-dao'),
project(':dao-impl:elasticsearch-dao'),
project(':dao-impl:elasticsearch-dao-7'),
project(':dao-impl:neo4j-dao'),
project(':gradle-plugins:metadata-annotations-lib'),
project(':gradle-plugins:metadata-annotations-schema'),
project(':gradle-plugins:metadata-annotations-test-models'),
project(':gradle-plugins:metadata-events-generator-lib'),
project(':gradle-plugins:metadata-events-generator-plugin'),
project(':restli-resources'),
]

allprojects {
Expand All @@ -87,7 +91,7 @@ allprojects {
apply plugin: 'checkstyle'

gradle.projectsEvaluated {
if (wErrorProjects.contains(project)) {
if (!skipWErrorProjects.contains(project)) {
tasks.withType(JavaCompile) {
options.compilerArgs << "-Xlint:all" << "-Werror" <<
"-Xlint:-deprecation" << // TODO
Expand Down
50 changes: 32 additions & 18 deletions dao-api/src/main/java/com/linkedin/metadata/dao/BaseLocalDAO.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ static class AddResult<ASPECT extends RecordTemplate> {

private static final int DEFAULT_MAX_TRANSACTION_RETRY = 3;

protected final BaseMetadataEventProducer _producer;
protected final BaseMetadataEventProducer<?, ASPECT_UNION, URN> _producer;
protected final LocalDAOStorageConfig _storageConfig;

// Maps an aspect class to the corresponding retention policy
private final Map<Class<? extends RecordTemplate>, Retention> _aspectRetentionMap = new HashMap<>();

// Maps as aspect class to a list of post-update hooks
private final Map<Class<? extends RecordTemplate>, List<BiConsumer<Urn, RecordTemplate>>> _aspectPostUpdateHooksMap =
new HashMap<>();
private final Map<Class<? extends RecordTemplate>, List<BiConsumer<URN, ? extends RecordTemplate>>>
_aspectPostUpdateHooksMap = new HashMap<>();

// Maps an aspect class to the corresponding equality tester
private final Map<Class<? extends RecordTemplate>, EqualityTester<? extends RecordTemplate>>
Expand All @@ -109,7 +109,8 @@ static class AddResult<ASPECT extends RecordTemplate> {
* com.linkedin.metadata.aspect
* @param producer {@link BaseMetadataEventProducer} for the metadata event producer
*/
public BaseLocalDAO(@Nonnull Class<ASPECT_UNION> aspectUnionClass, @Nonnull BaseMetadataEventProducer producer) {
public BaseLocalDAO(@Nonnull Class<ASPECT_UNION> aspectUnionClass,
@Nonnull BaseMetadataEventProducer<?, ASPECT_UNION, URN> producer) {
super(aspectUnionClass);
_producer = producer;
_storageConfig = LocalDAOStorageConfig.builder().build();
Expand All @@ -121,7 +122,8 @@ public BaseLocalDAO(@Nonnull Class<ASPECT_UNION> aspectUnionClass, @Nonnull Base
* @param producer {@link BaseMetadataEventProducer} for the metadata event producer
* @param storageConfig {@link LocalDAOStorageConfig} containing storage config of full list of supported aspects
*/
public BaseLocalDAO(@Nonnull BaseMetadataEventProducer producer, @Nonnull LocalDAOStorageConfig storageConfig) {
public BaseLocalDAO(@Nonnull BaseMetadataEventProducer<?, ASPECT_UNION, URN> producer,
@Nonnull LocalDAOStorageConfig storageConfig) {
super(storageConfig.getAspectStorageConfigMap().keySet());
_producer = producer;
_storageConfig = storageConfig;
Expand Down Expand Up @@ -159,20 +161,20 @@ public <ASPECT extends RecordTemplate> Retention getRetention(@Nonnull Class<ASP
* order of invocation when multiple hooks are added for a single aspect. Adding the same hook again will result in
* {@link IllegalArgumentException} thrown. Hooks are invoked in the order they're registered.
*/
public <URN extends Urn, ASPECT extends RecordTemplate> void addPostUpdateHook(@Nonnull Class<ASPECT> aspectClass,
public <ASPECT extends RecordTemplate> void addPostUpdateHook(@Nonnull Class<ASPECT> aspectClass,
@Nonnull BiConsumer<URN, ASPECT> postUpdateHook) {

checkValidAspect(aspectClass);
// TODO: Also validate Urn once we convert all aspect models to PDL with proper annotation

final List<BiConsumer<Urn, RecordTemplate>> hooks =
final List<BiConsumer<URN, ? extends RecordTemplate>> hooks =
_aspectPostUpdateHooksMap.getOrDefault(aspectClass, new LinkedList<>());

if (hooks.contains(postUpdateHook)) {
throw new IllegalArgumentException("Adding an already-registered hook");
}

hooks.add((BiConsumer<Urn, RecordTemplate>) postUpdateHook);
hooks.add(postUpdateHook);
_aspectPostUpdateHooksMap.put(aspectClass, hooks);
}

Expand All @@ -189,10 +191,11 @@ public <ASPECT extends RecordTemplate> void setEqualityTester(@Nonnull Class<ASP
* Gets the {@link EqualityTester} for an aspect, or {@link DefaultEqualityTester} if none is registered.
*/
@Nonnull
@SuppressWarnings("unchecked")
public <ASPECT extends RecordTemplate> EqualityTester<ASPECT> getEqualityTester(@Nonnull Class<ASPECT> aspectClass) {
checkValidAspect(aspectClass);
return (EqualityTester<ASPECT>) _aspectEqualityTesterMap.computeIfAbsent(aspectClass,
key -> new DefaultEqualityTester<ASPECT>());
key -> new DefaultEqualityTester<>());
}

/**
Expand Down Expand Up @@ -228,6 +231,7 @@ public void setAlwaysEmitAspectSpecificAuditEvent(boolean alwaysEmitAspectSpecif
*
* @deprecated Use {@link #enableLocalSecondaryIndex(boolean)} instead
*/
@Deprecated
public void setWriteToLocalSecondaryIndex(boolean writeToLocalSecondaryIndex) {
_enableLocalSecondaryIndex = writeToLocalSecondaryIndex;
}
Expand Down Expand Up @@ -315,7 +319,11 @@ public <ASPECT extends RecordTemplate> ASPECT add(@Nonnull URN urn, @Nonnull Cla
}
// 7. Invoke post-update hooks if there's any
if (_aspectPostUpdateHooksMap.containsKey(aspectClass)) {
_aspectPostUpdateHooksMap.get(aspectClass).forEach(hook -> hook.accept(urn, newValue));
for (BiConsumer<URN, ? extends RecordTemplate> hook : _aspectPostUpdateHooksMap.get(aspectClass)) {
@SuppressWarnings("unchecked")
final BiConsumer<URN, ASPECT> typedHook = ((BiConsumer<URN, ASPECT>) hook);
typedHook.accept(urn, newValue);
}
}

return newValue;
Expand All @@ -334,6 +342,7 @@ public <ASPECT extends RecordTemplate> ASPECT add(@Nonnull URN urn, @Nonnull Cla
* Similar to {@link #add(Urn, Class, Function, AuditStamp)} but takes the new value directly.
*/
@Nonnull
@SuppressWarnings("unchecked")
public <ASPECT extends RecordTemplate> ASPECT add(@Nonnull URN urn, @Nonnull ASPECT newValue,
@Nonnull AuditStamp auditStamp) {
return add(urn, (Class<ASPECT>) newValue.getClass(), ignored -> newValue, auditStamp);
Expand Down Expand Up @@ -378,8 +387,8 @@ protected abstract <ASPECT extends RecordTemplate> long saveLatest(@Nonnull URN
* @param newValue {@link RecordTemplate} of the new value of aspect
* @param version version of the aspect
*/
protected abstract <ASPECT extends RecordTemplate> void updateLocalIndex(@Nonnull URN urn,
@Nullable ASPECT newValue, long version);
protected abstract <ASPECT extends RecordTemplate> void updateLocalIndex(@Nonnull URN urn, @Nullable ASPECT newValue,
long version);

/**
* Returns list of urns from local secondary index that satisfy the given filter conditions.
Expand All @@ -399,8 +408,8 @@ protected abstract <ASPECT extends RecordTemplate> void updateLocalIndex(@Nonnul
*/
@Nonnull
public List<URN> listUrns(@Nonnull Class<URN> urnClazz, @Nullable URN lastUrn, int pageSize) {
final IndexFilter indexFilter = new IndexFilter()
.setCriteria(new IndexCriterionArray(new IndexCriterion().setAspect(urnClazz.getCanonicalName())));
final IndexFilter indexFilter = new IndexFilter().setCriteria(
new IndexCriterionArray(new IndexCriterion().setAspect(urnClazz.getCanonicalName())));
return listUrns(indexFilter, lastUrn, pageSize);
}

Expand Down Expand Up @@ -519,8 +528,10 @@ protected abstract <ASPECT extends RecordTemplate> void applyTimeBasedRetention(
* @return backfilled aspect
* @deprecated Use {@link #backfill(Set, Set)} instead
*/
@Deprecated
@Nonnull
public <ASPECT extends RecordTemplate> Optional<ASPECT> backfill(@Nonnull Class<ASPECT> aspectClass, @Nonnull URN urn) {
public <ASPECT extends RecordTemplate> Optional<ASPECT> backfill(@Nonnull Class<ASPECT> aspectClass,
@Nonnull URN urn) {
return backfill(BackfillMode.BACKFILL_ALL, aspectClass, urn);
}

Expand Down Expand Up @@ -560,7 +571,8 @@ public Map<URN, Map<Class<? extends RecordTemplate>, Optional<? extends RecordTe
private Map<URN, Map<Class<? extends RecordTemplate>, Optional<? extends RecordTemplate>>> backfill(
@Nonnull BackfillMode mode, @Nonnull Set<Class<? extends RecordTemplate>> aspectClasses, @Nonnull Set<URN> urns) {
checkValidAspects(aspectClasses);
final Map<URN, Map<Class<? extends RecordTemplate>, Optional<? extends RecordTemplate>>> urnToAspects = get(aspectClasses, urns);
final Map<URN, Map<Class<? extends RecordTemplate>, Optional<? extends RecordTemplate>>> urnToAspects =
get(aspectClasses, urns);
urnToAspects.forEach((urn, aspects) -> {
aspects.forEach((aspectClass, aspect) -> aspect.ifPresent(value -> backfill(mode, value, urn)));
});
Expand All @@ -584,7 +596,7 @@ public Map<URN, Map<Class<? extends RecordTemplate>, Optional<? extends RecordTe
@Nonnull Class<URN> urnClazz, @Nullable URN lastUrn, int pageSize) {

final List<URN> urnList = listUrns(urnClazz, lastUrn, pageSize);
return backfill(mode, aspectClasses, new HashSet(urnList));
return backfill(mode, aspectClasses, new HashSet<>(urnList));
}

/**
Expand All @@ -595,7 +607,8 @@ public Map<URN, Map<Class<? extends RecordTemplate>, Optional<? extends RecordTe
* @param urn {@link Urn} for the entity
* @param <ASPECT> must be a supported aspect type in {@code ASPECT_UNION}.
*/
private <ASPECT extends RecordTemplate> void backfill(@Nonnull BackfillMode mode, @Nonnull ASPECT aspect, @Nonnull URN urn) {
private <ASPECT extends RecordTemplate> void backfill(@Nonnull BackfillMode mode, @Nonnull ASPECT aspect,
@Nonnull URN urn) {
if (_enableLocalSecondaryIndex && (mode == BackfillMode.SCSI_ONLY || mode == BackfillMode.BACKFILL_ALL)) {
updateLocalIndex(urn, aspect, FIRST_VERSION);
}
Expand Down Expand Up @@ -687,6 +700,7 @@ public abstract <ASPECT extends RecordTemplate> ListResult<ASPECT> list(@Nonnull
* Similar to {@link #getWithExtraInfo(Set)} but only using only one {@link AspectKey}.
*/
@Nonnull
@SuppressWarnings("unchecked")
public <ASPECT extends RecordTemplate> Optional<AspectWithExtraInfo<ASPECT>> getWithExtraInfo(
@Nonnull AspectKey<URN, ASPECT> key) {
if (getWithExtraInfo(Collections.singleton(key)).containsKey(key)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public BaseReadDAO(@Nonnull Set<Class<? extends RecordTemplate>> aspects) {
* Similar to {@link #get(Set)} but only using only one {@link AspectKey}.
*/
@Nonnull
@SuppressWarnings("unchecked")
public <ASPECT extends RecordTemplate> Optional<ASPECT> get(@Nonnull AspectKey<URN, ASPECT> key) {
return (Optional<ASPECT>) get(Collections.singleton(key)).get(key);
}
Expand Down Expand Up @@ -116,6 +117,7 @@ public Map<Class<? extends RecordTemplate>, Optional<? extends RecordTemplate>>
* Similar to {@link #get(Set, Set)} but only for one aspect.
*/
@Nonnull
@SuppressWarnings("unchecked")
public <ASPECT extends RecordTemplate> Map<URN, Optional<ASPECT>> get(
@Nonnull Class<ASPECT> aspectClass, @Nonnull Set<URN> urns) {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
package com.linkedin.metadata.dao.equality;

import com.linkedin.data.template.DataTemplate;
import com.linkedin.data.template.RecordTemplate;
import javax.annotation.Nonnull;


/**
* A {@link EqualityTester} that always returns false.
*/
public class AlwaysFalseEqualityTester<T extends DataTemplate> implements EqualityTester<T> {
public class AlwaysFalseEqualityTester<T extends RecordTemplate> implements EqualityTester<T> {
private static final AlwaysFalseEqualityTester<?> INSTANCE = new AlwaysFalseEqualityTester<>();

/**
* Creates a new instance of {@link AlwaysFalseEqualityTester}.
* Returns the singleton instance of {@link AlwaysFalseEqualityTester}.
*/
public static <CLASS extends DataTemplate> AlwaysFalseEqualityTester<CLASS> newInstance() {
return new AlwaysFalseEqualityTester<>();
@SuppressWarnings("unchecked")
public static <T extends RecordTemplate> AlwaysFalseEqualityTester<T> instance() {
return (AlwaysFalseEqualityTester<T>) INSTANCE;
}

@Override
public boolean equals(@Nonnull T o1, @Nonnull T o2) {
public boolean equals(@Nonnull RecordTemplate o1, @Nonnull RecordTemplate o2) {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,23 @@

import com.linkedin.data.template.DataTemplate;
import com.linkedin.data.template.DataTemplateUtil;
import com.linkedin.data.template.RecordTemplate;
import javax.annotation.Nonnull;


/**
* A {@link EqualityTester} that uses {@link DataTemplateUtil#areEqual(DataTemplate, DataTemplate)} to check for
* semantic equality.
*/
public class DefaultEqualityTester<T extends DataTemplate> implements EqualityTester<T> {
public class DefaultEqualityTester<T extends RecordTemplate> implements EqualityTester<T> {
private static final DefaultEqualityTester<?> INSTANCE = new DefaultEqualityTester<>();

/**
* Creates a new instance of {@link DefaultEqualityTester}.
* Returns the singleton instance of {@link DefaultEqualityTester}.
*/
public static <CLASS extends DataTemplate> DefaultEqualityTester<CLASS> newInstance() {
return new DefaultEqualityTester<>();
@SuppressWarnings("unchecked")
public static <T extends RecordTemplate> DefaultEqualityTester<T> instance() {
return (DefaultEqualityTester<T>) INSTANCE;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package com.linkedin.metadata.dao.equality;

import com.linkedin.data.template.DataTemplate;
import com.linkedin.data.template.RecordTemplate;
import javax.annotation.Nonnull;


/**
* An interface for testing equality between two objects of the same type.
*/
public interface EqualityTester<T extends DataTemplate> {
public interface EqualityTester<T extends RecordTemplate> {

boolean equals(@Nonnull T o1, @Nonnull T o2);
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,9 @@ private ModelUtils() {
* Gets the corresponding aspect name for a specific aspect type.
*
* @param aspectClass the aspect type
* @param <T> must be a valid aspect type
* @return the corresponding aspect name, which is actually the FQCN of type
*/
public static <T extends DataTemplate> String getAspectName(@Nonnull Class<T> aspectClass) {
public static String getAspectName(@Nonnull Class<? extends DataTemplate<?>> aspectClass) {
return aspectClass.getCanonicalName();
}

Expand Down Expand Up @@ -241,7 +240,7 @@ public static <SNAPSHOT extends RecordTemplate> List<RecordTemplate> getAspectsF
* @return the extracted aspect
*/
@Nonnull
public static <SNAPSHOT extends RecordTemplate, ASPECT extends DataTemplate> Optional<ASPECT> getAspectFromSnapshot(
public static <SNAPSHOT extends RecordTemplate, ASPECT extends DataTemplate<?>> Optional<ASPECT> getAspectFromSnapshot(
@Nonnull SNAPSHOT snapshot, @Nonnull Class<ASPECT> aspectClass) {

return getAspectsFromSnapshot(snapshot).stream()
Expand All @@ -260,9 +259,9 @@ public static List<RecordTemplate> getAspectsFromSnapshotUnion(@Nonnull UnionTem

@Nonnull
private static List<RecordTemplate> getAspects(@Nonnull RecordTemplate snapshot) {
final Class<? extends WrappingArrayTemplate> clazz = getAspectsArrayClass(snapshot.getClass());
final Class<? extends WrappingArrayTemplate<?>> clazz = getAspectsArrayClass(snapshot.getClass());

WrappingArrayTemplate aspectArray = RecordUtils.getRecordTemplateWrappedField(snapshot, "aspects", clazz);
WrappingArrayTemplate<?> aspectArray = RecordUtils.getRecordTemplateWrappedField(snapshot, "aspects", clazz);

final List<RecordTemplate> aspects = new ArrayList<>();
aspectArray.forEach(item -> aspects.add(RecordUtils.getSelectedRecordTemplateFromUnion((UnionTemplate) item)));
Expand All @@ -286,12 +285,14 @@ public static <SNAPSHOT extends RecordTemplate, ASPECT_UNION extends UnionTempla

SnapshotValidator.validateSnapshotSchema(snapshotClass);

final Class<? extends WrappingArrayTemplate> aspectArrayClass = getAspectsArrayClass(snapshotClass);
final Class<? extends WrappingArrayTemplate<?>> aspectArrayClass = getAspectsArrayClass(snapshotClass);

try {
final SNAPSHOT snapshot = snapshotClass.newInstance();
RecordUtils.setRecordTemplatePrimitiveField(snapshot, "urn", urn.toString());
WrappingArrayTemplate aspectArray = aspectArrayClass.newInstance();
@SuppressWarnings("unchecked")
WrappingArrayTemplate<ASPECT_UNION> aspectArray =
(WrappingArrayTemplate<ASPECT_UNION>) aspectArrayClass.newInstance();
aspectArray.addAll(aspects);
RecordUtils.setRecordTemplateComplexField(snapshot, "aspects", aspectArray);
return snapshot;
Expand All @@ -301,11 +302,14 @@ public static <SNAPSHOT extends RecordTemplate, ASPECT_UNION extends UnionTempla
}

@Nonnull
private static <SNAPSHOT extends RecordTemplate> Class<? extends WrappingArrayTemplate> getAspectsArrayClass(
@SuppressWarnings("unchecked")
private static <SNAPSHOT extends RecordTemplate> Class<? extends WrappingArrayTemplate<?>> getAspectsArrayClass(
@Nonnull Class<SNAPSHOT> snapshotClass) {

try {
return snapshotClass.getMethod("getAspects").getReturnType().asSubclass(WrappingArrayTemplate.class);
return (Class<? extends WrappingArrayTemplate<?>>) snapshotClass.getMethod("getAspects")
.getReturnType()
.asSubclass(WrappingArrayTemplate.class);
} catch (NoSuchMethodException | ClassCastException e) {
throw new RuntimeException((e));
}
Expand Down
Loading

0 comments on commit 59bad03

Please sign in to comment.