Duplicate response processing causes schema corruption

[Vitaliy Kurokhtin (VitaliyKurokhtin)]

No description provided.

[PerthCharern]

Vitaliy Kurokhtin (@VitaliyKurokhtin)

Any chance you have an example handy to illustrate what exactly is turning out incorrectly?

I believe Darrel (@darrelmiller) added this change to allow reading produces if it comes after responses, but seems like it's causing some kind of regression. (#267)

[Vitaliy Kurokhtin (VitaliyKurokhtin)]

I'll add a test, but in short the problem is that ProcessProduces fetches schema by context.GetFromTempStorage(TempStorageKeys.ResponseSchema) which is, if responses are already processed, the last schema. And that is the schema that gets assigned to all responses.

[Vitaliy Kurokhtin (VitaliyKurokhtin)]

There. Test added, implementation fixed. I feel that previous implementation was wrong to assume that ParseProduces may be called at any point during parsing, since list of produces media types may be not available when we parse responses.

[Darrel (darrelmiller)]

I had made this change because I believe one of the tests in APIM had the produces at the end of the response. I'll review this change and provide feedback.

[Darrel (darrelmiller)]

I tested this before and after the fix and was able to repro the problem and confirm the fix solves it. Much appreciated Vitaliy Kurokhtin (@VitaliyKurokhtin) !

[PerthCharern]

Thanks Darrel (@darrelmiller) & Vitaliy Kurokhtin (@VitaliyKurokhtin)

Vitaliy Kurokhtin (@VitaliyKurokhtin) Could you help resolve the merge conflict by removing all the tags and verify that there is no impact on your code and tests? Once you confirm that, I'll check in the code. Thank you for this fix :)

[Vitaliy Kurokhtin (VitaliyKurokhtin)]

Sure, done!

[Tobin den Harder (tobindh)]

There is an issue with this fix if there are multiple produces. It clears the temp storage for the responseSchema in temp storage inside of the produces foreach, so if there are multiple content types (i.e. xml and json), only the first content type gets the schema assigned. Shouldn't this clear the temp storage outside of the produces foreach?

src/Microsoft.OpenApi.Readers/V2/OpenApiResponseDeserializer.cs in the ProcessProduces method.

[Darrel (darrelmiller)]

Tobin den Harder (@tobindh) Moving the clear to outside the Produces loop solves the problem with multiple produces, but it then breaks the issue with multiple responses. Not sure why. I will continue investigating.

[Vitaliy Kurokhtin (VitaliyKurokhtin)]

Tobin den Harder (@tobindh) fixed by #398