refactor: Removes the deprecated redirect endpoint

[michael-s-molina]

SUMMARY

As part of the 4.0 approved initiatives, this PR removes the deprecated Redirect API that supported short URLs (/r) and the url metadata table used to store them that was used before the permalink feature. Users lost the ability to generate R links ~1.5 years ago which seems sufficient time to remove the endpoint. It looks like we forgot to officially deprecate the endpoint in config.py when working on 3.0 but #19078 has the following message in UPDATING.md which is more relevant from a communication standpoint as opposed to the # deprecated comment in config.py:

• 19078: Creation of old shorturl links has been deprecated in favor of a new permalink feature that solves the long url problem (old shorturls will still work, though!). By default, new permalinks use UUID4 as the key. However, to use serial ids similar to the old shorturls, add the following to your superset_config.py: PERMALINK_KEY_TYPE = "id".

TESTING INSTRUCTIONS

CI should be sufficient for merging this PR. We'll do a complete testing of 4.0 after all approved proposals are merged.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d9a3c3e) 69.57% compared to head (8e73687) 69.56%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #26377      +/-   ##
==========================================
- Coverage   69.57%   69.56%   -0.01%     
==========================================
  Files        1893     1892       -1     
  Lines       74220    74162      -58     
  Branches     8263     8263              
==========================================
- Hits        51635    51592      -43     
+ Misses      20504    20489      -15     
  Partials     2081     2081              

Flag	Coverage Δ
hive	53.78% <100.00%> (+<0.01%)	⬆️
javascript	56.84% <ø> (ø)
mysql	78.23% <100.00%> (+0.03%)	⬆️
postgres	78.33% <100.00%> (+<0.01%)	⬆️
presto	53.73% <100.00%> (+<0.01%)	⬆️
python	83.21% <100.00%> (+0.01%)	⬆️
sqlite	77.91% <100.00%> (+<0.01%)	⬆️
unit	56.35% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[john-bodley]

@michael-s-molina although the redirect endpoint is deprecated and the functionality for creating /r links no longer exists, I wonder if we should try to preserve these URLs (for some period) by:

1. Migrate the records from the url to the key_value table where the resource column is redirect and the value column is a JSON key/value pair mapping url.id to url.url. These records could have an expiration of 1–2 years given the somewhat transient nature of URLs.
2. Keep the /r link endpoint which—albeit inefficiently—scans the key_value table (by leveraging JSON SQL UDFs) where the resource column is redirect and the value column contains the /r ID as they key.
3. Remove the deprecated RESTful HTTP endpoints in a far future version.

Alternatively, if code bloat is less of a concern, we could maybe punt this PR (as is) until v6.0 (or later) given the increased likelihood that the legacy redirects would be unused.

Note both options suffer from the same achilles heel in the sense that there’s no guarantee that an institution upgrade cadence will reflect that of the release cycle, hence users may feel short changed when the redirects no longer exist.

[john-bodley]

What does this change have to do with the redirect logic?

[michael-s-molina]

Nothing. It was something I caught while running the tests. It's a flaky test given that order is not guaranteed.

[michael-s-molina]

@michael-s-molina although the redirect endpoint is deprecated and the functionality for creating /r links no longer exists, I wonder if we should try to preserve these URLs (for some period) by:

1. Migrate the records from the url to the key_value table where the resource column is redirect and the value column is a JSON key/value pair mapping url.id to url.url. These records could have an expiration of 1–2 years given the somewhat transient nature of URLs.
2. Keep the /r link endpoint which—albeit inefficiently—scans the key_value table (by leveraging JSON UDFs) where the resource column is redirect and the value column contains the /r ID as they key.

Alternatively, if code bloat is less of a concern, we could maybe punt this PR (as is) until v6.0 (or later) given the increased likelihood that the legacy redirects would be unused.

If we're going to keep the /r endpoint for some period, I think is easier to keep the values in the url table, avoid the migrations, and punt this proposal to 5.0.

[eschutho]

I don't have a strong opinion on whether to keep the urls, but I did notice that it doesn't look like the endpoint was ever marked as deprecated. Not a blocker but something to keep in mind for future.

[michael-s-molina]

I don't have a strong opinion on whether to keep the urls, but I did notice that it doesn't look like the endpoint was ever marked as deprecated. Not a blocker but something to keep in mind for future.

@eschutho I updated the PR description with more context. Let me know if that's sufficient to resolve your concern.

[yousoph]

@michael-s-molina do you have examples handy of what a full short chart URL and what a short dashboard URL looked like before?

Thanks!

Nevermind, found it :) Didn't realize it was just /r/id

[dpgaspar]

interesting approach