Add support for rest api 1.1 interface #1396

yao-msft · 2021-08-24T20:16:48Z

Added support for market
Added support for source agreements
Added support for msstore type
Minor changes to workflow ui

Next pr will be the manifest v1.1 integration.

Microsoft Reviewers: Open in CodeFlow

ashpatil-msft · 2021-08-27T19:57:48Z

        bool Add(SourceDetails& details, IProgressCallback&) override final

I thought we spoke that Add would also make a call to /information endpoint. Isnt that the case? or is it something that will be added later?

In reply to: 907442626

Refers to: src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp:27 in 3a19d8f. [](commit_id = 3a19d8f, deletion_comment = False)

ashpatil-msft · 2021-08-27T20:13:26Z

        bool Add(SourceDetails& details, IProgressCallback&) override final

Just saw you are having a workflow task

In reply to: 907442626

Refers to: src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp:27 in 3a19d8f. [](commit_id = 3a19d8f, deletion_comment = False)

ashpatil-msft · 2021-08-27T20:16:21Z

src/AppInstallerCLICore/Commands/SourceCommand.cpp

@@ -78,7 +79,22 @@ namespace AppInstaller::CLI
            Workflow::EnsureRunningAsAdmin <<
            Workflow::GetSourceList <<
            Workflow::CheckSourceListAgainstAdd <<
-            Workflow::AddSource;
+            Workflow::AddSource <<
+            Workflow::OpenSourceForSourceAdd <<


OpenSourceForSourceAdd

Why not call OpenSource workflow task? #Resolved

Indeed, this workflow is awkward. If the process crashes then the source can be left configured (but presumably not accepted).

I would think it possible to OpenSourceFromDetails in order to work with the agreement, then add the source with an indication of acceptance.

JohnMcPMS · 2021-08-27T21:05:15Z

src/AppInstallerCLICore/Commands/ExportCommand.cpp

@@ -56,6 +57,7 @@ namespace AppInstaller::CLI
            Workflow::ReportExecutionStage(Workflow::ExecutionStage::Discovery) <<
            Workflow::OpenSource <<
            Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) <<
+            Workflow::HandleSourceAgreements <<


If this is just going to have to go in after every source open, should it not just be part of the workflow task itself. #Resolved

JohnMcPMS · 2021-08-27T21:11:02Z

src/AppInstallerCLICore/Commands/SourceCommand.cpp

@@ -78,7 +79,22 @@ namespace AppInstaller::CLI
            Workflow::EnsureRunningAsAdmin <<
            Workflow::GetSourceList <<
            Workflow::CheckSourceListAgainstAdd <<
-            Workflow::AddSource;
+            Workflow::AddSource <<
+            Workflow::OpenSourceForSourceAdd <<


Indeed, this workflow is awkward. If the process crashes then the source can be left configured (but presumably not accepted).

I would think it possible to OpenSourceFromDetails in order to work with the agreement, then add the source with an indication of acceptance.

JohnMcPMS · 2021-08-27T21:16:23Z

src/AppInstallerCLICore/Workflows/WorkflowBase.cpp

@@ -653,6 +673,12 @@ namespace AppInstaller::CLI::Workflow
    }

    void ReportManifestIdentity(Execution::Context& context)


In what cases are we now not reporting the version? #Pending

Mainly show command will handle versioning differently depending on what arguments are passed in

JohnMcPMS · 2021-08-27T21:25:50Z

src/AppInstallerCommonCore/Public/winget/ManifestValidation.h

-    std::vector<ValidationError> ValidateManifest(const Manifest& manifest);
-    void ValidateManifestLocalization(const ManifestVer& manifestVersion, const ManifestLocalization& localization, std::vector<ValidationError>& resultErrors);
+    // validateForRead: bool indicating the validation is performed during reading a manifest, which is less restrictive
+    std::vector<ValidationError> ValidateManifest(const Manifest& manifest, bool validateForRead = false);


I thought that we called that fullValidation before. It this different? #Resolved

JohnMcPMS · 2021-08-27T21:28:22Z

src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h

+        std::vector<std::string> UnsupportedQueryParameters;
+        std::vector<std::string> RequiredQueryParameters;
+
+        std::string GetErrorFieldsMessage(const std::vector<std::string>& input) const noexcept


These should be moved to cpp. #Resolved

JohnMcPMS · 2021-08-27T21:29:19Z

src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h

        }

        return "UnknownMatchField"sv;
    }
+
+    inline PackageMatchField StringToPackageMatchField(std::string_view field)


Both should be moved to cpp as well. #Resolved

JohnMcPMS · 2021-08-27T21:30:27Z

src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h

+            Label(std::move(label)), Text(std::move(text)), Url(std::move(url)) {}
+    };
+
+    struct SourceInformation


Comment on what this is. #Resolved

JohnMcPMS · 2021-08-27T21:32:32Z

src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h

@@ -190,4 +226,10 @@ namespace AppInstaller::Repository
    // Return value indicates whether the named source was found.
    // Passing an empty string drops all sources.
    bool DropSource(std::string_view name);
+
+    // Checks the source agreements and return a list that needs user acceptance.
+    std::vector<SourceDetails> CheckSourceAgreements(const std::vector<SourceDetails>& sources);


Why do these operate on vectors? You can get all of the sources from GetAvailableSources and can operate on them one at a time. #Resolved

Per offline chat, we'll make agreement checking one by one

JohnMcPMS · 2021-08-27T21:38:24Z

src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h

    };

+    // List of fields that require user agreements.
+    enum class ImplicitAgreementField : int


If this is to be a flags enum, you should call that out. I would even suggest in the name. #Resolved

yao-msft · 2021-08-27T22:22:35Z

src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw

+    <value>Do you agree to all the source agreements terms?</value>
+  </data>
+  <data name="SourceAgreementsNotAgreedTo" xml:space="preserve">
+    <value>Source agreements not agreed to. Execution cancelled.</value>


Source agreements not agreed to. Execution cancelled.<

please agree or remove #Resolved

ashpatil-msft · 2021-08-30T18:38:56Z

src/AppInstallerRepositoryCore/Rest/RestClient.cpp

    {
        if (version == Version_1_0_0)
        {
            return std::make_unique<Schema::V1_0::Interface>(api);
        }
-
+        else if (version == Version_1_1_0)


version == Version_1_1_0)

We should make this >= 1.1.0 and < 1.2.0 no? Because if we make a 1.1.* change, we should support it with this client #Resolved

ashpatil-msft · 2021-08-30T18:53:38Z

src/AppInstallerRepositoryCore/Rest/Schema/1_1/RestInterface_1_1.cpp

+                continue;
+            }
+
+            if (searchRequest.Filters.end() != std::find_if(searchRequest.Filters.begin(), searchRequest.Filters.end(), [&](const PackageMatchFilter& filter) { return filter.Field == matchField; }))


searchRequest.Filters

For searchRequest inclusions, should we modify the search request to not have the unsupported package Match fields? #Resolved

jamespik · 2021-08-30T18:48:47Z

src/AppInstallerCLICore/Workflows/InstallFlow.cpp

    {
+        if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::SkipAgreementsAcceptance))


Why is this an option?

I understand command line to auto accept agreement, but one should never be able to add a source and SKIP the agreement? #Resolved

SkipAgreementAcceptance is not a command line option, it's a flag set by ComContext, where for Com flow, we should skip agreement checks

Perhaps we shouldn't call this skip agreements, but something like "User already agreed to agreements".

@denelon RE naming.

Naming things is hard. I'm not sure how one would "read" this code. I agree we don't want this to be perceived as a way to programmatically skip agreements. I think the intent is either to indicate that we don't need to prompt because the agreement had already been accepted by the user or by the IT Administrator when set by policy.

AgreementPreviouslyAccepted ?

jamespik · 2021-08-30T18:55:05Z

src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw

-  <data name="LicenseNotAgreedTo" xml:space="preserve">
-    <value>License not agreed to. Installation cancelled.</value>
+  <data name="PackageAgreementsNotAgreedTo" xml:space="preserve">
+    <value>Package agreements not agreed to. Execution cancelled.</value>


"Execution cancelled" seems like a wierd way to phrase this to me. #Resolved

Perhaps "Operation"?

jamespik · 2021-08-30T18:55:55Z

src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw

+    <value>Failed to open the added source</value>
+  </data>
+  <data name="AcceptSourceAgreementsArgumentDescription" xml:space="preserve">
+    <value>Accept all source agreements during source operations</value>


Some of these have ending punctuation. others do not. Intended? #ByDesign

This one is for argument description, it aligns with other argument description, it does not have the ending period.

ashpatil-msft · 2021-08-31T01:55:10Z

src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw

+    <value>Do you agree to all the source agreements terms?</value>
+  </data>
+  <data name="SourceAgreementsNotAgreedTo" xml:space="preserve">
+    <value>One or more of the source agreements not agreed to. Operation cancelled. Please accept the source agreements or remove the corresponding sources.</value>


ashpatil-msft

* Fix bad merge (microsoft#1408) * Add support for rest api 1.1 interface (microsoft#1396) Added support for market Added support for source agreements Added support for msstore type Minor changes to workflow ui Next pr will be the manifest v1.1 integration. * Remove the packagedAPI experimental feature flag (microsoft#1419) * Integrating custom header with V1.1 changes (microsoft#1422) Integrating custom header with V1.1 changes * Expose pass through header through com interface. (microsoft#1420) * Add custom header. * Update behavior for composites. * Revert line ending changes. * Add comment. * Update composite behavior. * Fix error code * Update Loggers to support multiple Win32 app installs in single process (microsoft#1399) * Update Loggers to support multi threaded processing of Win32 app install requests in WPMServer Co-authored-by: JohnMcPMS <[email protected]> Co-authored-by: yao-msft <[email protected]> Co-authored-by: Ashwini Patil <[email protected]> Co-authored-by: sachintaMSFT <[email protected]>

denelon · 2021-10-01T16:23:44Z

Related to #1243

…1373) * Add c# in proc winrt wrapper and projection * Add wrapper implementation and project * Override factory to return objects. * Fix unreferenced parameters. * Guid name updates. * Undo unneccessary changes. * Remove projection. * Spelling changes. * Simplify packaged test csproj file. * Update guids\packagereference versions. * Remove anycpu * Append defines instead of overriding. * Attempt switch to packages.config for nuget * Add packagereference back. * Try packages.config again. * Try direct restore of csproj * Try workaround solutiondir issue * Try fix bundleplatforms. * Change outputpath * Remove output path * Make client and server have separate lib names. * Add output path back. * Merge build break fix to allow pipeline run. * Move test package to its own solution. * Microsoft.Management.Deployment.Client buildchange * Merge local master to comtesting (#16) * Fix bad merge (#1408) * Add support for rest api 1.1 interface (#1396) Added support for market Added support for source agreements Added support for msstore type Minor changes to workflow ui Next pr will be the manifest v1.1 integration. * Remove the packagedAPI experimental feature flag (#1419) * Integrating custom header with V1.1 changes (#1422) Integrating custom header with V1.1 changes * Expose pass through header through com interface. (#1420) * Add custom header. * Update behavior for composites. * Revert line ending changes. * Add comment. * Update composite behavior. * Fix error code * Update Loggers to support multiple Win32 app installs in single process (#1399) * Update Loggers to support multi threaded processing of Win32 app install requests in WPMServer Co-authored-by: JohnMcPMS <[email protected]> Co-authored-by: yao-msft <[email protected]> Co-authored-by: Ashwini Patil <[email protected]> Co-authored-by: sachintaMSFT <[email protected]> * Remove unneccessary cpp files * Run Com Interface tests * Add appxrecipe to spelling * Update test path * Update test paths. * Remove test adapter dll from testassembly * Add powershell task to install wingetdev * Add runsettings file to publish logs. * Test installing package in vstest * Try newer windows server. * Try enable com tracing for tests. * Try direct run of vstest * Fix install script to call add script. * Fix appxpackages path. * Fix path again, remove trace * Extract unsigned package for loose register * reenable tracing * Include server in packagedtests * Remove build dependency on wingetserver project * Try tracelog comtracing. * Fix powershell args. * Handle variable azure src dir paths. * Make Dll version of com server and use for tests * Fix library name parameter. * Remove com tracing. Fix test. * Fix copyright and spelling. * Try decrease build disk size. Co-authored-by: JohnMcPMS <[email protected]> Co-authored-by: yao-msft <[email protected]> Co-authored-by: Ashwini Patil <[email protected]> Co-authored-by: sachintaMSFT <[email protected]>

yao-msft added 12 commits August 21, 2021 19:14

snap1

6bd318d

snap 2

a3b92f9

snap 3

7f81311

agreement handling

a18c4aa

msstore type support

0475060

Unsupported request handling

b235540

Fix preindexed source

1e9d9f1

Fix merge

bf61aae

improve show version

bbb7482

Integrate agreements handling

7d743de

Fix tests

8b178c9

Fixes and tests

3aa3cb7

yao-msft requested a review from a team as a code owner August 24, 2021 20:16

yao-msft added 5 commits August 24, 2021 13:28

spelling

4f7d698

Merge remote-tracking branch 'upstream/master' into restmarket

ca67092

Fix

415861d

Merge remote-tracking branch 'upstream/master' into restmarket

d08bf70

Verbose logging for http message

3a19d8f

ashpatil-msft reviewed Aug 27, 2021

View reviewed changes

JohnMcPMS reviewed Aug 27, 2021

View reviewed changes

yao-msft commented Aug 27, 2021

View reviewed changes

denelon linked an issue Aug 30, 2021 that may be closed by this pull request

Client verbose logging does not log sufficient information to diagnose issues interacting with rest sources. #1400

Closed

ashpatil-msft reviewed Aug 30, 2021

View reviewed changes

jamespik requested changes Aug 30, 2021

View reviewed changes

ghost added Needs-Author-Feedback Issue needs attention from issue or PR author and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels Aug 30, 2021

Merge remote-tracking branch 'upstream/master' into restmarket

beb06ac

yao-msft added 2 commits August 30, 2021 15:47

Fix merge

38d3b9c

PR comments

2a7744f

ashpatil-msft reviewed Aug 31, 2021

View reviewed changes

yao-msft added 2 commits August 30, 2021 19:29

Fix tests

37ddb9b

Update message

fe4176a

ashpatil-msft approved these changes Aug 31, 2021

View reviewed changes

yao-msft merged commit 3a12003 into microsoft:master Aug 31, 2021

denelon linked an issue Aug 31, 2021 that may be closed by this pull request

winget source add doesn't warn you when adding an unsupported source. #1416

Closed

denelon mentioned this pull request Aug 31, 2021

winget source add doesn't warn you when adding an unsupported source. #1416

Closed

denelon linked an issue Oct 1, 2021 that may be closed by this pull request

WinGet 1.1 manifest schema #1243

Closed

denelon mentioned this pull request Oct 1, 2021

Complex installers #1242

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for rest api 1.1 interface #1396

Add support for rest api 1.1 interface #1396

yao-msft commented Aug 24, 2021 •

edited by ghost

Loading

ashpatil-msft commented Aug 27, 2021 •

edited by yao-msft

Loading

ashpatil-msft commented Aug 27, 2021

ashpatil-msft Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

yao-msft Aug 28, 2021

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

yao-msft Aug 28, 2021

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

yao-msft Aug 27, 2021 •

edited

Loading

ashpatil-msft Aug 30, 2021 •

edited by yao-msft

Loading

ashpatil-msft Aug 30, 2021 •

edited by yao-msft

Loading

jamespik Aug 30, 2021 •

edited by yao-msft

Loading

yao-msft Aug 30, 2021

jamespik Aug 30, 2021

jamespik Aug 30, 2021

denelon Aug 30, 2021

jamespik Aug 30, 2021 •

edited by yao-msft

Loading

jamespik Aug 30, 2021

jamespik Aug 30, 2021 •

edited by yao-msft

Loading

yao-msft Aug 30, 2021

ashpatil-msft Aug 31, 2021

ashpatil-msft left a comment

denelon commented Oct 1, 2021

		@@ -653,6 +673,12 @@ namespace AppInstaller::CLI::Workflow
		}

		void ReportManifestIdentity(Execution::Context& context)

		{
		if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::SkipAgreementsAcceptance))

Add support for rest api 1.1 interface #1396

Add support for rest api 1.1 interface #1396

Conversation

yao-msft commented Aug 24, 2021 • edited by ghost Loading

Microsoft Reviewers: Open in CodeFlow

ashpatil-msft commented Aug 27, 2021 • edited by yao-msft Loading

ashpatil-msft commented Aug 27, 2021

ashpatil-msft Aug 27, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnMcPMS Aug 27, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnMcPMS Aug 27, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnMcPMS Aug 27, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

JohnMcPMS Aug 27, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

JohnMcPMS Aug 27, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

JohnMcPMS Aug 27, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

JohnMcPMS Aug 27, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnMcPMS Aug 27, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

yao-msft Aug 27, 2021 • edited Loading

Choose a reason for hiding this comment

ashpatil-msft Aug 30, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

ashpatil-msft Aug 30, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

jamespik Aug 30, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamespik Aug 30, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamespik Aug 30, 2021 • edited by yao-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashpatil-msft left a comment

Choose a reason for hiding this comment

denelon commented Oct 1, 2021

yao-msft commented Aug 24, 2021 •

edited by ghost

Loading

ashpatil-msft commented Aug 27, 2021 •

edited by yao-msft

Loading

ashpatil-msft Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

JohnMcPMS Aug 27, 2021 •

edited by yao-msft

Loading

yao-msft Aug 27, 2021 •

edited

Loading

ashpatil-msft Aug 30, 2021 •

edited by yao-msft

Loading

ashpatil-msft Aug 30, 2021 •

edited by yao-msft

Loading

jamespik Aug 30, 2021 •

edited by yao-msft

Loading

jamespik Aug 30, 2021 •

edited by yao-msft

Loading

jamespik Aug 30, 2021 •

edited by yao-msft

Loading