Fix AlarmStateInitializer to store most recent kafka records from the alarm state topic

[jbellister-slac]

Fixes #3078

Change the end index of the substring() call to 6 so that it will be possible for type to equal the AlarmSystem.STATE_PREFIX in the equality check.

After I made this change, confirmed that acknowledgements do persist between alarm server reboots.

Although the change is minor this does cause this code block to be hit that wasn't possible before. Reading through the code it looks ok to me, and testing showed things seemed to be working as expected.

The eventual change this leads to outside this class is that initial_states will have something to pass to the AlarmServerPV here:

phoebus/services/alarm-server/src/main/java/org/phoebus/applications/alarm/server/ServerModel.java

Line 394 in 6a6c5be

final AlarmServerPV pv = new AlarmServerPV(this, parent.getPathName(), name, initial_states.remove(path));

[kasemir]

What you propose looks better, but while you're at it, how about this, which is overall shorter

for (final ConsumerRecord<String, String> record : records)
{
    // Only handle state updates
    if (record.key().startsWith(AlarmSystem.STATE_PREFIX))
    {
      final String path = record.key().substring(6);
      ...
    }
}

[shroffk]

if (record.key().startsWith(AlarmSystem.STATE_PREFIX))

+1 in addition to the small improvement in not having to create a new String object this is also much easier to read

[jbellister-slac]

Thanks! Agreed that it is easier to read this way. Change made and confirmed it still works as expected.

[kasemir]

Also, one might wonder why "6" in here:

if (record.key().startsWith(AlarmSystem.STATE_PREFIX))
{
    final String path = record.key().substring(6);

It's because AlarmSystem.STATE_PREFIX happens to contain "state:" and we want to get what's after that.
It'll break if we ever changed the value of the state prefix to say "status:", and now we'd have to skip 7 chars.

This would be safer:

if (record.key().startsWith(AlarmSystem.STATE_PREFIX))
{
    final String path = record.key().substring(AlarmSystem.STATE_PREFIX.length());

... but now one could argue that just "6" is faster than calling String.length().
It would certainly hurt in C where strlen(some_string) actually counts all the chars for each call.
With Java strings, I guess it only fetches one number, so quite fast and certainly safer than hoping the length of AlarmSystem.STATE_PREFIX is and remains 6.

[kasemir]

Thanks, that looks better.
And if it actually works, that'd be perfect!

[kasemir]

Thanks for looking into this!