TestCase_S13_TC05_PutChanges_MappingExist_ImplyFlagOne

[gabor-v]

Hi!

@ChangDu2021 In MS-FSSHTTPB - 2.2.2.1.4 at the end of this part ’Expected Storage Index Extended GUID’ I found the following about the ’Imply Null Expected if No Mapping’ flag:
„If the flag specifies one and a mapping exists, the protocol server MUST return a Cell Error failure value of 12 indicating a coherency failure, as specified in section 2.2.3.2.1.”

But at the end of TestCase_S13_TC05_PutChanges_MappingExist_ImplyFlagOne (SharedTestCase/S13_PutChanges.cs) it asserts success instead of coherencyFailure (comment says there: „expect the server responds the Coherency failure error”).
Which errorCode is the correct in this case?

Thank you!

[tomjebo]

@ChangDu2021, I've looked at this from the specification and from our code and it looks like TestCase_S13_TC05_PutChanges_MappingExist_ImplyFlagOne maybe should not supply the ExpectedStorageIndexExtendedGUID if it's really expecting coherencyfailure error as the comments say:

            // Assign the ImplyNullExpectedIfNoMapping to 1.
            putChange.ImplyNullExpectedIfNoMapping = 1;
            putChange.ExpectedStorageIndexExtendedGUID = storageIndex;

            // Put changes to the protocol server to expect the server responds the Coherency failure error.
            CellStorageResponse response = Adapter.CellStorageRequest(this.DefaultFileUrl, new SubRequestType[] { cellSubRequest });
            CellSubResponseType cellSubResponse = SharedTestSuiteHelper.ExtractSubResponse<CellSubResponseType>(response, 0, 0, this.Site);

            this.Site.Assert.AreEqual(ErrorCodeType.Success, SharedTestSuiteHelper.ConvertToErrorCodeType(cellSubResponse.ErrorCode, this.Site), "The PutChanges operation should succeed.");

But I hope you can check this out and confirm.

[tomjebo]

@gabor-v (and @ChangDu2021) actually, I see that TestCase_S13_TC06_PutChanges_NotSpecifiedExpectedStorageIndexExtendedGUID_ImplyFlagOne does the correct sequence to check for Coherencyfailure. I didn't notice it before because I was just looking at TC05.

So it looks like TC05 test the success case and TC06 tests the failure case.

Can you confirm?

[tomjebo]

@gabor-v, to elaborate on this, I think TC05 is testing this case from 2.2.2.1.4 (see clauses enclosed in "**"):

**If the expected Storage Index was specified and the key that is to be updated in the protocol
server’s Storage Index exists in the expected Storage Index, the corresponding values in the
protocol server’s Storage Index and the expected Storage Index MUST match;** otherwise, the
protocol server MUST return a Cell Error failure value of 12 indicating a coherency failure, as
specified in section 2.2.3.2.1. If the values match, the protocol server MUST ensure that the
Storage Index update is atomically updated such that no other update is overwritten.

and TC06 is testing this case:

If the expected Storage Index was not specified or the key that is to be updated in the protocol
server’s Storage Index does not exist in the expected Storage Index, the Imply Null Expected if
No Mapping flag MUST be evaluated. If this flag is zero, the protocol server MUST apply the
change without checking the current value; otherwise, if the flag specifies one, the protocol server
MUST only apply the change if no mapping exists (the key that is to be updated in the protocol
server’s Storage Index doesn’t exist or it maps to nil). **If the flag specifies one and a mapping
exists, the protocol server MUST return a Cell Error failure value of 12 indicating a coherency
failure, as specified in section 2.2.3.2.1.**

[gabor-v]

@tomjebo Thank you, that would make sense for me. I think the comments vs code confused me in TC05 originally. It seems to me it can be a problem in TC06 as well. Otherwise for me this issue would be closeable, but before I do that I wait for your answer(s) first.

[ChangDu2021]

@gabor-v TC06 tests the part at the end of ’Expected Storage Index Extended GUID’, "If the flag specifies one and a mapping exists, the protocol server MUST return a Cell Error failure value of 12 indicating a coherency failure, as specified in section 2.2.3.2.1."

Below is the content in RS file about R941.
ns1:Requirement
ns1:REQ_IDMS-FSSHTTPB_R941</ns1:REQ_ID>
ns1:Doc_Sect2.2.2.1.4</ns1:Doc_Sect>
ns1:Description[In Put Changes structure] Expected Storage Index Extended GUID (variable): [if Imply Null Expected if No Mapping flag specifies 1,] If the flag specifies one and a mapping exists, the protocol server MUST return a Cell Error failure value of 12 indicating a coherency failure as specified in section 2.2.3.2.1.</ns1:Description>
ns1:BehaviorProtocol</ns1:Behavior>
ns1:ScopeServer</ns1:Scope>
ns1:IsNormativeNormative</ns1:IsNormative>
ns1:VerificationTest Case</ns1:Verification>
</ns1:Requirement>

[gabor-v]

@ChangDu2021 Thanks, it seems we all agree now about what the intention of both test methods are. The only open part left is the comments in test code. For example:

• TC06 tells me two times about 'flag set to zero' (rows 342 and 351), but uses in code flag set to one.
• TC05 tells me in row 333: 'expect the server responds the Coherency failure error', but asserts success.

[tomjebo]

@tomjebo Thank you, that would make sense for me. I think the comments vs code confused me in TC05 originally. It seems to me it can be a problem in TC06 as well. Otherwise for me this issue would be closeable, but before I do that I wait for your answer(s) first.

Yes, I think the comments could be updated to be more clear or correct.

[tomjebo]

Below is the content in RS file about R941.

Yes, thanks. The TC05 case was the one I didn't see a requirement for though. And both will need modified comments I think.

[ChangDu2021]

@gabor-v For TC06, thank you to point out the error in the comments. I agree you about the error in row 342. But for row 351, I think it is correct because ImplyNullExpectedIfNoMapping is set to 0 in function PutChangesCellSubRequest. Then, in row 357, it is changed to 1.

// Create a putChanges cellSubRequest without specified Expected Storage Index Extended GUID attribute and Imply Null Expected if No Mapping flag set to zero FsshttpbCellRequest cellRequest = SharedTestSuiteHelper.CreateFsshttpbCellRequest(); ExGuid storageIndexExGuid; List<DataElement> dataElements = DataElementUtils.BuildDataElements(System.Text.Encoding.Unicode.GetBytes(Common.GenerateResourceName(this.Site, "FileContent")), out storageIndexExGuid); PutChangesCellSubRequest putChange = new PutChangesCellSubRequest(SequenceNumberGenerator.GetCurrentFSSHTTPBSubRequestID(), storageIndexExGuid);

`public PutChangesCellSubRequest(ulong subRequestID, ExGuid storageIndexExGuid)
{
this.RequestID = subRequestID;
this.RequestType = Convert.ToUInt64(RequestTypes.PutChanges);
this.StorageIndexExtendedGUID = storageIndexExGuid;
this.ExpectedStorageIndexExtendedGUID = new ExGuid();
this.ImplyNullExpectedIfNoMapping = 0;
this.Partial = 0;
this.PartialLast = 0;
this.FavorCoherencyFailureOverNotFound = 1;
this.AbortRemainingPutChangesOnFailure = 0;
this.MultiRequestPutHint = 0;
this.ReturnCompleteKnowledgeIfPossible = 1;
this.LastWriterWinsOnNextChange = 0;

        this.IsAdditionalFlagsUsed = false;
        this.IsLockIdUsed = false;
        this.IsDiagnosticRequestOptionInputUsed = false;
    }`

[gabor-v]

@ChangDu2021 Thanks, I agree. I think the method name and the comment row 350 confused me first, but the subRequest is set through more steps indeed.