Title says it all. There's no reason for to have more than 1 omitted link for each occurrence of an omission.

CommentFileSizeAuthor
#13 2996000-13.patch14.18 KBgabesullice
#13 interdiff.txt3.42 KBgabesullice
#11 2996000-11.patch12.89 KBgabesullice
#11 interdiff.txt1.9 KBgabesullice
#9 2996000-9.patch12.23 KBgabesullice
#7 2996000-7.patch12.18 KBgabesullice
#7 2996000-7.tests_only.patch10.61 KBgabesullice
#7 interdiff.txt913 bytesgabesullice
#5 2996000-5.patch11.87 KBgabesullice
#5 cleanup.txt678 bytesgabesullice
#5 2996000-5.tests_only.patch10.3 KBgabesullice
#3 2996000-3.patch1.33 KBwim leers

Comments

gabesullice created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Remove duplicate omitted links. » Deduplicate `meta.omitted.links`

This is what you mean, right?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Deduplicate `meta.omitted.links` » Deduplicate `meta.omitted.links` based on `href` (`via` link)
Status: Active » Needs review
StatusFileSize
new 2996000-3.patch1.33 KB

Perhaps something like this?

Status: Needs review » Needs work

The last submitted patch, 3: 2996000-3.patch, failed testing. View results

gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new 2996000-5.tests_only.patch10.3 KB
new cleanup.txt678 bytes
new 2996000-5.patch11.87 KB

Yep, that's exactly it.

I started with the tests though and did some clean up that preps #2956084: Impossible to raise an error when an `include` is requested for an inaccessible relationship field..

Test only patch + cleanup is the interdiff.

Status: Needs review » Needs work

The last submitted patch, 5: 2996000-5.patch, failed testing. View results

gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt913 bytes
new 2996000-7.tests_only.patch10.61 KB
new 2996000-7.patch12.18 KB

The last submitted patch, 7: 2996000-7.tests_only.patch, failed testing. View results

gabesullice’s picture

gabesullice
Primary language English
commented
StatusFileSize
new 2996000-9.patch12.23 KB

Rerolled.

Status: Needs review » Needs work

The last submitted patch, 9: 2996000-9.patch, failed testing. View results

gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.9 KB
new 2996000-11.patch12.89 KB

Ah, nice catch! /me high-fives the testbot.

That error reminded me of something that bothered me before...

In #3, link keys lost their non-deterministic property, i.e. the same URLs always have the same hash. That means it would theoretically be possible to hardcode a client to look for that hash. It was necessary to do that in order to deduplicate items by their URL, but it's not necessary for that to be true across requests.

I don't want clients to even have the possibility to hardcode a link key hash. So this interdiff introduces a salt in order to get the best of both worlds :)

Status: Needs review » Needs work

The last submitted patch, 11: 2996000-11.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt3.42 KB
new 2996000-13.patch14.18 KB

Whoops, that was a silly oversight.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

  • gabesullice committed 7bcb819 on 8.x-2.x 
    Issue #2996000 by gabesullice, Wim Leers: Deduplicate `meta.omitted....
gabesullice’s picture

gabesullice
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.