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

fix: enable werror for dao-api. #33

Merged
merged 1 commit into from
Nov 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ apply from: "./gradle/release.gradle"
// than in one big change.
def wErrorProjects = [
project(':core-models'),
// project(':dao-api'),
// project(':dao-api-impl'),
project(':dao-api'),
// project(':dao-impl:ebean-dao'),
// project(':dao-impl:elasticsearch-dao'),
// project(':dao-impl:neo4j-dao'),
jplaisted marked this conversation as resolved.
Show resolved Hide resolved
// project(':restli-resources'),
project(':testing'),
project(':validators')
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;
jplaisted marked this conversation as resolved.
Show resolved Hide resolved
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> {
jplaisted marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -47,7 +47,7 @@ private ModelUtils() {
* @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 +241,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 +260,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 +286,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 +303,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