Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error with retrieving key of SinkRecords #115

Open
subkanthi opened this issue May 13, 2023 · 3 comments
Open

Error with retrieving key of SinkRecords #115

subkanthi opened this issue May 13, 2023 · 3 comments
Labels
enhancement New feature or improved functionality.

Comments

@subkanthi
Copy link
Contributor

subkanthi commented May 13, 2023

Currently if the key is a byte array and if its not UTF-8 encoded, the key is not retrieved.

    private String getKeyString(SinkRecord record) {
        String keyString = null;
        final Object recordKey = record.key();
        if (recordKey != null) {
            if (recordKey instanceof byte[] && ByteArrayUtils.isUTF8Encoded((byte[]) recordKey)) {
                keyString = new String((byte[]) recordKey, StandardCharsets.UTF_8);
            } else if (recordKey instanceof String) {
                keyString = (String) recordKey;
            }
        }
        return keyString;
    }

┆Issue is synchronized with this Jira Story by Unito

@sync-by-unito
Copy link

sync-by-unito bot commented May 13, 2023

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3579

@jaley
Copy link
Contributor

jaley commented Jun 8, 2023

@subkanthi - that's a private method in ConfigValueEvaluator, and if it returns null, the calling code checks to see whether the key is referenced in the mapping string. If it is, then that's an error, because we need to be able to interpret the key as a string in order to use it in the channel mapping. There's a config to optionally skip records with keys we can't use.

Arguably, the code should check whether we're using #{key} in the mapping before extracting a key, as it's a waste to check every record to see if it's UTF-8 if it's not being used anyway. We could potentially also support other string encodings, like UTF-16.

What were you finding is broken by the above code? What's the expected behaviour?

@subkanthi
Copy link
Contributor Author

@jaley

recordKey would not meet the if and else if conditions, its a byte[] array but not UTF8 encoded and so the keyString will be null.

            if (recordKey instanceof byte[] && ByteArrayUtils.isUTF8Encoded((byte[]) recordKey)) {
                keyString = new String((byte[]) recordKey, StandardCharsets.UTF_8);
            } else if (recordKey instanceof String) {
                keyString = (String) recordKey;
            }

@subkanthi - that's a private method in ConfigValueEvaluator, and if it returns null, the calling code checks to see whether the key is referenced in the mapping string. If it is, then that's an error, because we need to be able to interpret the key as a string in order to use it in the channel mapping. There's a config to optionally skip records with keys we can't use.

Arguably, the code should check whether we're using #{key} in the mapping before extracting a key, as it's a waste to check every record to see if it's UTF-8 if it's not being used anyway. We could potentially also support other string encodings, like
UTF-16.

What were you finding is broken by the above code? What's the expected behaviour?

@mattheworiordan mattheworiordan added the enhancement New feature or improved functionality. label Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improved functionality.
Development

No branches or pull requests

3 participants