Block processor attributes is Map<String, Object> but may contain non-String keys.

[jjaderberg]

A block processor needs to take a Map<String, Object> argument for "attributes", but the map that is passed may contain non-String keys.

The following block processor throws a java.lang.ClassCastException: java.lang.Long cannot be cast to java.lang.String when treating all map keys as strings.
The exception is thrown in the for loop because one or more keys in the map are Integers.
If I change the style of the block in the AsciiDoc source to source, no exception is thrown.

My guess is that "custom attributes" are keyed by Integers in the attributes map.
If that's the case, maybe the signature of BlockProcessor#process() should type 'attributes' as Map<Object, Object>, or maybe some type coersion magic needs to happen to make sure the generics of the map in the signature are respected.

It works to treat the map as if it were <Object, Object>, at least for the Integer case.
Since there is a Integer#toString() method, treating a key as an Object in the for loop works just fine.

Block processor:

import org.asciidoctor.ast.StructuralNode;
import org.asciidoctor.extension.BlockProcessor;
import org.asciidoctor.extension.Reader;

import java.util.HashMap;
import java.util.Map;

public class UnexpectedIntegerKeysBlockProcessor extends BlockProcessor {

    private static Map<String, Object> configs = new HashMap<String, Object>() {{
        put(CONTEXTS, CONTEXT_LISTING);
    }};

    public UnexpectedIntegerKeysBlockProcessor(String name, Map<String,Object> config) {
        super(name, configs);
    }

    @Override
    public Object process(StructuralNode parent, Reader reader, Map<String,Object> attributes) {
        for (String key : attributes.keySet()) {
            System.out.println(key + ": " + attributes.get(key));
        }
        return null;
    }
}

Test:

import org.asciidoctor.Asciidoctor;
import org.asciidoctor.OptionsBuilder;
import org.junit.Test;

import java.io.File;
import java.net.URISyntaxException;

import static org.asciidoctor.Asciidoctor.Factory.create;

public class UnexpectedIntegerKeysBlockProcessorTest {

    @Test
    public void attributesShouldOnlyContainStringKeys() {

        Asciidoctor asciidoctor = create();
        asciidoctor.javaExtensionRegistry().block("snippet", UnexpectedIntegerKeysBlockProcessor.class);

        try {
            File referenceFile =
                    new File(getClass().getClassLoader().getResource("unexpectedintegerkeys.adoc").toURI ());
            String referenceResult = asciidoctor.convertFile(referenceFile, OptionsBuilder.options()
                    .headerFooter( false )
                    .toFile( false ));
        } catch (URISyntaxException ex) {
            System.out.println( ex.getMessage() );
        }
    }
}

AsciiDoc source:

[snippet, java]
----
Java code.
----

[jjaderberg]

%s/Integer/Long/g

[mojavelinux]

You are correct that block attributes can have numeric keys. These are the positional attributes. Unfortunately, accommodating that scenario would mean a change in a lot of places. One solution is to coerce those numeric keys to strings at the boundary of Ruby and Java. The other is updating the signature of attributes on all nodes to be a <Object,Object> (though, technically, <Object,String> would work too since the value of block attributes can only be string or null).

[robertpanzer]

I think we have sorted it out in the 1.6.0 branch by skipping numeric attributes which should always be positional attributes.

[mojavelinux]

I think we have sorted it out in the 1.6.0 branch by skipping numeric attributes which should always be positional attributes.

I vaguely remember discussing that as well. I case did come up recently when accessing the positional attribute directly was necessary. However, I think if we decide we do want to still keep them, we will use string-based keys to map them back in so that we can stick with string-based keys. The numeric keys really should be considered an internal API.

[robertpanzer]

Oops, it's not sorted out.
Thank you for reporting this. I made some changes to take care of this, but apparently this slipped through.

Thank you for reporting!

[robertpanzer]

Fixed in a2967ea.

[mojavelinux]

👍

[jjaderberg]

Oh no! I mean, thanks for fixing quickly, but the fix makes things all the worse for me because it simply removes attributes with numeric keys rather than coerce them to Strings--so now I can't get at the positional attributes!

I can work around the original issue, but I don't know that I can work around the fix :) Would you consider coercing numeric keys to String instead of ignoring/removing them?

[robertpanzer]

Hi,

The current approach is to define the positional attributes as options to
the processor so that positional attributes are automatically mapped to
named attributes:
https://github.com/asciidoctor/asciidoctorj/blob/asciidoctorj-1.6.0/asciidoctorj-core/src/test/groovy/org/asciidoctor/extension/AnnotatedLongInlineMacroProcessor.groovy#L9

Cheers
Robert

Am Sonntag, 24. April 2016 schrieb jjaderberg :

Oh no! I mean, thanks for fixing quickly, but the fix makes things all the
worse for me because it simply removes attributes with numeric keys rather
than coerce them to Strings--so now I can't get at the positional
attributes!

I can work around the original issue, but I don't know that I can work
around the fix :) Would you consider coercing numeric keys to String
instead of ignoring/removing them?

—
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
#450 (comment)

[jjaderberg]

I see--yes, I can work with that. Thanks!