mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
128 stars 41 forks source link

[Bug]: new hard base filter should be generated after making 5 changes #15219

Open ioanarusiczki opened 9 hours ago

ioanarusiczki commented 9 hours ago

What happened?

Followup for https://github.com/mozilla/addons/issues/15014

Scenario 11 - before starting I forced base Config set to 5

STR:

  1. blocked a version and uploaded mlbf
  2. deleted the version and uploaded mlbf
  3. blocked again the same version and uploaded mlbf -> this means 3 changes for hard blocks
  4. repeated all the steps above with another version -> this means 3 more new changes
  5. blocked again a 3rd different version and uploaded mlbf -> total of 7 changes

What did you expect to happen?

I'd expect at the 5th change a new hard base filter to be generated.

Attaching the records

7changesrecords.json

Is there an existing issue for this?

┆Issue is synchronized with this Jira Task

diox commented 9 hours ago

If you're blocking and unblocking the same version in between the regenerations, then it counts as 0 changes: between the first time you triggered a generation and the second, nothing has actually changed.

ioanarusiczki commented 9 hours ago

If I block/restrict a guid (it's an addon with 1 version) trigger mlbf delete the same guid trigger mlbf

is equal to 0 changes.

Ok...

diox commented 9 hours ago

No :)

At the last step, that's one change: the base filter "knew" about one block, that block is gone, that's 1 change.

But this:

That's 0 changes: the base filter didn't know about the block you've deleted, so when you trigger mlbf generation again, nothing has changed, it didn't have the time to "see" the block.

KevinMind commented 9 hours ago

@diox I don't think that is correct.

T0: version is not blocked T1 (step 1) block version T2: (step 1) generate MLBF (1 change) T3: (step 2) delete the block T4: (step 2) generate MLBF (1 more change) ... so on.

From my reading @ioanarusiczki is running the cron between each step so they each should count as changes.

Based on your second comment it seems like the first scenario is what's happening, is that right @ioanarusiczki ?

KevinMind commented 9 hours ago

Also, an important 0 step is to force_base the filters to ensure the initial change count is zero. @ioanarusiczki can you update the STR to indicate you have done this?

Edit: It is in the description already 👍

diox commented 8 hours ago

Ah yes, sorry. But it's important to know exactly when blocks are added/deleted compared to when the base filter was last generated.

KevinMind commented 8 hours ago

Really steps 1,2,3 should be separate steps, one for CRUDing a block and one for updating the filter.

KevinMind commented 8 hours ago

"deleted the version" also you are deleting the block not the version right?