Add support for contributed ignore patterns

Also fixes handling of license pattern matches overlapping with ignore patterns. Ignore patterns should not be able to remove valid license pattern matches. Fix tests and handling of overlapping ignores Speed up indexing a little Add tests
openSUSE · Aug 16, 2024 · 2fd55ad · 2fd55ad
1 parent 221dd89
commit 2fd55ad
Show file tree

Hide file tree

Showing 21 changed files with 431 additions and 172 deletions.
diff --git a/assets/vue/EditSnippet.vue b/assets/vue/EditSnippet.vue
@@ -19,6 +19,8 @@
       <input type="hidden" name="package" :value="this.package.id" v-if="this.package !== null" />
       <input type="hidden" name="highlighted" :value="this.highlighted" />
       <input type="hidden" name="edited" :value="this.edited" />
+      <input type="hidden" name="hash" :value="this.hash" />
+      <input type="hidden" name="from" :value="this.from" />
       <div class="row">
         <div class="col mb-3">
           <label class="form-label" for="pattern">Snippet</label>
@@ -149,6 +151,10 @@
           >
             Propose Pattern
           </button>
+          <span v-if="hasContributorRole === true && this.hash !== null && this.from !== null">
+            &nbsp;
+            <button name="propose-ignore" type="submit" value="1" class="btn btn-danger mb-2">Propose Ignore</button>
+          </span>
         </div>
       </div>
       <div v-if="closest !== null" class="row closest-container">
@@ -174,18 +180,23 @@
 
 <script>
 import {setupPopoverDelayed} from './helpers/links.js';
+import {getParams} from './helpers/params.js';
 import UserAgent from '@mojojs/user-agent';
 import CodeMirror from 'codemirror';
 
 export default {
   name: 'EditSnippet',
   data() {
+    const params = getParams();
+
     return {
       closest: null,
       closestUrl: '/snippet/closest',
       decisionUrl: `/snippet/decision/${this.currentSnippet}`,
       edited: '0',
       editor: null,
+      from: params.from ?? null,
+      hash: params.hash ?? null,
       highlighted: '',
       keywords: {},
       license: '',

diff --git a/assets/vue/ProposedPatterns.vue b/assets/vue/ProposedPatterns.vue
@@ -8,28 +8,31 @@
     </div>
     <div v-if="changes !== null && changes.length > 0">
       <div v-for="change in changes" :key="change.id" class="row change-container">
-        <div
-          v-if="change.state === 'proposed' && change.action === 'create_pattern'"
-          class="col-12 change-file-container"
-        >
+        <div v-if="change.state === 'proposed'" class="col-12 change-file-container">
           <div class="change-header">
-            Create pattern from
-            <a :href="change.editUrl" target="_blank">
-              <b v-if="change.data.edited === true">edited snippet</b>
-              <b v-else>unedited snippet</b> </a
-            >, by <b>{{ change.login }}</b>
-            <span v-if="change.package !== null"
-              >,
-              <a :href="change.package.pkgUrl" target="_blank"
-                >for <b>{{ change.package.name }}</b></a
-              >
+            <span v-if="change.action === 'create_pattern'">
+              Create license pattern from
+              <a :href="change.editUrl" target="_blank">
+                <b v-if="change.data.edited === true">edited snippet</b>
+                <b v-else>unedited snippet</b> </a
+              >, by <b>{{ change.login }}</b>
+              <span v-if="change.package !== null"
+                >,
+                <a :href="change.package.pkgUrl" target="_blank"
+                  >for <b>{{ change.package.name }}</b></a
+                >
+              </span>
+            </span>
+            <span v-else-if="change.action === 'create_ignore'">
+              Create ignore pattern from <a :href="change.editUrl" target="_blank"> <b>snippet</b></a
+              >, by <b>{{ change.login }}</b>
             </span>
             <span v-if="currentUser === change.login" class="float-end">
               <a @click="rejectProposal(change)" href="#"><i class="fas fa-times"></i></a>
             </span>
           </div>
           <div class="change-source">
-            <table class="pattern">
+            <table :class="getClassForCode(change)">
               <tbody>
                 <tr v-for="line in change.lines" :key="line.num">
                   <td class="linenumber">{{ line.num }}</td>
@@ -39,42 +42,52 @@
             </table>
           </div>
           <div class="change-form">
-            <div class="row">
-              <div class="col mb-3">
-                <label class="fomr-label" for="license">License</label>
-                <input v-model="change.data.license" type="text" class="form-control" />
-              </div>
-            </div>
-            <div class="row">
-              <div class="col-lg-2 mb-3">
-                <div class="form-floating">
-                  <select v-model="change.data.risk" class="form-control">
-                    <option>0</option>
-                    <option>1</option>
-                    <option>2</option>
-                    <option>3</option>
-                    <option>4</option>
-                    <option>5</option>
-                    <option>6</option>
-                    <option>9</option>
-                  </select>
-                  <label for="risk" class="form-label">Risk</label>
+            <div v-if="change.action === 'create_pattern'">
+              <div class="row">
+                <div class="col mb-3">
+                  <label class="fomr-label" for="license">License</label>
+                  <input v-model="change.data.license" type="text" class="form-control" />
                 </div>
               </div>
-              <div class="col-lg-2">
-                <div class="form-check">
-                  <input v-model="change.data.patent" type="checkbox" class="form-check-input" />
-                  <label class="form-check-label" for="patent">Patent</label>
+              <div class="row">
+                <div class="col-lg-2 mb-3">
+                  <div class="form-floating">
+                    <select v-model="change.data.risk" class="form-control">
+                      <option>0</option>
+                      <option>1</option>
+                      <option>2</option>
+                      <option>3</option>
+                      <option>4</option>
+                      <option>5</option>
+                      <option>6</option>
+                      <option>9</option>
+                    </select>
+                    <label for="risk" class="form-label">Risk</label>
+                  </div>
+                </div>
+                <div class="col-lg-2">
+                  <div class="form-check">
+                    <input v-model="change.data.patent" type="checkbox" class="form-check-input" />
+                    <label class="form-check-label" for="patent">Patent</label>
+                  </div>
+                  <div class="form-check">
+                    <input v-model="change.data.trademark" type="checkbox" class="form-check-input" />
+                    <label class="form-check-label" for="trademark">Trademark</label>
+                  </div>
                 </div>
-                <div class="form-check">
-                  <input v-model="change.data.trademark" type="checkbox" class="form-check-input" />
-                  <label class="form-check-label" for="trademark">Trademark</label>
+                <div class="col-lg-2">
+                  <div class="form-check">
+                    <input v-model="change.data.export_restricted" type="checkbox" class="form-check-input" />
+                    <label class="form-check-label" for="export_restricted">Export Restricted</label>
+                  </div>
                 </div>
               </div>
-              <div class="col-lg-2">
-                <div class="form-check">
-                  <input v-model="change.data.export_restricted" type="checkbox" class="form-check-input" />
-                  <label class="form-check-label" for="export_restricted">Export Restricted</label>
+            </div>
+            <div v-else-if="change.action === 'create_ignore'">
+              <div class="row">
+                <div class="col mb-3">
+                  <label class="fomr-label" for="license">Package</label>
+                  <input v-model="change.data.from" type="text" class="form-control" />
                 </div>
               </div>
             </div>
@@ -100,7 +113,9 @@
           <div class="change-confirmation"><i class="fas fa-sync fa-spin"></i> Updating proposal</div>
         </div>
         <div v-else-if="change.state === 'accepted'" class="col-12">
-          <div class="change-confirmation">Change has been accepted, reindexing related packages in 10 minutes</div>
+          <div class="change-confirmation">
+            Change has been accepted, reindexing related packages in 10 minutes if necessary
+          </div>
         </div>
         <div v-else-if="change.state === 'rejected'" class="col-12">
           <div class="change-confirmation">Proposal has been removed</div>
@@ -145,16 +160,22 @@ export default {
   methods: {
     async acceptProposal(change) {
       change.state = 'updating';
-      for (const key of ['patent', 'trademark', 'export_restricted']) {
-        change.data[key] = change.data[key] === true ? '1' : '0';
-      }
+
       const ua = new UserAgent({baseURL: window.location.href});
       const form = change.data;
       form.contributor = change.login;
       form.delay = 600;
-      form['create-pattern'] = 1;
-      form.checksum = change.token_hexsum;
-      await ua.post(change.createUrl, {form});
+      if (change.action === 'create_pattern') {
+        for (const key of ['patent', 'trademark', 'export_restricted']) {
+          change.data[key] = change.data[key] === true ? '1' : '0';
+        }
+        form['create-pattern'] = 1;
+        form.checksum = change.token_hexsum;
+        await ua.post(change.createUrl, {form});
+      } else if (change.action === 'create_ignore') {
+        await ua.post(change.ignoreUrl, {form: {...form, hash: change.token_hexsum, package: change.data.from}});
+      }
+
       change.state = 'accepted';
     },
     async getChanges() {
@@ -171,12 +192,17 @@ export default {
         change.editUrl = `/snippet/edit/${change.data.snippet}`;
         change.removeUrl = `/licenses/proposed/remove/${change.token_hexsum}`;
         change.createUrl = `/snippet/decision/${change.data.snippet}`;
+        change.ignoreUrl = '/reviews/add_ignore';
 
         if (change.package !== null) change.package.pkgUrl = `/reviews/details/${change.package.id}`;
         if (change.closest !== null) change.closest.licenseUrl = `/licenses/edit_pattern/${change.closest.id}`;
 
-        for (const key of ['edited', 'patent', 'trademark', 'export_restricted']) {
-          change.data[key] = change.data[key] === '1' ? true : false;
+        if (change.action === 'create_pattern') {
+          for (const key of ['edited', 'patent', 'trademark', 'export_restricted']) {
+            change.data[key] = change.data[key] === '1' ? true : false;
+          }
+        } else if (change.action === 'create_ignore') {
+          change.editUrl = `${change.editUrl}?hash=${change.token_hexsum}&from=${change.data.from}`;
         }
 
         const highlighted = change.data.highlighted ?? [];
@@ -200,6 +226,12 @@ export default {
         code: !line.highlighted
       };
     },
+    getClassForCode(change) {
+      return {
+        'change-code-ignore': change.action === 'create_ignore',
+        'change-code-pattern': change.action === 'create_pattern'
+      };
+    },
     handleScroll() {
       if (window.innerHeight + Math.ceil(window.scrollY) >= document.documentElement.scrollHeight) {
         this.loadMore();
@@ -305,4 +337,7 @@ export default {
 .change-highlighted-line {
   background-color: #ffebe9;
 }
+.change-code-ignore {
+  background: repeating-linear-gradient(-45deg, #ffebe9, #ffebe9 1px, #fff 1px, #fff 5px);
+}
 </style>
diff --git a/lib/Cavil.pm b/lib/Cavil.pm
@@ -254,7 +254,8 @@ sub startup ($self) {
   $logged_in->get('/snippet/edit/<id:num>')->to('Snippet#edit')->name('edit_snippet');
   $logged_in->get('/snippet/meta/<id:num>')->to('Snippet#meta')->name('snippet_meta');
   $public->post('/snippet/closest')->to('Snippet#closest')->name('snippet_closest');
-  $public->get('/snippets/from_file/:file/<start:num>/<end:num>')->to('Snippet#from_file')->name('new_snippet');
+  $admin_or_contributor->get('/snippets/from_file/:file/<start:num>/<end:num>')->to('Snippet#from_file')
+    ->name('new_snippet');
   $admin_or_contributor->post('/snippet/decision/<id:num>')->to('Snippet#decision')->name('snippet_decision');
 
   # Upload (experimental)

diff --git a/lib/Cavil/Command/learn.pm b/lib/Cavil/Command/learn.pm
@@ -16,8 +16,9 @@
 package Cavil::Command::learn;
 use Mojo::Base 'Mojolicious::Command', -signatures;
 
-use Mojo::Util qw(getopt);
-use Mojo::File qw(path);
+use Cavil::Util qw(pattern_checksum);
+use Mojo::Util  qw(getopt);
+use Mojo::File  qw(path);
 
 has description => 'Training data for machine learning';
 has usage       => sub ($self) { $self->extract_usage };
@@ -50,7 +51,7 @@ sub _convert ($self, $convert) {
 
   for my $old ($dir->list->each) {
     my $content  = $old->slurp;
-    my $checksum = $patterns->checksum($content);
+    my $checksum = pattern_checksum($content);
     my $new      = $old->sibling("$checksum.txt");
     $new->spew($content);
     $old->remove;
@@ -104,7 +105,7 @@ sub _output_patterns ($self, $good, $bad) {
       my $pattern = $hash->{pattern};
       $pattern =~ s/\ *\$SKIP\d+\ */ /sg;
 
-      my $checksum = $patterns->checksum($pattern);
+      my $checksum = pattern_checksum($pattern);
       my $file     = $good->child("$checksum.txt");
       next unless _spew($file, $pattern);
       say "Exporting pattern $id ($file)";

diff --git a/lib/Cavil/Controller/Reviewer.pm b/lib/Cavil/Controller/Reviewer.pm
@@ -32,11 +32,26 @@ sub add_ignore ($self) {
   my $validation = $self->validation;
   $validation->required('hash')->like(qr/^[a-f0-9]{32}$/i);
   $validation->required('package');
+  $validation->optional('delay')->num;
+  $validation->optional('contributor');
   return $self->reply->json_validation_error if $validation->has_error;
 
-  my $hash    = lc $validation->param('hash');
-  my $package = $validation->param('package');
-  $self->packages->ignore_line($package, $hash);
+  my $owner_id       = $self->users->id_for_login($self->current_user);
+  my $contributor    = $validation->param('contributor');
+  my $contributor_id = $contributor ? $self->users->id_for_login($contributor) : undef;
+  my $delay          = $validation->param('delay') // 0;
+
+  my $hash = $validation->param('hash');
+  $self->packages->ignore_line(
+    {
+      package     => $validation->param('package'),
+      hash        => $hash,
+      owner       => $owner_id,
+      contributor => $contributor_id,
+      delay       => $delay
+    }
+  );
+  $self->patterns->remove_proposal($hash);
 
   return $self->render(json => 'ok');
 }