Missing flags getter added

[jpstotz]

The HeaderParser class is pretty handy when using org.eclipse.jetty.http2:http2-common as a library in other projects. Unfortunately it misses a getter to retrieve the flags and there are scenarios where you don't want to call hasFlag for every possible flag...

[sbordet]

I'm not sure I understand in what scenario would the getter help?

You would have to get the int and then & it with every possible flag?

You typically don't need this because in the right places you look only for certain flags.
For example you don't look for Flags.END_STREAM if you're dealing with SETTINGS frames, etc.

[jpstotz]

@sbordet I need to have access to the raw flags value. At the moment I have to use reflection to do so just to get it work.
May be there are implementations which use non-official flags or the standard has changed or something else is different.
In the end it is way more safe to have the raw value instead of checking each possible flag.

Therefore having a getter in place is IMHO always a good idea. I don't see a point here why HeaderParser should hide the flags field. There is no advantage or requirement to hide it.

[sbordet]

I need to have access to the raw flags value.

Why?

There is no advantage or requirement to hide it.

There is no advantage to expose it that you have articulated, besides "I have to access it".

The flags are used as a bitmap, so it is very easy to get confused and write code such as getFlags() == Flags.END_STREAM, which is wrong.
That's why it is hidden.

[jpstotz]

@sbordet For example I want to print the flags value in it's original form.

Just hiding a field because developers exists who does not understand how bitwise flags work is IMHO a bad decision.

In my opinion a better way to prevent an API from being used in a wrong way is by adding a detailed description.
Would you finally agree to add a getter if it would contain a JavaDoc comment?

[gregw]

@sbordet The information is already available via hasFlag, so the flags value can always be recovered with something like

   return
     (p.hasFlag(0x01) ? 0x01 : 0) |
     (p.hasFlag(0x02) ? 0x02 : 0) |
     (p.hasFlag(0x04) ? 0x04 : 0) |
     (p.hasFlag(0x08) ? 0x08 : 0) |
     (p.hasFlag(0x80) ? 0x10 : 0) |
     (p.hasFlag(0x80) ? 0x20 : 0) |
     (p.hasFlag(0x80) ? 0x40 : 0) |
     (p.hasFlag(0x80) ? 0x80 : 0) ;

So we are not hiding the data and if there is a user that wants them collectively, then I see no harm providing that.

@jpstotz javadoc would be helpful, specially as it could have a reference to the Flags class

[gregw]

@sbordet nudge, I'm not seeing the issue with a simple getter like this?