* Fix serialization tests & don't block thread
- Fixed potential null ref when closing streams
- Always clean up serialization queue if an error occurs
- Stop blocking dispatcher thread by not awaiting task that processes the message
- Improved error logging in EventFlowValidator to help debug issues
- Close stream on exception
Added a method that handles serialization requests
Support splitting save over multiple requests to reduce overall message size
Added unit tests
String changes used a new version of the string tool for generation.
Will publish PR separately for the changes to build & localization
so this can run on Mac without .Net Core 1.0
* Remove .Net Core 1.0 requirement for Loc and fix mac build
Use StringResourceTool to remove .Net Core 1.0 dependency for Loc
Fixed Mac build by copying the code used in internal repo for build.
- This now mostly matches internal, so expect this should work OK.
* Remove XUnit warnings which clogged up output
* Use default namespace to fix loc lookup errors
- Fixed errors due to localization change
- Also removed internals visible attribute that could cause subtle issues where the hosting service doesn't work for other apps if things change
* Add CI build and test for new projects
* Up version number so we can pick up fix in nuget packages
* Use netstandard for coreservices DLL
- This is possible thanks to recent updates that target SMO bits using netstandard
* Fix localization failure during dotnet pack
- Namespace changes on SRGen, so updated references to accept the name change
* Port v2 of Hosting service to SqlToolsService
- Renamed project to .v2 so that existing hosted service isn't impacted
- Copied over the CoreServices project which contains ConnectionServiceCore and other reusable services for anything interacting with MSSQL
- Ported unit test project across and verified tests run.
* Nuget package support for reusable DLLs
* Use 1.1 version per Karl's suggestion
* Use correct license URL and project URL
* Use new SMO packages
* Fix GUID parse error breaking on-prem connections
- Fixes https://github.com/Microsoft/sqlopsstudio/issues/1896
- Verified this is only used in retry logic on Azure, so the fact it's failing on-prem isn't relevant. It's still unclear why in some cases the session ID is non-null but not a GUID.
* Added back logging if parse of the GUID fails
ObjectExplorerTests were adding 1 minute to total test time due to
unnecessarily trying and failing to connect to a non-existent server.
Mocking out this call saves 1minute, reducing test time in half
* Differentiate External and FileTable types in object explorer list
- Add (External) and (File Table) to the labels
- Pre-fetch these properties to avoid perf hit
Note on testing: These are both quite hard to initialize, so adding tests to the integration tests would require always having a server with Polybase enabled / FileTable enabled. Given this I feel it should be OK to use manual testing. Long term, we should have unit-level tests that cover the functionality or figure out how to have a perfect pre-configured server and a set of tests to run the breadth of this logic against it.
* Fix handling of vNext servers to default to latest known version
- Default uplevel / unknown versions to the latest version
- In the IsValidFor check also just see if the server version registers as "All" since this is the default for unknown servers. This is a backup handler, the 1st fix is the primary one
* Support `GO N` syntax to execute multiple times
- Plumbed through the batch execution count from the parser and used in the batch execution code path
- Functionality matches SSMS:
- Outputs loop start/end messages that match SSMS if you're doing multi-batch execution
- Outputs an "ignoring failure" error if an error happens during a batch
- Added tests for this
- Manually verified end to end also
* Fixing test error
- Fixes https://github.com/Microsoft/sqlopsstudio/issues/97.
- This should fix the bulk of the issues reported by users since it's the 1st use of this connection in the query code path.
- Implemented the fix in the connection service. This will always open a connection when calling `GetOrOpenConnection`. I cannot see a reason why the connection returned from this should ever be closed.
- resolve issues in both this code path and the edit data code path since both use this method.
- HandleSimpleExecuteRequest now handles the case where no rows are in a result by cleanly returning a success message but with no rows included. This is handled in the front-end instead and goes through the standard path (with a clean explanation message) instead of showing a `error: no results to return`
- MetadataService was always meant to include the type name in the return result, as otherwise the front end has to guess. In order to fix a bug where this resulted in scripting based on the metadata failing (as front-end used `Procedure` instead of `StoredProcedure`), I'm returning the data here. I'll have a matching front end fix but this is overall a good solution to have.
* Avoid crashing if azure edition is System.
* Add new Azure-specific options to the prototype and return through the service calls
* Send azure properties over database info request
* IsCloud should include Azure SQL DW
- Fixed references to this feature flag
- Updated edition handling to include ManagedInstance and removed AzureV1 check since it's never used and it's been retired.
* Fix https://github.com/Microsoft/vscode-mssql/issues/986 and add better server edition naming
- Fixed a number of issues related to the binding queue, specifically the fact that it didn't capture exceptions that occurred on the binding thread. This caused the thread to crash the service.
- The root cause of the error was when you get a connection error during init of the SmoMetadataProvider which threw an exception. Because of this no Binder was created, and the code would null ref later during processing
- Added logic to handle null ref issue and other related code
- Added a unit test for the new error handling path, and supported still returning some value in this case
- Separately, have a fix for an issue where the edition is shown as "SQL Azure" for all Azure connections. Fixing to be "Azure SQL DB" for this case and handle when the database reports as DW or Stretch instead. This maps better to users concept of what the edition should be and avoids returning what is an outdated term.
* Messed up the test - fixing this by returning the expected return object
- Compatibility Level should be a numerical value. In using `enum.ToString()` the result was `Version140` instead of `140`
- Backup date should be sensible if never backed up. Added a simple check that states it was never backed up if the backup date is before 1900.
- Add special handling for token expired errors so they send with a clear flag that'll allow clients to take action on this case
- Send error message instead of callstack for all messages, and ensure all resource manager paths send back inner exceptions so users can understand the true root cause.
* Test firewall rule handling is able to process through the service layer
* Additional tests for authentication and the resource wrapper code
* Positive test case for CreateFirewallRule
* Fixed copyright and usings
* Multi-thread server lookup to increase perf for multiple subscriptions
* Use parallel execution for listsubscriptions
- Also refactored parallel execution code to be a bit more generic
Implementation of the resource provider APIs in order to support Create Firewall Rule. Provides definition for a ResourceProvider and Authentication service. The ResourceProvider supports firewall rules for now, and since authentication is routed through that method it will call into the auth service to set up the current account to be used.
Additional notes:
- Fixed deserialization by adding an Accept header. This shouldn't be necessary, but for some reason the firewall rule defaults to XML without this
- Use generic server list and parse the ID to get the resource group, avoiding a large number of extra calls for each RG
- Errors now include error message from the API
Porting of the vast majority of Azure-related code from SSDT. This is very large, so I want to put this out as one large "lift and shift" PR before I do the tools-service specific JSON-RPC service handlers, connect a new account handler (as the code to get necessary info from accounts and subscriptions isn't fully complete) and add tests over these
**What's in this PR**:
- Created 3 new projects:
- Microsoft.SqlTools.ResourceProvider will host the executable that accepts requests for Azure-related actions over the JSON-RPC protocol. This must be separate from other DLLs since a direct dependency on the Azure SDK DLLs fails (they're NetStandard 1.4 and you can't reference them if you have RuntimeIdentifiers in your .csproj file)
- Microsoft.SqlTools.ResourceProvider.Core is where all the main business logic is, including definitions and logic on how to navigate over resources and create firewall rules, etc.
- Microsoft.SqlTools.ResourceProvider.DefaultImpl is the actual Azure implementation of the resource provider APIs. The reason for separating this is to support eventual integration back into other tools (since their Azure and Identity services will be different).
- Implemented the AzureResourceManager that connects to Azure via ARM APIs and handles creating firewall rule and querying databases. The dependent DLLs have had major breaking changes, so will need additional verification to ensure this works as expected
- Ported the unit tests for all code that was not a viewmodel. Viewmodel test code will be ported in a future update as we plumb through a service-equivalent to these. Also, the DependencyManager code which has overlap with our service provider code is commented out. Will work to uncomment in a future update as it has value to test some scenarios
**What's not in this PR**:
- Identity Services. We currently just have a stub for the interface, and even that will likely change a little
- anything JSON-RPC or registered service related. These will be adapted from the viewmodels and added in a separate PR
- Additional handling of document events with "perforce:" or other SCM strings
- Much of the handling had been added already, but adding in additional validation and ensuring that everywhere `Workspace.GetFile` is called we add a not-null check to avoid null reference errors
* Support "SQL" settings in addition to MSSQL
- Handles having 2 separate configuration definitions and merging / treating them as 1 throughout the app
- If a settings group such as Intellisense is defined on mssql, it will override any generic SQL properties
- Retains backwards compatibility with existing settings.
* LanguageService must send result or intellisense hangs
- Unless the language services methods return results for requests, the VSCode language service protocol will never send a response up to its higher-level code. This means with intellisense off, it appears to hang instead of saying "No results found". This is clearly sub-optimal and if any other extension wants to provide SQL suggestions, it would break them from sending results
- Minor refactor to fully remove Instance-field references in the code
* Add LanguageFlavorNotification handling and refactor LanguageService
- Added new notification handler for language flavor changed
- Refactored the LanguageService so that it no longer relies on so many intertwined static calls, which meant it was impossible to test without modifying the static instance. This will help with test reliability in the future, and prep for replacing the instance with a service provider.
* Skip if not an MSSQL doc and add test
* Handle definition requests
* Fix diagnostics handling
- Fix non-dbo schema support, adding in specific parameter for schema. If this isn't specified, the code will still fall back to splitting up the objectName if it's a multi-part identifier. This ensures that input from action bar or CLI scenarios can still work since we'll accept multi-part names there
- Fix failure to edit data on Azure. This was failing as the SMO query to get the table info failed when cloning SqlConnection. In .Net Core it currently loses the passwor unless PersistSecurity = true.
- Fix bug in error reporting where ID and Method were switched. This caused bad breaks and was caught during integration testing
* Ensure connection open for OE queries and fix connection disposal
- Dispose connection in Metadata service, to ensure we cleanly dispose and don't rely on garbage colleciton
- Fixed issue where if the connection was closed, expanding databases in the Server would fail. This is because SMO doesn't always reopen the connection, certainly not for Server level queries. The solution is to always check if open and reopen.
- Added unit tests for this, which required mocking the relevant IsOpen / OpenConnection methods. Refactored SMO wrapper calls into a dedicated class file to handle this
* Avoid throwing exception in LangageService for OE connections
- Handle non-File URIs by skipping language service binding in those cases. Added test for this
- VSCode passes untitled:, git: and other paths, often without a '/'. Updated logic to handle this.
* Handle windows file paths
* Ensure connection open for OE queries and fix connection disposal
- Dispose connection in Metadata service, to ensure we cleanly dispose and don't rely on garbage colleciton
- Fixed issue where if the connection was closed, expanding databases in the Server would fail. This is because SMO doesn't always reopen the connection, certainly not for Server level queries. The solution is to always check if open and reopen.
- Added unit tests for this, which required mocking the relevant IsOpen / OpenConnection methods. Refactored SMO wrapper calls into a dedicated class file to handle this
* Connect with different properties should actually change context
- Up to now, calling Connect for a previously-connected URI would disconnect, then reconnect ot the original (not new) target. WIth these changes we handle changes to database name or other key properties by updating the ConnectionInfo and connecting to the new target
- Some interesting scenarios are raised by our API, notably that an empty database name maps to the default DB (which we know nothing about). This limits the new feature such that only if the DB Name is specified, we'll change the connection. Hence 2 calls to an empty DB will not result in a DB change.
Additional changes:
- After discussion with Ben, we're simplifying the cancellation logic. He had made changes to support this, so the main update is that we dispose the token in the final block after its last use (hence avoiding a disposed exception) and clean up the number of Waits required since we already have async cancellation support
- Factored some logic such that the OnConnection callback isn't invoked until after we've updated the database name in the GetConnectionCompleteParams method. Again, this supports reporting the actual DB name instead of leaving it blank for default DB requests.
* PR comment fixes
* Bump SMO bits to align with 17.0 RC3 release
* Add updated SMO binary
* Fix formatter failures caused by SqlProc breaking changes
- Changes to SqlProcedureDefinition subclasses mean that the definition starts at "Create Procedure" instead of after it. This highlighted an issue where the prefix region handling was via comma separated list (which adds newlines to the prefix region) instead of newline separated list (which does not).
- Changed the structure to use the 2 different formatters for relevant regions, and verified all tests now pass.
- Also removed unnnecessary Export clause from the SqlTableDefinitionFormatter, since this was spotted during debugging
* Fix sample project.json dependencies
- Fix up the dependencies in our samples as this was failing the build on TravisCI. Should consider in the future whether or not we want to exclude these from CI runs, but for now the benefit is that we're forced to keep the samples up to date.
- Ensure minimum of 2 newlines after GO statement
- All existing unit tests are passing, indicating this is respecting scenarios where users have comment lines after GO statements (this happens in many other tests)
- Connection info cache is now per-DB instead of per-server. This has the downside of increasing #queries, but is required in order to correctly handle isSqlDW since this changes for each database on an Azure server.
- Reverting to a beta version of System.Composition that is compatible with NetCore 1.1. Otherwise we end up with a mishmash of versions which breaks Mac OpenSSL support
- Reverting to a beta version of System.Composition that is compatible with NetCore 1.1. Otherwise we end up with a mishmash of versions which breaks Mac OpenSSL support
- Fixed handling of PlaceCommasBeforeNextStatement so that it doesn't add whitespace unnecessarily. This ensures that repeated formats of the same text shouldn't keep adding whitespace
- Added tests covering a number of scenarios related to this
- Avoid incrementing indents for compound binary boolean expressions. Multiple 'AND x = Y' statements were each indenting instead of having the same indent level.
- Fixes https://github.com/Microsoft/vscode-mssql/issues/709
- Add telemetry point for format requests
- Update PeekDefinition telemetry so that it captures whether the attempt succeeded and if it was connected. This will help us understand whether our current behavior of just notifying success/failure to the output window was useful or not. It will also help us understand whether we're doing a good job implementing this / if we need to improve reliability and preformance.
- Fixes https://github.com/Microsoft/vscode-mssql/issues/705
- Updated the keyword list to include most common keyword ranges and added test to cover top 10 missing keywords
- Fixed a bug in TestUtilities.cs where text range was not defined correctly.
- Note: changes were overridden since master had older fixes cherrypicked that affected the merge. All correct, final changes were in Dev so no actual files to commit
- TSqlFormatterService with support for formatting document and text range inside document
- Settings support for all formatting options.
- Extensibility support so that the service can be initialized using MEF extensibility, and can find all necessary TSqlFormatters using the same process
Fix Initialize request error on startup
- Messages were being read from the input channel before all request handlers were registered
- In particular, the Initialize request which is key for any server to talk to the client was getting lost because the message reader thread begins consuming, and we take an extra few hundred milliseconds due to MEF startup before we register the handler
- The solution is to initialize the message handler so request handlers can register, but not actually start processing incoming messages until all handers are ready. This is a safer way to go and should improve reliability overall
Improvements from internal prototype:
- Normalizing baselines to handle the line ending differences on Mac & Linux vs. Windows
- Significantly shortened most lines by implementing base class methods to wrap common objects from Visitor.Context and removing unnecessary "this." syntax
- Refactored the SqlCommonTableExpressionFormatter and related classes to reduce code count significantly. This provides a pattern to follow when refactoring other classes for similar clarity. It's likely a lot of common logic could be found and reused across these.
- Reduced overall code size by adding utility methods
* Revert NetCore target to 1.0.0 to fix Jenkins
- Changing to 1.* ends up requiring .Net Core 1.1 to be install on the machine. We need a better solution that can work around this and let us stay on 1.0 for now. Checking in to unblock builds, will fix Travis CI later.
* Installing dotnet as part of Travis setup. There is a build in dotnet: argument that uses dotnet-install scripts and supports specific version installation
- On Connecting to a server with no DB specified, we will actually get a connection to Master or some default DB.
- This DB should be used when notifying others of a new connection, and when returning information to the caller so that the correct name can be displayed in the UI.
- Added basic unit tests to cover this scenario
* CredentialService initial impl with Win32 support
- Basic CredentialService APIs for Save, Read, Delete
- E2E unit tests for Credential Service
- Win32 implementation with unit tests
* Save Password support on Mac v1
- Basic keychain support on Mac using Interop with the KeyChain APIs
- All but 1 unit test passing. This will pass once API is changed, but checking this in with the existing API so that if we decide to alter behavior, we have a reference point.
* Remove Username from Credentials API
- Removed Username option from Credentials as this caused conflicting behavior on Mac vs Windows
* Cleanup Using Statements and add Copyright
* Linux CredentialStore Prototype
* Linux credential store support
- Full support for Linux credential store with tests
* Plumbed CredentialService into Program init
* Addressing Pull Request comments
- Basic plumbing to support connections for a URI rather than global connections. Typical use case is editor requests to connect, but this isn't the only possible use
- Tests pass but need updating to cover new functionality, and re-enable AutoCompleteService test once there is a ServiceDiscovery component that registers and returns services. This is necessary as .Instance won't allow for dependency injection and proper testing.
- Basic plumbing to support connections for a URI rather than global connections. Typical use case is editor requests to connect, but this isn't the only possible use
- Tests pass but need updating to cover new functionality, and re-enable AutoCompleteService test once there is a ServiceDiscovery component that registers and returns services. This is necessary as .Instance won't allow for dependency injection and proper testing.
- Handles errors when connecting
- Adds method to capture the settings so the connection service can use these. Expect to read settings from here (and possible connections) in the future
- Minor rename to the test namespace to be consistent with refactor
Kevin Cunnane