Merge pull request #282 from microsoft/staging

Release - 11/21/23

.editorconfig

@@ -200,3 +200,7 @@ dotnet_style_prefer_inferred_tuple_names = true:suggestion
 dotnet_style_prefer_inferred_anonymous_type_member_names = true:suggestion
 dotnet_style_prefer_compound_assignment = true:warning
 dotnet_style_prefer_simplified_interpolation = true:suggestion
+
+# Spelling
+
+spelling_exclusion_path = .\exclusion.dic

build/azure-pipelines.yml

@@ -20,7 +20,7 @@ parameters:
     - release
 
 variables:
-  MSIXVersion: '0.600'
+  MSIXVersion: '0.700'
   solution: '**/GitHubExtension.sln'
   appxPackageDir: 'AppxPackages'
   testOutputArtifactDir: 'TestResults'

build/scripts/CreateBuildInfo.ps1

@@ -5,7 +5,7 @@ Param(
 )
 
 $Major = "0"
-$Minor = "6"
+$Minor = "7"
 $Patch = "99" # default to 99 for local builds
 
 $versionSplit = $Version.Split(".");

exclusion.dic

@@ -0,0 +1 @@
+devhome

src/GitHubExtension/Client/Validation.cs

@@ -14,11 +14,30 @@ private static bool IsValidHttpUri(string uriString, out Uri? uri)
     }
 
     public static bool IsValidGitHubURL(Uri uri)
+    {
+        return IsValidGitHubComURL(uri) || IsValidGitHubEnterpriseServerURL(uri);
+    }
+
+    public static bool IsValidGitHubComURL(Uri uri)
     {
         // Valid GitHub URL has three segments.  The first is '/'.
         if (uri.Segments.Length < 3 || (!uri.Host.Equals("github.com", StringComparison.OrdinalIgnoreCase) && !uri.Host.Equals("www.github.com", StringComparison.OrdinalIgnoreCase)))
         {
-            Log.Logger()?.ReportDebug($"{uri.OriginalString} is not a valid github uri");
+            Log.Logger()?.ReportDebug($"{uri.OriginalString} is not a valid GitHub uri");
+            return false;
+        }
+
+        return true;
+    }
+
+    public static bool IsValidGitHubEnterpriseServerURL(Uri server)
+    {
+        // Valid GHES URL has three segments.
+        // There are no restrictions on the hostname, except what is covered in IsValidHttpUri()
+        // https://docs.github.com/en/[email protected]/admin/configuration/configuring-network-settings/configuring-the-hostname-for-your-instance
+        if (server.Segments.Length < 3)
+        {
+            Log.Logger()?.ReportDebug($"{server.OriginalString} is not a valid GHES repo uri");
             return false;
         }
 

src/GitHubExtension/Constants.cs

@@ -6,6 +6,5 @@ internal class Constants
 {
 #pragma warning disable SA1310 // Field names should not contain underscore
     public const string DEV_HOME_APPLICATION_NAME = "DevHome";
-    public const string GITHUB_REPOS_API = "https://api.github.com/repos/";
 #pragma warning restore SA1310 // Field names should not contain underscore
 }

src/GitHubExtension/DataManager/DataStoreOperationParameters.cs

@@ -1,7 +1,6 @@
 // Copyright (c) Microsoft Corporation and Contributors
 // Licensed under the MIT license.
 
-using GitHubExtension.DeveloperId;
 using Microsoft.Windows.DevHome.SDK;
 
 namespace GitHubExtension;

src/GitHubExtension/DataManager/GitHubDataManager.cs

@@ -16,6 +16,13 @@ public partial class GitHubDataManager : IGitHubDataManager, IDisposable
     private static readonly TimeSpan NotificationRetentionTime = TimeSpan.FromDays(7);
     private static readonly TimeSpan SearchRetentionTime = TimeSpan.FromDays(7);
     private static readonly TimeSpan PullRequestStaleTime = TimeSpan.FromDays(1);
+
+    // It is possible different widgets have queries which touch the same pull requests.
+    // We want to keep this window large enough that we don't delete data being used by
+    // other clients which simply haven't been updated yet but will in the near future.
+    // This is a conservative time period to check for pruning and give time for other
+    // consumers using the data to update its freshness before we remove it.
+    private static readonly TimeSpan LastObservedDeleteSpan = TimeSpan.FromMinutes(6);
     private static readonly long CheckSuiteIdDependabot = 29110;
 
     private static readonly string Name = nameof(GitHubDataManager);
@@ -417,6 +424,10 @@ private async Task UpdatePullRequestsForLoggedInDeveloperIdsAsync(DataStoreOpera
 
                 Log.Logger()?.ReportDebug(Name, $"Updated developer pull requests for {repoFullName}.");
             }
+
+            // After we update for this developer, remove all pull requests for this developer that
+            // were not observed recently.
+            PullRequest.DeleteAllByDeveloperLoginAndLastObservedBefore(DataStore, devId.LoginId, DateTime.UtcNow - LastObservedDeleteSpan);
         }
     }
 
@@ -467,6 +478,9 @@ private async Task UpdatePullRequestsAsync(Repository repository, Octokit.GitHub
 
             CreatePullRequestStatus(dsPullRequest);
         }
+
+        // Remove unobserved pull requests from this repository.
+        PullRequest.DeleteLastObservedBefore(DataStore, repository.Id, DateTime.UtcNow - LastObservedDeleteSpan);
     }
 
     private void CreatePullRequestStatus(PullRequest pullRequest)
@@ -602,6 +616,9 @@ private async Task UpdateIssuesAsync(Repository repository, Octokit.GitHubClient
             // were not recently updated (within the last minute), remove them from the search result.
             SearchIssue.DeleteBefore(DataStore, search, DateTime.Now - TimeSpan.FromMinutes(1));
         }
+
+        // Remove issues from this repository that were not observed recently.
+        Issue.DeleteLastObservedBefore(DataStore, repository.Id, DateTime.UtcNow - LastObservedDeleteSpan);
     }
 
     // Removes unused data from the datastore.
@@ -622,7 +639,7 @@ private void SetLastUpdatedInMetaData()
         MetaData.AddOrUpdate(DataStore, LastUpdatedKeyName, DateTime.Now.ToDataStoreString());
     }
 
-    // Converts fullname -> owner, name.
+    // Converts fullName -> owner, name.
     private string[] GetOwnerAndRepositoryNameFromFullName(string fullName)
     {
         var nameSplit = fullName.Split(new[] { '/' });

src/GitHubExtension/DataManager/GitHubDataManagerUpdate.cs

@@ -8,7 +8,7 @@ namespace GitHubExtension;
 public partial class GitHubDataManager
 {
     // This is how frequently the DataStore update occurs.
-    private static readonly TimeSpan UpdateInterval = TimeSpan.FromMinutes(2);
+    private static readonly TimeSpan UpdateInterval = TimeSpan.FromMinutes(5);
     private static DateTime lastUpdateTime = DateTime.MinValue;
 
     public static async Task Update()

src/GitHubExtension/DataManager/IGitHubSearchManager.cs

@@ -2,7 +2,6 @@
 // Licensed under the MIT license.
 
 using GitHubExtension.DataManager;
-using GitHubExtension.DataModel;
 
 namespace GitHubExtension;
 

src/GitHubExtension/DataModel/DataObjects/Issue.cs

@@ -40,6 +40,8 @@ public class Issue
 
     public long TimeClosed { get; set; } = DataStore.NoForeignKey;
 
+    public long TimeLastObserved { get; set; } = DataStore.NoForeignKey;
+
     // Label IDs are a string concatenation of Label internalIds.
     // We need to duplicate this data in order to properly do inserts and
     // to compare two objects for changes in order to add/remove associations.
@@ -66,6 +68,10 @@ private DataStore? DataStore
     [Computed]
     public DateTime ClosedAt => TimeClosed.ToDateTime();
 
+    [Write(false)]
+    [Computed]
+    public DateTime LastObservedAt => TimeLastObserved.ToDateTime();
+
     // Derived Properties so consumers of these objects do not
     // need to do further queries of the datastore.
     [Write(false)]
@@ -155,6 +161,7 @@ private static Issue CreateFromOctokitIssue(DataStore dataStore, Octokit.Issue o
             TimeCreated = okitIssue.CreatedAt.DateTime.ToDataStoreInteger(),
             TimeUpdated = okitIssue.UpdatedAt.HasValue ? okitIssue.UpdatedAt.Value.DateTime.ToDataStoreInteger() : 0,
             TimeClosed = okitIssue.ClosedAt.HasValue ? okitIssue.ClosedAt.Value.DateTime.ToDataStoreInteger() : 0,
+            TimeLastObserved = DateTime.UtcNow.ToDataStoreInteger(),
         };
 
         // Labels are a string concat of label internal ids.
@@ -207,28 +214,22 @@ private static Issue AddOrUpdateIssue(DataStore dataStore, Issue issue)
         var existing = GetByInternalId(dataStore, issue.InternalId);
         if (existing is not null)
         {
-            if (issue.TimeUpdated > existing.TimeUpdated)
-            {
-                issue.Id = existing.Id;
-                dataStore.Connection!.Update(issue);
-                issue.DataStore = dataStore;
-
-                if (issue.LabelIds != existing.LabelIds)
-                {
-                    UpdateLabelsForIssue(dataStore, issue);
-                }
-
-                if (issue.AssigneeIds != existing.AssigneeIds)
-                {
-                    UpdateAssigneesForIssue(dataStore, issue);
-                }
+            // Existing issues must be updated and always marked observed.
+            issue.Id = existing.Id;
+            dataStore.Connection!.Update(issue);
+            issue.DataStore = dataStore;
 
-                return issue;
+            if (issue.LabelIds != existing.LabelIds)
+            {
+                UpdateLabelsForIssue(dataStore, issue);
             }
-            else
+
+            if (issue.AssigneeIds != existing.AssigneeIds)
             {
-                return existing;
+                UpdateAssigneesForIssue(dataStore, issue);
             }
+
+            return issue;
         }
 
         // No existing issue, add it.
@@ -353,4 +354,20 @@ private static void UpdateAssigneesForIssue(DataStore dataStore, Issue issue)
             }
         }
     }
+
+    // Delete records in a repository not observed before the specified date.
+    public static void DeleteLastObservedBefore(DataStore dataStore, long repositoryId, DateTime date)
+    {
+        // Delete issues older than the time specified for the given repository.
+        // This is intended to be run after updating a repository's issues so that non-observed
+        // records will be removed.
+        var sql = @"DELETE FROM Issue WHERE RepositoryId = $RepositoryId AND TimeLastObserved < $Time;";
+        var command = dataStore.Connection!.CreateCommand();
+        command.CommandText = sql;
+        command.Parameters.AddWithValue("$Time", date.ToDataStoreInteger());
+        command.Parameters.AddWithValue("$RepositoryId", repositoryId);
+        Log.Logger()?.ReportDebug(DataStore.GetCommandLogMessage(sql, command));
+        var rowsDeleted = command.ExecuteNonQuery();
+        Log.Logger()?.ReportDebug(DataStore.GetDeletedLogMessage(rowsDeleted));
+    }
 }

src/GitHubExtension/DataModel/DataObjects/PullRequest.cs

@@ -53,6 +53,8 @@ public class PullRequest
 
     public long TimeClosed { get; set; } = DataStore.NoForeignKey;
 
+    public long TimeLastObserved { get; set; } = DataStore.NoForeignKey;
+
     // Label IDs are a string concatenation of Label internalIds.
     // We need to duplicate this data in order to properly do inserts and
     // to compare two objects for changes in order to add/remove associations.
@@ -80,6 +82,10 @@ public class PullRequest
     [Computed]
     public DateTime ClosedAt => TimeClosed.ToDateTime();
 
+    [Write(false)]
+    [Computed]
+    public DateTime LastObservedAt => TimeLastObserved.ToDateTime();
+
     // Derived Properties so consumers of these objects do not
     // need to do further queries of the datastore.
     [Write(false)]
@@ -336,6 +342,7 @@ private static PullRequest CreateFromOctokitPullRequest(DataStore dataStore, Oct
             TimeUpdated = okitPull.UpdatedAt.DateTime.ToDataStoreInteger(),
             TimeMerged = okitPull.MergedAt.HasValue ? okitPull.MergedAt.Value.DateTime.ToDataStoreInteger() : 0,
             TimeClosed = okitPull.ClosedAt.HasValue ? okitPull.ClosedAt.Value.DateTime.ToDataStoreInteger() : 0,
+            TimeLastObserved = DateTime.UtcNow.ToDataStoreInteger(),
         };
 
         // Labels are a string concat of label internal ids.
@@ -385,28 +392,22 @@ private static PullRequest AddOrUpdatePullRequest(DataStore dataStore, PullReque
         var existingPull = GetByInternalId(dataStore, pull.InternalId);
         if (existingPull is not null)
         {
-            if (pull.TimeUpdated > existingPull.TimeUpdated)
-            {
-                pull.Id = existingPull.Id;
-                dataStore.Connection!.Update(pull);
-                pull.DataStore = dataStore;
-
-                if (pull.LabelIds != existingPull.LabelIds)
-                {
-                    UpdateLabelsForPullRequest(dataStore, pull);
-                }
-
-                if (pull.AssigneeIds != existingPull.AssigneeIds)
-                {
-                    UpdateAssigneesForPullRequest(dataStore, pull);
-                }
+            // Existing pull requests must always be updated to update the LastObserved time.
+            pull.Id = existingPull.Id;
+            dataStore.Connection!.Update(pull);
+            pull.DataStore = dataStore;
 
-                return pull;
+            if (pull.LabelIds != existingPull.LabelIds)
+            {
+                UpdateLabelsForPullRequest(dataStore, pull);
             }
-            else
+
+            if (pull.AssigneeIds != existingPull.AssigneeIds)
             {
-                return existingPull;
+                UpdateAssigneesForPullRequest(dataStore, pull);
             }
+
+            return pull;
         }
 
         // No existing pull request, add it.
@@ -526,4 +527,45 @@ private static void UpdateAssigneesForPullRequest(DataStore dataStore, PullReque
             }
         }
     }
+
+    // Delete records in a repository not observed before the specified date.
+    public static void DeleteLastObservedBefore(DataStore dataStore, long repositoryId, DateTime date)
+    {
+        // Delete pull requests older than the time specified for the given repository.
+        // This is intended to be run after updating a repository's Pull Requests so that non-observed
+        // records will be removed.
+        var sql = @"DELETE FROM PullRequest WHERE RepositoryId = $RepositoryId AND TimeLastObserved < $Time;";
+        var command = dataStore.Connection!.CreateCommand();
+        command.CommandText = sql;
+        command.Parameters.AddWithValue("$Time", date.ToDataStoreInteger());
+        command.Parameters.AddWithValue("$RepositoryId", repositoryId);
+        Log.Logger()?.ReportDebug(DataStore.GetCommandLogMessage(sql, command));
+        var rowsDeleted = command.ExecuteNonQuery();
+        Log.Logger()?.ReportDebug(DataStore.GetDeletedLogMessage(rowsDeleted));
+    }
+
+    // Delete all records from a particular user before the specified date.
+    // This is for removing developer pull requests across any repository that were not updated
+    // recently. This should remove non-open pull requests from the developer across all repositories.
+    public static void DeleteAllByDeveloperLoginAndLastObservedBefore(DataStore dataStore, string loginId, DateTime date)
+    {
+        var developerUsers = User.GetDeveloperUsers(dataStore);
+        foreach (var user in developerUsers)
+        {
+            if (user.Login != loginId)
+            {
+                continue;
+            }
+
+            var sql = @"DELETE FROM PullRequest WHERE AuthorId = $UserId AND TimeLastObserved < $Time;";
+            var command = dataStore.Connection!.CreateCommand();
+            command.CommandText = sql;
+            command.Parameters.AddWithValue("$Time", date.ToDataStoreInteger());
+            command.Parameters.AddWithValue("$UserId", user.Id);
+            Log.Logger()?.ReportDebug(DataStore.GetCommandLogMessage(sql, command));
+            var rowsDeleted = command.ExecuteNonQuery();
+            Log.Logger()?.ReportDebug(DataStore.GetDeletedLogMessage(rowsDeleted));
+            break;
+        }
+    }
 }

src/GitHubExtension/DataModel/GitHubDataStoreSchema.cs

@@ -13,7 +13,7 @@ public GitHubDataStoreSchema()
     }
 
     // Update this anytime incompatible changes happen with a released version.
-    private const long SchemaVersionValue = 0x0004;
+    private const long SchemaVersionValue = 0x0005;
 
     private static readonly string Metadata =
     @"CREATE TABLE Metadata (" +
@@ -79,6 +79,7 @@ public GitHubDataStoreSchema()
         "TimeCreated INTEGER NOT NULL," +
         "TimeUpdated INTEGER NOT NULL," +
         "TimeClosed INTEGER NOT NULL," +
+        "TimeLastObserved INTEGER NOT NULL," +
         "HtmlUrl TEXT NULL COLLATE NOCASE," +
         "Locked INTEGER NOT NULL," +
         "AssigneeIds TEXT NULL COLLATE NOCASE," +
@@ -116,6 +117,7 @@ public GitHubDataStoreSchema()
         "TimeUpdated INTEGER NOT NULL," +
         "TimeMerged INTEGER NOT NULL," +
         "TimeClosed INTEGER NOT NULL," +
+        "TimeLastObserved INTEGER NOT NULL," +
         "HtmlUrl TEXT NULL COLLATE NOCASE," +
         "Locked INTEGER NOT NULL," +
         "Draft INTEGER NOT NULL," +