Add support for updating properties via the Project Query API #6681
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previous work made it possible to query managed projects for property page data via the Project Query API. This change adds support for the `ProjectModelActionNames.SetEvaluatedUIPropertyValue` and `ProjectModelActionNames.SetUnevaluatedUIPropertyValue` actions. Most of the logic is the same for both cases: 1. First, we find and cache the list of `IRule`s containing the properties to update in `OnBeforeExecutingBatchAsync`. We need to do this first because the action executor code in CPS will eventually take a lock, and at that point we won't be able to obtain the `IRule`s anymore. This is largely a matter of looping over the individual actions, figuring out which configurations of the project need to be updated, and getting the appropriate page rule from each. 2. Second, in `ReceiveResultAsync` we process each actual property update. We retrieve the cached rule, find the property, and set its value. 3. Finally, in `OnRequestProcessFinishedAsync` we clear out the list of cached rules. The only place the implementations vary is in actually setting the property value, going through the evaluated or unevaluated property setters as appropriate.
Our Project Query API implementation frequently needs to access project configurations and rules. To speed access to these we cache them inside `PropertyPageQueryCache`. The operation of this type is rather mechanical, and as such is not very interesting from the point of view of testing the Project Query API implementation. At the same time it depends on multiple project services, and setting up mocks is cumbersome. To simplify this, we previously abstracted away various uses of `PropertyPageQueryCache` with the `IPropertyPageQueryCache` interface. However, this still left a number of scenarios that were difficult to test as they depended on instantiating and using a `PropertyPageQueryCache`, and so they still had the same difficulties with adding mocks. Here we go a step further and abstract away the creation of `IPropertyPageQueryCache` instances with the help of a new interface, `IPropertyPageQueryCacheProvider`. The "product" version of this is very simple: it just creates and returns a `PropertyPageQueryCache`. A future change will utilize this to make it easier to create mock `IPropertyPageQueryCache`s.
Extract out the core logic of setting a property within the matching configurations of a project from the traversal of the Project Query API types. `ProjectSetUIPropertyValueActionCore` has no dependencies on Project Query API types and is responsible for finding and updating the actual properties. On the other hand, `ProjectSetUIPropertyValueActionBase<T>` and its derived types are responsible for extracting the necessary data from the Project Query API types, and delegating to `ProjectSetUIPropertyValueActionCore`. There are a couple of benefits to this: the types each have fewer responsibilities, and it is easier to write tests for the core logic as we don't need to wrap up the test data in the Project Query APIs types.
Also expand the set of helper methods for creating mocks.
tmeschter
force-pushed
the
200929-SupportUpdatingProperties
branch
from
9a7e2fb to
18622b8
Compare
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
drewnoakes
approved these changes
Oct 15, 2020
...ectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/IPropertyPageQueryCacheProvider.cs
Outdated
Show resolved
Hide resolved
|
Testing this via the UI and found that (in the local scenario) changes are not persisted to disk until I hit Ctrl+s. Not sure why this is exactly. It's late here so I'll investigate more tomorrow. |
I don't think that's necessarily a problem--CPS sees the changes even if they haven't been persisted to disk. |
|
The legacy property pages update the file on disk immediately. I'm unsure how saving the new editor actually causes the file on disk to change. There must be a link between the project file and the editor via the RDT. I've noticed that while the new editor is open, that attempts to edit the project file's XML result in the UI editor being focused. Again this has nothing to do with this PR :) |
tmeschter
added
Feature-Codespaces
ghost
merged commit 
ghost
added this to the
tmeschter
added a commit
to tmeschter/roslyn-project-system
that referenced
this pull request
Apr 29, 2022
Related to [dotnet/maui dotnet#6681](dotnet/maui#6681). Related to AB#1525249. Currently MAUI includes platform-specific files are for every platform in the MSBuild evaluation, and then at build a target removes _all_ these items and then adds back the ones for the current platform. This works for an actual build but screws up VS, as the Project System doesn't properly handle a `Compile` item that is present in evaluation but removed during a build. The end result is that the platform-specific files are not actually treated as platform-specific, leading to a number of user-visible issues: 1. Platform-specific files incorrectly show all target frameworks in the Nav Bar 2. Spurious errors in the Error List 3. Squiggles in the editor, unless the user changes the Nav Bar to the "correct" target framework 4. Possibly they will not see actual errors in their code (e.g. if you use a Windows-only API in an Android-specific file, but the Nav Bar is set to Windows, you won't get an squiggle about that) 5. The spurious errors block Hot Reload as the Language Service thinks the project is very broken and will not attempt to apply changes With this change the Project System will now check items for special metadata indicating that they should be ignored. If an item is marked with `ExcludeFromCurrentConfiguration=true` the Project System will pretend it does not exist in the evaluation data for the current build configuration. And as the file no longer exists in the evaluation data, it no longer matters that the Project System does not properly handle the removal in the target. See [dotnet/maui dotnet#6681](dotnet/maui#6681) for the related MAUI SDK changes that add the metadata to the appropriate items.
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In addition to supporting querying for data, the Project Query API also specifies a set of actions you can take on various entities that are part of the API. In PR 277099 we defined two new actions that work on projects: setting the "evaluated" value of one of the project's properties, and setting the "unevaluated" value. The differences between the two are relatively small and not particularly interesting, so I'm not going to go into detail on those. Rather, I will focus on the code shared by both.
The
ProjectActionProvidertype plugs our implementation of these actions into the larger Project Query API system. The attributes on this type specify two critical pieces of information: which actions are supported (in this caseSetEvaluatedUIPropertyValueandSetUnevaluatedUIPropertyValue) and which action targets are supported (in this case, the "project" entity indicated byProjectType.TypeName). Most target entities will support multiple actions, and certain actions may be able to target multiple entities. For example, a "delete" action could apply to a file, a reference, or an entire project. The attributes allow us to specify which combination of targets and actions are handled by a particular provider.An
IQueryActionProvidersuch asProjectActionProvideris responsible for returning anIQueryActionExecutorfor a particular step of a query via theCreateQueryActionDataTransformermethod. Here we create either aProjectSetEvaluatedUIPropertyValueActionor aProjectSetUnevaluatedUIPropertyValueActioninstance. All of the interesting logic is in their shared base type,ProjectSetUIPropertyValueActionBase<T>. The entire purpose of this type is to extract interesting information from the Project Query API objects passed in, and the processing of this data is delegated toProjectSetUIPropertyValueActionCore. This division means each type has fewer responsibilities, but the main purpose of the separation is to make it easier to test the core logic of updating property values without the tests having to deal with the Project Query API types.Executing actions within a query takes place in three stages, corresponding to the methods in
ProjectSetUIPropertyValueActionCore. TheOnBeforeExecutingBatchAsyncmethod is called once before the "body" of the query executes, and provides us a chance to do some setup. In this case, we retrieve and cache all of theIRules representing the property pages we are updating across all relevant configurations of all targets projects. Once the body of the query starts the execute, the Project Query API will be holding a lock on the project and we will no longer be able to retrieve these.The
ExecuteAsyncmethod is called once for each targeted project. We pull out all of the relevantIRules from the cache, and apply the property update to each in turn.The
OnAfterExecutingBatchmethod is called once at the end of the query execution and provides an opportunity for clean-up. Here we just clean out the cache. This is probably unnecessary as we don't expect this instance to be re-used for other query executions, but this ensures we don't hold on to configuration-specific data longer than necessary.Microsoft Reviewers: Open in CodeFlow