this should be done this with an if/else instead of catching an exception

This should return ByteBufs, and the Entry type should be a flyweight. We shouldn't need to allocate any objects when we read this

so ByteBuf[2]? in this case I'm thinking we can expose the mime buffer this way:

• if the mime is an encoded id, a 1 byte long ByteBuf that represents the still encoded id (raw slice from the original buffer, need to do further bit manipulation)
• if the mime is as custom string, a 2 to 129 bytes long ByteBuf, a slice of the underlying buffer that encompasses both the length byte and the mime string itself.

Since the mime string slide would have no more readable bytes than necessary to decode the string, the first byte could simply be discarded. Yet without it there would be a corner case ambiguity for 1 byte long buffer`, which can either be:

1. the case of a 1 byte encoded id
2. the case of a 1 byte long String

So reading the mime buffer is a matter of checking its size is 1 byte and applying the mask OR size is 2+ bytes and skipping 1 byte then reading as CharSequence.

Does that sound good?

note: the second ByteBuf would be a simple sliced of the original part that contains the metadata content, so its readableBytes reflect the size of the "content" (not including the encoded length)

We shouldn't need an Entry object to create an entry. This should be done with a method so we don't need to allocate an object.

i.e.
encodeIncrementally(ByteBufAllocator allocator, CompositeByteBuf compositeMetadataBuffer, String key, String value)

encodeIncrementally(ByteBufAllocator allocator, CompositeByteBuf compositeMetadataBuffer, WellKnownMimeTypes type, String value)

and so on

Checkout out how FrameType looks up FrameType via its code:
https://github.com/rsocket/rsocket-java/blob/develop/rsocket-core/src/main/java/io/rsocket/frame/FrameType.java#L209

You can pre-compute the values into an array so you can look up using an array index instead of a for loop

thanks for the pointer, done 👍

it looks like the same for the String based lookup could be improved with a static Map<String, WellKnownMimeType>:

Benchmark                                          Mode  Cnt         Score         Error  Units
WellKnownMimeTypePerf.fromStringMapLookup         thrpt   10  43906830.064 ± 1775994.988  ops/s
WellKnownMimeTypePerf.fromStringValuesLoopLookup  thrpt   10   2041048.116 ±   42704.445  ops/s

wdyt?

yeah - that's a big difference

Does this adjust the readIndex? One the RSocket frames do is not adjust the read index to the call because it leads to weird problems when use the frames else.

Also I thought the first bit not the first byte was what in was what determined length- so if the first bit is 0 do you need to skip anything?

it indeed increases the readerIndex(). the first bit encodes the flag wether or not the remaining 7 bits represent a length or a mime id (if I interpreted the spec correctly, cc @nebhale). But since the previous step in decoding produced a buffer slice that has exactly stringMimeTypeLength + 1 readableBytes(), we can assume this whole byte is A) starting with 1 and B) irrelevant to decode again.

The idea behind including this extra byte in the buffer was to avoid ambiguity between 2 possible 1-byte cases: 1 byte representing a compressed mime id vs 1 byte representing a 1-char mime string. By including it, the minimum length for the string case is 2 bytes, and thus we can differentiate by simply looking at readableBytes().

note that I assumed it was ok to move the readerIndex() when decoding and slicing?
(I rely on said index having moved when further decoding the next entry in the composite metadata buffer)

Generally we've found its not okay to leave it changed once you're done with the flyweight. If you look at the RSocket frames they all reset the index afterwards to leave the bytebuf in the same state they found it.

It's probably easier to use ByteBufUtil.writeAscii(allocator, customMime); to create the mimeBuffer

I pushed a change that simplifies that encode method, avoiding the use of a CompositeByteBuf. But I think UTF8 writing is still required to detect non-ASCII chars, as writeAscii silently removes such characters (see commit message)

Where is the flyweight for the items in this array?

only the first of the two buffers needs to be decoded (IF it exists, as the array can be empty in case of malformed composite), and the methods to do so are in the same CompositeMetadataFlyweight:

• decodeMimeIdFromMimeBuffer in case readableBytes() == 1
• decodeMimeTypeFromMimeBuffer otherwise

The way I was thinking about doing this was Iterator that kept track of indexes and then a flyweight you would place over the ByteBuf slices the iterator emitted. You'd have an EntryFlyweight with a couple static methods.

Ok I see - the CompositeMetadata is the flyweight for an indiviual entry(?). How do you tell the demaraction between CompositeMetadata? Like if you have 8 metadatas and you want to iterator over them?

that's where the new CompositeMetadata comes in, with hasNext / decodeNext. alternatively the flyweight method returns an empty array when all was decoded

ok I see - you mutate the read index. That doesn't work like any of the other frames/payloads in RSocket. They all reset there read indexes are finished- they change the state of the ByteBuf to back where it was. We've found it's a lot easier to deal with ByteBufs that way, and update the read indexes as a side-effect makes the code harder to debug.

does that apply to slices? decodeMime[Type|Id]FromMimeBuffer both work from a slice of the original whole metadata buffer, so that skipBytes(1) in decodeMimeTypeFromMimeBuffer doesn't move the readerIndex of the whole composite metadata...

That said, the main decoding method does indeed move the full metadata buffer's readerIndex(). I thought it was unavoidable since the approach was to decode a subset of the whole buffer, without state keeping, and the length of the slices (metaheader and metacontent) can't be known without decoding.

But maybe that state can be externalized, as a startFromIndex parameter to decodeMimeAndContentBuffers? It would be a burden put on the user to keep track of the next startFromIndex to read from, though... She would have to do something like index = index + slices[0].readableBytes() + slices[1].readableBytes() + 1... (see below for comment on edit)

This can of course be done inside the CompositeMetadata "iterator", but for low level decoding it can be more work.

note The best way to compute the next entry index is to provide a flyweight util method, since the decoded slices are not necessarily covering contiguous portions of the orignal full metadata buffer (namely currently they don't include the 3 bytes content length)