TypedMap.Key is unintuitive

[leadVisionary]

Consider the following code

package software.amazon.swage.howto;

import software.amazon.swage.collection.TypedMap;
import software.amazon.swage.metrics.ContextData;
import software.amazon.swage.metrics.Metric;
import software.amazon.swage.metrics.MetricRecorder;

import java.time.Instant;
import java.util.Objects;

/**
 * This task shows how to write metrics using Swage.
 */
public final class WriteMetricsTask implements Runnable {
    /**
     * A Metric to write.
     */
    private static final Metric METRIC = Metric.define("Cool");
    /**
     * A value for the Metric.
     */
    private static final Number NUMBER = 42;
    /**
     * that does the recording.
     */
    private final MetricRecorder recorder;
    /**
     * the ContextData that will be recorded from.
     */
    private final ContextData data;

    /**
     * constructor.
     * @param r should not be null.
     * @param data should not be null.
     */
    WriteMetricsTask(final MetricRecorder r,
                     final ContextData data) {
        this.recorder = Objects.requireNonNull(r);
        this.data = Objects.requireNonNull(data);
    }

    @Override
    public void run() {
        final TypedMap c = data
                .with(TypedMap.key("StartTime", String.class),
                        Long.valueOf(Instant.now().toEpochMilli()).toString())
                .build();
        final MetricRecorder.Context context = recorder.context(c);
        context.record(METRIC, NUMBER, null, Instant.now());
    }
}

with a test:

package software.amazon.swage.howto;

import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;

import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import software.amazon.swage.collection.TypedMap;
import software.amazon.swage.metrics.ContextData;
import software.amazon.swage.metrics.MetricRecorder;

import static org.junit.Assert.*;
import static org.mockito.BDDMockito.*;

public class WriteMetricsTaskTest {
    @Mock MetricRecorder recorder;
    @Mock ContextData data;
    @Captor ArgumentCaptor<TypedMap> captor;
    WriteMetricsTask toTest;


    @Before
    public void setup() {
        MockitoAnnotations.initMocks(this);
        given(recorder.context(captor.capture())).willCallRealMethod();
        toTest = new WriteMetricsTask(recorder, data);
    }

    @Test
    public void canWriteMetrics() {
        toTest.run();
        final TypedMap captured = captor.getValue();
        verify(recorder).context(captured);
        assertTrue(captured.containsKey(TypedMap.key("StartTime", String.class)));
    }
}

That test fails. The assertion is false. Looking at the JavaDoc for TypedMap.Key seems to imply this was intentional.

Why would anyone find that usable? What would the intended usage look like here?

[jerpowers]

Yes this is intentional. See javadoc on the TypedMap.

"* Uses of the map are expected to define the keys they use up-front for their

• particular domain, using data only for those well defined keys. Keys are
• specific to the usage for a TypedMap, arbitrarily creating new keys is
• discouraged."

In your example, you would have a key for StartTime defined once, and accessible to whatever needs to know about StartTime in the map. The unit test would then reference the key object, not try to create a new one. Uniqueness of keys are defined by key objects, not by string names.

[leadVisionary]

So if I want new keys, should I be creating a new implementation of TypedMap?

When I want to check that my map contains something, does that mean the Key needs to be accessible in some global registry of keys, a la

Container.SOME_KEY?

I guess what I'm driving at is why doesn't TypedMap.Key implement equals and hashCode if it's meant to be stored in a map?

https://stackoverflow.com/questions/2265503/why-do-i-need-to-override-the-equals-and-hashcode-methods-in-java

[jerpowers]

If you want new keys, you define new keys. But yes they have to be accessible from everywhere you need to reference the values stored under the key. Usually there will be some set of predefined keys, similar to an enum definition. You shouldn't need new implementations of TypedMap unless there is some unique implementation type you want (similar to any other interface).

why doesn't TypedMap.Key implement equals and hashCode if it's meant to be stored in a map?

Because TypedMap is not the same as the Map collection. The key is meant to be stored in a TypedMap, not in a Map. Again, see the javadoc - it's not quite the same contract as Map. In the case of TypedMap keys, equals is identity equals, which is the same as default object equals.

You can think of a TypedMap similar to an EnumMap or IdentityMap, but where the key also contains information on the type of object stored under the key.

[leadVisionary]

Apologies, I didn't express myself clearly. The test case was a small subset of the problem I'm trying to solve, How can I write a library that uses the contract exposed by MetricRecorder to write structured data?

Let's say I have certain constraints I wish to enforce on my TypedMap context, such as the existence of certain fields. Metrics will not be successfully written if the fields are not there, so I try to enforce this at the programmatic layer with code like the following:

public final class ServiceQueryLogRecorder extends MetricRecorder {
    /**
     * the log.
     */
    private static final Logger LOG = Logger.getLogger(ServiceQueryLogRecorder.class.getName());
    /**
     * The query log to write to.
     */
    private final ServiceQueryLog log;

    /**
     * constructor.
     * @param log should not be null.
     */
    public ServiceQueryLogRecorder(final ServiceQueryLog log) {
        this.log = Objects.requireNonNull(log);
    }

    @Override
    public void record(final Metric label,
                          final Number value,
                          final Unit unit,
                          final Instant time,
                          final TypedMap context) {
        try {
            log.append(SwageServiceQueryLogEntryFactory.INSTANCE.from(label, value, unit, time, context));
        } catch (final Exception e) {
            LOG.log(Level.WARNING, "malformed metric", e);
        }
    }

    @Override
    protected void count(final Metric label, final long delta, final TypedMap context) {
        record(label, delta, Unit.NONE, Instant.now(), context);
    }
}

enum SwageServiceQueryLogEntryFactory {
    /**
     * Make this a SINGLETON via Effective Java Item 3.
     */
    INSTANCE;
    /**
     * creates an entry.
     * @param label the metric to log
     * @param value the value for the metric
     * @param unit an optional unit for the value
     * @param time the time the metric occurred
     * @param context additional values, should have StartTime, Marketplace, and Program values.
     * @return a fully formed entry.
     */
    public ServiceQueryLogEntry from(final Metric label,
                                            final Number value,
                                            final Unit unit,
                                            final Instant time,
                                            final TypedMap context) {
        if (label == null) {
            throw new IllegalArgumentException("Should not get null metric");
        }
        if (value == null) {
            throw new IllegalArgumentException("Should not get null value");
        }
        if (unit == null) {
            throw new IllegalArgumentException("Should not get null unit");
        }
        if (time == null) {
            throw new IllegalArgumentException("Should not get null time");
        }
        if (context == null) {
            throw new IllegalArgumentException("Should not get null context");
        }
        if (!context.containsKey(QueryLogContext.PROGRAM)) {
            throw new IllegalArgumentException("Should have Program key set");
        }
        if (!context.containsKey(QueryLogContext.START_TIME)) {
            throw new IllegalArgumentException("Should have StartTime key set");
        }
        final ServiceQueryLogEntry.MetricsLine.Value v;
        if (context.containsKey(QueryLogContext.METRIC_CLASS) && context.containsKey(QueryLogContext.INSTANCE)) {
            v = new ServiceQueryLogEntry.MetricsLine.DimensionedValue(
                    label.toString(),
                    value,
                    unit.toString(),
                    context.get(QueryLogContext.METRIC_CLASS),
                    context.get(QueryLogContext.INSTANCE));
        } else {
            v = new ServiceQueryLogEntry.MetricsLine.Value(label.toString(), value, unit.toString());
        }
        final ServiceQueryLogEntry.MetricsLine metricsLine = new ServiceQueryLogEntry.MetricsLine(v);

        return new ServiceQueryLogEntry.Builder()
                .withStartTime(context.get(QueryLogContext.START_TIME))
                .withMarketplace(context.get(QueryLogContext.MARKETPLACE))
                .withProgram(context.get(QueryLogContext.PROGRAM))
                .withEndTime(Long.valueOf(time.toEpochMilli()).toString())
                .withMetricsLine(metricsLine)
                .withClientProgram(context.get(QueryLogContext.CLIENT_PROGRAM))
                .withOperation(context.get(QueryLogContext.OPERATION))
                .build();
    }
}

If I write an application that listens on a Socket for strings and tries to reconstruct values from a format like
"Metric:Number:Unit:MapKey1=MapValue1|MapKey2=MapValue2...}

I have no access to the classpath of the calling code, nor mechanism for specifying a precise object reference. I may try to construct the map with something like the following:

/**
     * unmarshals a string of type key=value|key2=value2|key3=value3...
     * @param piece a string of the right type
     * @return a built map
     */
    private static TypedMap deserializeMap(final String piece) {
        final ContextData cd = new ContextData();
        for (final String entry : piece.split("\\|")) {
            final String[] parts = entry.split("=");
            cd.add(TypedMap.key(parts[0], String.class), parts[1]);
        }
        return cd.build();
    }

Even if I passed along a class name in the payload, so I wasn't guessing at String but used Class.forName, I still wouldn't get the same reference/instance. So I will fail/throw an exception when I look for the value of a particular TypedMap.Key.

If StandardContext doesn't give me all the fields I need, or the exact right keys for them, and I would like to extend ContextData, how can a MetricRecorder that doesn't have access to my keys perform any kind of validation?

In another scenario, say I'm writing a caching library, and I want to be able to write metrics by accepting an instance of MetricRecorder and using its interface without caring about the implementation.

If I want to check Keys in the map, how can I do so without taking a compile time dependency on the exposed public field of a particular implementation?

How can I add new elements into the TypedMap that I can pass along to other objects without requiring them to take a compile time dependency on my fields for query?

Are these scenarios outside of the intended usage of the API? How would you solve these problems?

[leadVisionary]

I found a way out of the corner I was in with a couple of utility methods:

/**
     * convenience method to see if a context has a key defined.
     * @param context to check for value
     * @param name the key name you're looking for
     * @return true if some key like it is there
     */
    private static boolean hasKeyNamed(final TypedMap context, final String name) {
        return context.keySet()
                .parallelStream()
                .map(TypedMap.Key::toString)
                .anyMatch(s -> s.equalsIgnoreCase(name));
    }

    /**
     * convenience method to retrieve the value for a context with a named key.
     * @param context to get the value from
     * @param name the key name you're looking for
     * @return the value you
     */
    private static String getValueForKeyNamed(final TypedMap context, final String name) {
        if (!hasKeyNamed(context, name)) {
            return "";
        }
        return context.stream()
                .filter(e -> e.getKey().name.equalsIgnoreCase(name))
                .findAny()
                .get()
                .getValue()
                .toString();
    }

Since the right primitives are exposed to basically check the key's name and valueType (I glanced over but didn't realize that these are public, not private fields), if you need this kind of uncommon use case you can figure out how to get what you want.

If this comes up often in the future, it may be worth considering adding these or a variant to TypedMap directly.