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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record.Create factory function does not initialise Headers - Results in Null reference exception if trying to Access Headers #316

Open
dave-signify opened this issue Apr 1, 2024 · 1 comment

Comments

@dave-signify
Copy link

dave-signify commented Apr 1, 2024

Description

Hello!

First off, thanks for making this super lib @LGouellec and I hope you had a nice weekend! I love to see succesful .NET ports flourishing, and I'm enjoying using it 馃挴 :-).

Please forgive me if I'm going about this the wrong way. However, I believe I may have found an issue with the Create factory function in the Record class. I discovered this when trying to write a unit test for the function I am passing into the .Transform function as part of the stream topology creation. I understand this may be a purposeful design choice to not initialise the headers when calling Create via the Record public API, so if there's another way of going about mocking or stubbing out a Record class including setting the Headers from within a unit test process, that would be ideal.

How to reproduce

Simply call the Create factory function on the Record class as shown below:

var aRecord = Record<string,string>.Create("some-key", "some-value");

Now try to call the Headers.Add function:

aRecord.Headers.Add(new Header("some-header-key", Encoding.ASCII.GetBytes("some-header-value")));

The above call to aRecord.Headers.Add will throw an unhandled NullReferenceException. This is preventing me from writing my unit test where I would like to create my own Record with some headers in my test.

I believe the cause of the issue is here

private Record(K key, V value)
where the public factory function calls this private constructor, but the constructor doesn't initialise the Headers property.

I suppose the question is, is this a bug? Or is this a design choice? If it's a design choice is there another way I should be creating instances of Records in my unit tests?

If this is a bug @LGouellec I'd be happy to submit a Pull request for your review! :-)

Thank you! :)

Streamiz nuget version: 1.5.1
Mac OS Sanomoa 14.3.1
Kafka v: latest docker image (local testing)
Not a critical issue - testability issue more so

@LGouellec
Copy link
Owner

Hey @dave-signify ,

Sorry for the late reply, I confirm it's a bug/lack.
There is a workaround :

        private class MyTransformer : ITransformer<string, string, string, string>
        {
            private ProcessorContext<string,string> context;

            public MyTransformer()
            {
            }
            
            public void Init(ProcessorContext<string, string> context)
            {
                this.context = context;
            }

            public Record<string, string> Process(Record<string, string> record)
            {
                var newRecord = Record<string, string>.Create(record.Key, record.Value.ToUpper());
                var newHeaders = new Headers();
                newHeaders.Add("newHeader", Encoding.UTF8.GetBytes("value"));
                context.SetHeaders(newHeaders);
                return newRecord;
            }

            public void Close()
            {
                
            }
        }

But feel to submit a PR to support this feature at the Record class level

  • Add a new Headers parameters in the Record<string,string>.Create factory
  • Forward the new headers in the downstream processor here via the Context.SetHeaders();method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants