JMC-5639: Can't resolve event type names without Recording Settings event

[Marcus Hirt]

Hi Ken,

I would get the distinct event types through an aggregator rather than 
iterating the IItemIterables, like so:

	private static String getEventTypeNames(IItemCollection items) {
		Set<String> names = items.getAggregate(Aggregators.distinct("", TYPE_NAME_ACCESSOR_FACTORY)); //$NON-NLS-1$
		List<String> quotedNames = new ArrayList<>();
		if (names == null || names.isEmpty()) {
			Set<IType<?>> types = items.getAggregate(Aggregators.distinct("", JfrAttributes.EVENT_TYPE)); //$NON-NLS-1$
			for (IType<?> type : types) {
				quotedNames.add("'" + type.getIdentifier() + "'"); //$NON-NLS-1$ //$NON-NLS-2$
			}
		} else {
			for (String name : names) {
				quotedNames.add("'" + name + "'"); //$NON-NLS-1$ //$NON-NLS-2$
			}
		}
		Collections.sort(quotedNames);
		return StringToolkit.join(quotedNames, ", "); //$NON-NLS-1$
	}

Note that both of these implementations, whilst probably good enough in 
practice, are not strictly correct. If there are only some types lacking 
recording settings, they will not be listed. This means the customer may need 
to make several attempts before finding them all. A fully correct 
implementation would compare the set of type ids available in the collection, 
to the set of type ids represented in the recording settings, and only add the 
type ids for the type ids missing in the recording settings.

A smaller and simpler correct fix, given that the TYPE_NAME_ACCESSOR_FACTORY 
is only used by the getEventTypeNames method, would probably be to edit the 
TYPE_NAME_ACCESSOR_FACTORY to something like this:

	/*
	 * Returns the type name, as available in the recording settings, or simply the type id, if not available.
	 */
	private static final IAccessorFactory<String> TYPE_NAME_ACCESSOR_FACTORY = new IAccessorFactory<String>() {
		@Override
		public <T> IMemberAccessor<String, T> getAccessor(final IType<T> type) {
			final IMemberAccessor<LabeledIdentifier, T> ta = JdkAttributes.REC_SETTING_FOR.getAccessor(type);
			return new IMemberAccessor<String, T>() {
				@Override
				public String getMember(T inObject) {
					LabeledIdentifier eventType = ta.getMember(inObject);
					return eventType == null ?  type.getIdentifier() : eventType.getName();
				}

			};
		}
	};

Hope this helps!

Kind regards,
Marcus

On 2018-09-18, 14:08, "jmc-dev on behalf of Ken Dobson" <jmc-dev-bounces at openjdk.java.net on behalf of kdobson at redhat.com> wrote:

    Hi all,
    
    This is a pretty simple fix for
    https://bugs.openjdk.java.net/projects/JMC/issues/JMC-5663 if someone could
    please review it that would be great.
    
    diff -r 49f6575169ce
    core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java
    ---
    a/core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java
    Tue Jul 31 20:02:30 2018 +0200
    +++
    b/core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java
    Wed Sep 12 16:11:17 2018 -0400
    @@ -952,12 +952,16 @@
    
         private static String getEventTypeNames(IItemCollection items) {
             Set<String> names = items.getAggregate(Aggregators.distinct("",
    TYPE_NAME_ACCESSOR_FACTORY)); //$NON-NLS-1$
    -        if (names == null) {
    -            return null;
    +        List<String> quotedNames = new ArrayList<>();
    +        if (names == null || names.isEmpty()) {
    +            for(IItemIterable iter : items){
    +                quotedNames.add("'" + iter.getType().getIdentifier() +
    "'");
    +            }
             }
    -        List<String> quotedNames = new ArrayList<>();
    -        for (String name : names) {
    -            quotedNames.add("'" + name + "'"); //$NON-NLS-1$ //$NON-NLS-2$
    +        else {
    +            for (String name : names) {
    +                quotedNames.add("'" + name + "'"); //$NON-NLS-1$
    //$NON-NLS-2$
    +            }
             }
             Collections.sort(quotedNames);
             return StringToolkit.join(quotedNames, ", "); //$NON-NLS-1$
    
    Thanks,
    
    Ken