Skip to content

Commit

Permalink
More comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@yao-msft
yao-msft committed Oct 19, 2021
1 parent d995b9d commit bcb2660
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 22 deletions.
5 changes: 4 additions & 1 deletion src/AppInstallerRepositoryCore/ISource.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ namespace AppInstaller::Repository
// in which case the identifier should begin with a '*' character.
virtual const std::string& GetIdentifier() const = 0;

// Get the source's configuration from settings.
// Get the source's configuration from settings. Source details can be used during opening the source.
virtual const SourceDetails& GetDetails() const = 0;

// Get the source's information after the source is opened.
virtual SourceInformation GetInformation() const { return {}; };

// Update the last update time of the source sincesource may be updated after source reference is created.
virtual void UpdateLastUpdateTime(std::chrono::system_clock::time_point time) = 0;

// Opens the source.
Expand All @@ -36,6 +38,7 @@ namespace AppInstaller::Repository
// Gets the available sources if the source is composite.
virtual std::vector<std::shared_ptr<ISource>> GetAvailableSources() const { return {}; }

// Set custome header for rest source. Returns false if custom header is not supported.
virtual bool SetCustomHeader(std::string_view header) { UNREFERENCED_PARAMETER(header); return false; }
};

Expand Down
26 changes: 20 additions & 6 deletions src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ namespace AppInstaller::Repository
// The last time that this source was updated.
std::chrono::system_clock::time_point LastUpdateTime = {};

// Whether the source supports InstalledSource correlation.
bool SupportInstalledSearchCorrelation = true;
};

Expand Down Expand Up @@ -143,24 +144,33 @@ namespace AppInstaller::Repository
// Represents a source which would be interacted from outside of repository lib.
struct Source
{
// Default constructor to get all sources available. Can be used in source list, etc.
Source();

// Constructor to get a named source, passing empty string will get all available sources(same as the default constructor).
Source(std::string_view name);

// Constructor to get a PredefinedSource. Like installed source, etc.
Source(PredefinedSource source);

// Constructor to get a source coming with winget. Like winget community source, etc.
Source(WellKnownSource source);

// Constructor for
// Constructor for a source to be added.
Source(std::string_view name, std::string_view arg, std::string_view type);

// Constructor for creating a composite source from a list of available sources.
Source(const std::vector<Source>& availableSources);

// Constructor for creating a composite source from an installed source and available source(may be composite already).
Source(
const Source& installedSource,
const Source& availableSource,
CompositeSearchBehavior searchBehavior = CompositeSearchBehavior::Installed);

// Bool operator to check if a source reference is successfully acquired.
// Theoretically, the constructor could just throw when CreateSource returns empty.
// To avoid putting try catch everywhere, we use bool operator here.
operator bool() const;

// Gets the source's identifier; a unique identifier independent of the name
Expand All @@ -169,12 +179,13 @@ namespace AppInstaller::Repository
// in which case the identifier should begin with a '*' character.
const std::string GetIdentifier() const;

// Get the source's configuration from settings.
// Get the source's configuration details from settings.
const SourceDetails GetDetails() const;

// Get the source's information.
const SourceInformation GetInformation() const;

// Custom header
// Set custom header.
bool SetCustomHeader(std::string_view header);

// Execute a search on the source.
Expand Down Expand Up @@ -210,20 +221,23 @@ namespace AppInstaller::Repository

/* Source operations */

// Opens the source.
// Opens the source. if skipUpdateBeforeOpen is true, source will be opened without check backgroiund update.
std::vector<SourceDetails> Open(IProgressCallback& progress, bool skipUpdateBeforeOpen = false);

// Add source
// Add source. Source add command.
bool Add(IProgressCallback& progress);

// Update Source. Source update command.
std::vector<SourceDetails> Update(IProgressCallback& progress);

// Remove source. Source remove command.
bool Remove(IProgressCallback& progress);

// Drop source. Source reset command.
void Drop();

private:

// Constructor for creating a Source object from an existing ISource. Used by GetAvailableSources.
Source(std::shared_ptr<ISource> source, bool isNamedSource);

std::shared_ptr<ISource> InitializeSourceReference(std::string_view name);
Expand Down
10 changes: 2 additions & 8 deletions src/AppInstallerRepositoryCore/RepositorySource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,14 +477,8 @@ namespace AppInstaller::Repository
// Check all sources for the given name.
SourceList sourceList;

auto source = sourceList.GetCurrentSource(sourceDetails.Name);
THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, source != nullptr);

// Check for a hidden source data that we don't want to collide.
// TODO: Refactor the source interface so that we don't do this
auto hiddenSource = sourceList.GetSource(sourceDetails.Name);
THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS,
hiddenSource && hiddenSource->Origin != SourceOrigin::User && hiddenSource->Origin != SourceOrigin::Metadata);
auto source = sourceList.GetSource(sourceDetails.Name);
THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, source != nullptr && source->Origin != SourceOrigin::Metadata && !source->IsTombstone);

// Check sources allowed by group policy
auto blockingPolicy = GetPolicyBlockingUserSource(sourceDetails.Name, sourceDetails.Type, sourceDetails.Arg, false);
Expand Down
10 changes: 3 additions & 7 deletions src/AppInstallerRepositoryCore/SourceList.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ namespace AppInstaller::Repository

bool ShouldBeHidden(const SourceDetailsInternal& details)
{
return details.IsTombstone || details.Origin == SourceOrigin::Metadata;
return details.IsTombstone || details.Origin == SourceOrigin::Metadata || !details.IsVisible;
}
}

Expand Down Expand Up @@ -282,11 +282,7 @@ namespace AppInstaller::Repository
details.Arg = s_Source_DesktopFrameworks_Arg;
details.Data = s_Source_DesktopFrameworks_Data;
details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin;
// Cheat the system and call this a tombstone. This effectively hides it from everything outside
// of this file, while still allowing it to properly save metadata. There might be problems
// if someone chooses the exact same name as this, which is why its name is very long.
// TODO: When refactoring the source interface, handle this with Visibility or similar.
details.IsTombstone = true;
details.IsVisible = false;
return details;
}
}
Expand Down Expand Up @@ -546,7 +542,7 @@ namespace AppInstaller::Repository
result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::WinGet));
}

// Since we are using the tombstone trick, this is added just to have the source in the internal
// Since the source is not visible outside, this is added just to have the source in the internal
// list for tracking updates. Thus there is no need to check a policy.
result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::DesktopFrameworks));
}
Expand Down
5 changes: 5 additions & 0 deletions src/AppInstallerRepositoryCore/SourceList.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ namespace AppInstaller::Repository

// If true, this is a tombstone, marking the deletion of a source at a lower priority origin.
bool IsTombstone = false;

// If false, this is not visible in GetCurrentSource or GetAllSources, it's only available when explicitly requested.
bool IsVisible = true;

// Accepted agreements info.
std::string AcceptedAgreementsIdentifier;
int AcceptedAgreementFields = 0;
};
Expand Down

1 comment on commit bcb2660

@github-actions
Copy link

@github-actions github-actions bot commented on bcb2660 Oct 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@check-spelling-bot Report

Unrecognized words, please review:

  • backgroiund
  • custome
  • sincesource
To accept these unrecognized words as correct, run the following commands

... in a clone of the [email protected]:yao-msft/winget-cli.git repository
on the sourcerefactor branch:

update_files() {
perl -e '
my $new_expect_file=".github/actions/spelling/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/yao-msft/winget-cli/comments/58246638" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  
update_files
rm $comment_body
git add -u

Please sign in to comment.