generate edges depending on type category #2882
Conversation
| <property name="edgeAttribute2" value="NAME"/> | ||
| <property name="edgeAttribute3" value="ID"/> | ||
| <property name="activityDateField" value = "PREMIERED"/> | ||
| <property name="allowedTypeCategories"> |
There was a problem hiding this comment.
Do we want separate allowed type categories for the source and the sink ?
There was a problem hiding this comment.
I would let the bidirectional config dictate that.
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| import org.apache.accumulo.core.util.Pair; |
There was a problem hiding this comment.
I thought we were trying to avoid using this class, maybe replace with apache common's Pair?
There was a problem hiding this comment.
ah, right. slipped my mind
There was a problem hiding this comment.
AbstractMap.SimplEntry is also a fine replacement.
| long futureDelta, pastDelta; | ||
| long newFormatStartDate; | ||
|
|
||
| protected List<datawave.data.type.Type.Category> allowedCategories = Lists.newArrayList(); |
There was a problem hiding this comment.
I would prefer to see a static import for Type.Category
| RawRecordContainer myEvent = getEvent(conf); | ||
|
|
||
| EdgeHandlerTestUtil.processEvent(fields, edgeHandler, myEvent, 0, true, false); | ||
| Assert.assertEquals(expectedKeys, EdgeHandlerTestUtil.edgeKeyResults.keySet()); |
There was a problem hiding this comment.
If this test is demonstrating that no edge keys are produced I would rename the method and rewrite the assertions to be clear. Someone who isn't familiar with this code like myself might wonder what edges are produced when the preconditions belong to different groups, see that we create a set of expected keys, and then go looking for where we updated the expected collection.
There was a problem hiding this comment.
"// test edge config only allows FULL category. LIST_ELEMENTS edges should not be produced in this case"
No problem renaming the test. When I hit these in other MRs I usually expand the rest of the Test class to see what is unaffected and other cases currently handled.
"When the preconditions belong to different groups" ? Not sure what you mean there. You have one precondition per edge, each edge is aware or unaware of the group, and the fields belong to different groups or the same group. The test names try to indicate which combination of attributes
is being tested, which, I agree, is not intuitive.
e.g.,
| // geo | ||
| for (String normalized : ((OneToManyNormalizerType<Geometry>) geoType).normalizeToMany("POINT(10 10)")) { | ||
| for (String normalized : ((OneToManyNormalizerType<Geometry>) geoType).normalizeToMany("POINT(10 10)").stream() | ||
| .map(org.apache.accumulo.core.util.Pair::getFirst).collect(Collectors.toList())) { |
There was a problem hiding this comment.
Use apache common's Pair and use static import
Conflicts: core/utils/type-utils/src/main/java/datawave/data/normalizer/OneToManyNormalizer.java core/utils/type-utils/src/main/java/datawave/data/type/GeometryType.java core/utils/type-utils/src/main/java/datawave/data/type/ListType.java core/utils/type-utils/src/main/java/datawave/data/type/OneToManyNormalizerType.java core/utils/type-utils/src/test/java/datawave/data/type/ListTypeTest.java
Requires #2885