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

What is the point to declare internal abstract methods in the public abstract class that no-one will be able to use? #977

Closed
hovsepm opened this issue Nov 6, 2019 · 8 comments · Fixed by #978
Assignees
Labels
bug Something isn't working QUERY

Comments

@hovsepm
Copy link

hovsepm commented Nov 6, 2019

We are continuously addressing and improving the SDK, if possible, make sure the problem persist in the latest SDK version.

Describe the bug
We use Cosmos SDK v3.2 and want to upgrade to v3.4 but since new version introduces internal abstract method in public abstract class FeedIterator<T> there is no way to compile the code anymore. We do implement FeedIterator class in our Mock infrastructure for testing purposes but now the new version throws 'MockCosmosQuery<T>' does not implement inherited abstract member 'FeedIterator<T>.TryGetContinuationToken(out string)' error while if we try to implement missing method it will now complain that 'MockCosmosQuery<T>.TryGetContinuationToken(out string)': no suitable method found to override. Could you please make TryGetContinuationToken method public in the public abstract class?

To Reproduce
Steps to reproduce the behavior. If you can include code snippets or links to repositories containing a repro of the issue that can helps us in detecting the scenario it would speed up the resolution.

Expected behavior
Give some alternative how to implement FeedIterator<T> class

Actual behavior
Unable to upgrade to v3.4 and have out test infrastructure working.

Environment summary
SDK Version: 3.4.
OS Version : Windows

Additional context
Please think how your library may be used by external developers before marking methods internal.

@j82w
Copy link
Contributor

j82w commented Nov 6, 2019

This is a bug the internal method should not be on the public type. We missed this on the PR. I'll work on a fix.

@j82w
Copy link
Contributor

j82w commented Nov 6, 2019

The PR is out. I'll try to get a hotfix out tomorrow. There is a work item to include public samples as part of the test projects to prevent this in the future.

@j82w j82w self-assigned this Nov 6, 2019
@j82w j82w added bug Something isn't working QUERY labels Nov 6, 2019
@j82w
Copy link
Contributor

j82w commented Nov 6, 2019

The reason there is internal is the SDK is used by internal parts of the service that have custom requirements. The internal projects have internal visible to so it can access the internal methods. The internal types should not impact the public types. There is now a unit test to avoid this type of breaking change in the future.

@hovsepm
Copy link
Author

hovsepm commented Nov 6, 2019

Thanks for quick fix.

@j82w
Copy link
Contributor

j82w commented Nov 7, 2019

@hovsepm the 3.4.1 is out. Please let me know if there is any issues.

@hovsepm
Copy link
Author

hovsepm commented Nov 7, 2019

@j82w already referenced and confirming that it works. Thank for the fix!

@kellyprankin
Copy link

We are continuously addressing and improving the SDK, if possible, make sure the problem persist in the latest SDK version.

Describe the bug
We use Cosmos SDK v3.2 and want to upgrade to v3.4 but since new version introduces internal abstract method in public abstract class FeedIterator<T> there is no way to compile the code anymore. We do implement FeedIterator class in our Mock infrastructure for testing purposes but now the new version throws 'MockCosmosQuery<T>' does not implement inherited abstract member 'FeedIterator<T>.TryGetContinuationToken(out string)' error while if we try to implement missing method it will now complain that 'MockCosmosQuery<T>.TryGetContinuationToken(out string)': no suitable method found to override. Could you please make TryGetContinuationToken method public in the public abstract class?

To Reproduce
Steps to reproduce the behavior. If you can include code snippets or links to repositories containing a repro of the issue that can helps us in detecting the scenario it would speed up the resolution.

Expected behavior
Give some alternative how to implement FeedIterator<T> class

Actual behavior
Unable to upgrade to v3.4 and have out test infrastructure working.

Environment summary
SDK Version: 3.4.
OS Version : Windows

Additional context
Please think how your library may be used by external developers before marking methods internal.

Where are you finding this MockCosmosQuery? I need something like this.

@j82w
Copy link
Contributor

j82w commented Nov 18, 2019

FeedIterator<T> is an abstract class. You can extend it to do testing or use it to create a Mock. MockCosmosQuery is hovsepm custom class they created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working QUERY
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants