Feat/filter control blocks

[ElijahSwiftIBM]

💡 Issue Reference

Issue: #5

💻 What does this address?

Control block filtering is a valuable enhancement. In basic cases it allows for simple validation that a control block containing the right data is collected, but in more complex cases, it allows users to grab information on just a singular relevant control block or collection of them in a large table.

📟 Implementation Details

The core control block class now has a new unordered map for filtration that is built on object creation. The filter is passed from block to block along with inclusion patterns and validated along the way. At the "end" of the line, the control block's value is validated against that of the filter. In the event of a match, the json data is returned, otherwise an empty json comes back. Large lists of control blocks like ASCB's and ASSB's were edited to filter out these null values, allowing for a true filter.

📋 Is there a test case?

I added the following tests in both the python and the shell test suites (the names in python are fare more descriptive)

test_cbxp_raises_cbxp_error_if_filter_uses_non_included_control_block
test_cbxp_raises_cbxp_error_if_filter_uses_unknown_key
test_cbxp_raises_cbxp_error_if_filter_dict_wrong_size (python only)
test_cbxp_raises_cbxp_error_if_filter_value_not_string (python only)
test_cbxp_raises_cbxp_error_if_no_filter_match
test_cbxp_can_use_basic_filter
test_cbxp_can_use_include_filter_with_generic_include
test_cbxp_can_use_include_filter_with_discrete_include

[lcarcaramo]

DCO signoff seems to be missing on all commits. Maybe consider squashing all of these commits into one commit that has a DCO signoff?

[lcarcaramo]

Somewhat unrelated, but I think we should remove the trailing spaces from extracted string fields in control blocks. Now that we are extracting more string fields, I am now noticing this more:

"assbjbns": "BPXAS   ",

[ElijahSwiftIBM]

Somewhat unrelated, but I think we should remove the trailing spaces from extracted string fields in control blocks. Now that we are extracting more string fields, I am now noticing this more:

"assbjbns": "BPXAS   ",

I'm not opposed to that, but I would prefer to keep that out of the scope for this change since we are close to the end of the review for this PR.

[lcarcaramo]

Thinking about it now, this still isn't thread safe because of the static cbxp_result_t. You could potentially have a use after free if CBXP is being called concurrently in multiple threads due to one thread potentially reading a result JSON string from the static cbxp_result_t structure and another thread already freed the buffer before the first thread tries to read it.

Maybe have a separate cbxp_free() to free the pointer and and unlock the mutex? Then I think the API will be fully thread safe. The user will have to explicitely call cbxp_free() for subsequent calls to be made, with the assumption that everything that needed to be read was read before calling cbxp_free(). OpenSSL uses a similar convention to this: https://docs.openssl.org/master/man3/

Also, I think you can use std::mutex here rather than pthread_mutex_t since this is C++ code: https://www.geeksforgeeks.org/cpp/std-mutex-in-cpp/

[lcarcaramo]

Add debug message indicating that a filter is being added to the options map?

[lcarcaramo]

Add debug message indicating that a filter is being added to the current filters?

[lcarcaramo]

Add debug messages indicating the comparison that is being performed?

[lcarcaramo]

Add debug messages indicating the result and the reason for the result?

[lcarcaramo]

For the Python interface, it might be cleaner to have maybe a CBXPFilter object and a CBXPFilterOperation enum like the following:

class CBXPFilterOperation(Enum):
    EQUAL = "="
    LESS_THAN = "<"
    GREATER_THAN = ">"
    LESS_THAN_OR_EQUAL = "<="
    GREATER_THAN_OR_EQUAL = ">="

class CBXPFilter:
    def __init__(self, key: str, operation: CBXPFilterOperation, value: Union[str,int]):
        self.key = key
        self.operation = operation
        self.value = value

Then the user would use filters something like this:

from cbxp import cbxp, CBXPFilter, CBXPFilterOperation

filters = [
    CBXPFilter(
        "assb.assbjbni", 
        CBXPFilterOperation.EQUAL,
        "IBMUSER"
    ),
    CBXPFilter(
        "assb.assbjbns",
        CBXPFilterOperation.EQUAL,
        "BPXAS"
    ),
    CBXPFilter(
        "ascbasid",
        CBXPFilterOperation.LESS_THAN_OR_EQUAL,
        50
    )
]

cbdata = cbxp("ascb", includes=["assb"] filters=filters)

We would still have to make sure the key and value don't contain any commas, but I think this would be a little bit cleaner than providing a key-value pair string. Instead we would build the key-value pair strings dynamically from the provided CBXPFilter objects. It is a little verbose, but given that the Python interface is intended for more complex automation scenarios rather than just trying to extract control block data on demand like you would with the shell interface, I think this makes sense.

[lcarcaramo]

Somewhat unrelated, but I think we should remove the trailing spaces from extracted string fields in control blocks. Now that we are extracting more string fields, I am now noticing this more:

"assbjbns": "BPXAS   ",

I'm not opposed to that, but I would prefer to keep that out of the scope for this change since we are close to the end of the review for this PR.

If you want to take a look, I think all you would have to do is add a little code for trimming the trailing spaces in ControlBlockFieldFormatter::getString()

cbxp/cbxp/control_blocks/control_block_field_formatter.hpp

Line 25 in 1ddbd29

return ascii_field_unique_ptr.get();