fix "OTSParameterInvalid Expiration time is invalid when enableStream is false" bug

[dayzhou]

Given that EnableStream = true, now to disable stream, when calling

UpdateTable(&tablestore.UpdateTableRequest{
  TableName: "...",
  StreamSpec: &tablestore.StreamSpecification{
    EnableStream: false,
  },
})

the following error is returned:

OTSParameterInvalid Expiration time is invalid when enableStream is false

I guess the reason is that the server side is doing some naive checking, e.g.,

if !req.StreamSpec.EnableStream {
  if req.StreamSpec.ExpirationTime != nil {
    return error
  }
}

And sorry, I only added 3 or 4 lines, but my editor gofmt a little bit, so it looks like there's a lot of changes.

[CLAassistant]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

Da Zhou seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coveralls]

Coverage decreased (-29.6%) to 62.818% when pulling db6757afe1bf674e5200e9dcdd6fbc8af0dec83f on dayzhou:fix-disable-stream into e824b971235a0dc587e92a71632641acfbb59e7f on aliyun:master.

[coveralls]

Coverage decreased (-29.6%) to 62.818% when pulling db6757afe1bf674e5200e9dcdd6fbc8af0dec83f on dayzhou:fix-disable-stream into e824b971235a0dc587e92a71632641acfbb59e7f on aliyun:master.

[danielxiaoran]

Server is not allowed to set false with ExpirationTime. I think it's not a good way to check it in sdk side and throw the ExpirationTime if you set false.

[danielxiaoran]

by the way if you have any issue about tablestore, you can join our dingding discussion group. Thanks.

[dayzhou]

Yes, I agree with you that it should be the server side doing the validation. But I think I didn't make myself clear. I'm not throwing any ExpirationTime error in my PR, I simply mean that there is no way to close/disable the stream in the current Go API because the server side isn't doing the right validation.

Here goes the details, when trying to disable a stream by calling tablestore.UpdateTable with tablestore.UpdateTableRequest, you set the its StreamSpec field literally:

updateTableRequest.StreamSpec = &tablestore.StreamSpecification{
  EnableStream: false,
}

then in the UpdateTable method (line 404 in api.go), you convert it to otsprotocol.StreamSpecification by taking the addresses:

req.StreamSpec = &otsprotocol.StreamSpecification{
  EnableStream:   &request.StreamSpec.EnableStream,
  ExpirationTime: &request.StreamSpec.ExpirationTime}

so the otsprotocol.StreamSpecification.ExpirationTime field is alway a non-nil pointer. However, (I found this by trial and error) the server side only accepts a nil otsprotocol.StreamSpecification.ExpirationTime field if EnableStream == false.

So unless the server side is doing the right checking, the client side logic has to change.

[dayzhou]

by the way, I have pushed a new commit which removes the formatting part made by the gofmt tool. Maybe it's easier to see what changes I've made

[coveralls]

Coverage decreased (-29.6%) to 62.778% when pulling e0edec567fa993be6715a9867e4ae53e12899142 on dayzhou:fix-disable-stream into e824b971235a0dc587e92a71632641acfbb59e7f on aliyun:master.

[coveralls]

Coverage decreased (-29.6%) to 62.778% when pulling e0edec567fa993be6715a9867e4ae53e12899142 on dayzhou:fix-disable-stream into e824b971235a0dc587e92a71632641acfbb59e7f on aliyun:master.

[danielxiaoran]

if request.StreamSpec != nil {

•  var expirationTime *int32
  

•  if request.StreamSpec.EnableStream {
  

•  	expirationTime = &request.StreamSpec.ExpirationTime
  

•  }
   req.StreamSpec = &otsprotocol.StreamSpecification{		  		req.StreamSpec = &otsprotocol.StreamSpecification{
   	EnableStream: &request.StreamSpec.EnableStream,		  			EnableStream: &request.StreamSpec.EnableStream,
  

•  	ExpirationTime: &request.StreamSpec.ExpirationTime}		 +			ExpirationTime: expirationTime,
  

•  }
  

  } }

是不是改错了？

你可以加我们钉钉直接沟通快一些哈

[dayzhou]

我的项目现在用这段fix后的代码能正常Disable stream的，fix之前的UpdateTable()不能disable stream。

我本来想的是UpdateTableRequest里把EnableStream设为false就行了，结果发现你们的服务端总是会返回错误，后来发现是因为当EnableStream为false的时候，你们的服务器只接受nil的otsprotocol.StreamSpecification.ExpirationTime，但你们提供的API传的ExpirationTime永远非nil。我做的改动就是先检查一下EnableStream是否为false，如果是，ExpirationTime字段就只传一个nil指针，否则就还用你们原来的方式。就这一处小小的改动..

你可以先试试你们现在的go API能不能关闭stream，如果可以那估计是我调用的方式不对，如果不能再用我的fix试试。我这边没装钉钉，你可以加我微信详聊：z15955156619