From b573512312d82e894db7aac89f4938a6b61e1e70 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 7 Nov 2024 04:21:53 +0800 Subject: [PATCH 1/6] Correctly query the primary button in a form (#32438) The "primary button" is used at many places, but sometimes they might conflict (due to button switch, hidden panel, dropdown menu, etc). Sometimes we could add a special CSS class for the buttons, but sometimes not (see the comment of QuickSubmit) This PR introduces `querySingleVisibleElem` to help to get the correct primary button (the only visible one), and prevent from querying the wrong buttons. Fix #32437 --------- Co-authored-by: silverwind --- web_src/js/features/comp/QuickSubmit.ts | 8 +++++++- web_src/js/features/repo-issue-edit.ts | 26 +++++++++++++------------ web_src/js/utils/dom.test.ts | 11 ++++++++++- web_src/js/utils/dom.ts | 11 +++++++++-- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/web_src/js/features/comp/QuickSubmit.ts b/web_src/js/features/comp/QuickSubmit.ts index 3ff29f4fac..385acb319f 100644 --- a/web_src/js/features/comp/QuickSubmit.ts +++ b/web_src/js/features/comp/QuickSubmit.ts @@ -1,3 +1,5 @@ +import {querySingleVisibleElem} from '../../utils/dom.ts'; + export function handleGlobalEnterQuickSubmit(target) { let form = target.closest('form'); if (form) { @@ -12,7 +14,11 @@ export function handleGlobalEnterQuickSubmit(target) { } form = target.closest('.ui.form'); if (form) { - form.querySelector('.ui.primary.button')?.click(); + // A form should only have at most one "primary" button to do quick-submit. + // Here we don't use a special class to mark the primary button, + // because there could be a lot of forms with a primary button, the quick submit should work out-of-box, + // but not keeps asking developers to add that special class again and again (it could be forgotten easily) + querySingleVisibleElem(form, '.ui.primary.button')?.click(); return true; } return false; diff --git a/web_src/js/features/repo-issue-edit.ts b/web_src/js/features/repo-issue-edit.ts index 77a76ad3ca..af97ee4eab 100644 --- a/web_src/js/features/repo-issue-edit.ts +++ b/web_src/js/features/repo-issue-edit.ts @@ -3,7 +3,7 @@ import {handleReply} from './repo-issue.ts'; import {getComboMarkdownEditor, initComboMarkdownEditor, ComboMarkdownEditor} from './comp/ComboMarkdownEditor.ts'; import {POST} from '../modules/fetch.ts'; import {showErrorToast} from '../modules/toast.ts'; -import {hideElem, showElem} from '../utils/dom.ts'; +import {hideElem, querySingleVisibleElem, showElem} from '../utils/dom.ts'; import {attachRefIssueContextPopup} from './contextpopup.ts'; import {initCommentContent, initMarkupContent} from '../markup/content.ts'; import {triggerUploadStateChanged} from './comp/EditorUpload.ts'; @@ -77,20 +77,22 @@ async function onEditContent(event) { } }; - comboMarkdownEditor = getComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); - if (!comboMarkdownEditor) { - editContentZone.innerHTML = document.querySelector('#issue-comment-editor-template').innerHTML; - const saveButton = editContentZone.querySelector('.ui.primary.button'); - comboMarkdownEditor = await initComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); - const syncUiState = () => saveButton.disabled = comboMarkdownEditor.isUploading(); - comboMarkdownEditor.container.addEventListener(ComboMarkdownEditor.EventUploadStateChanged, syncUiState); - editContentZone.querySelector('.ui.cancel.button').addEventListener('click', cancelAndReset); - saveButton.addEventListener('click', saveAndRefresh); - } - // Show write/preview tab and copy raw content as needed showElem(editContentZone); hideElem(renderContent); + + comboMarkdownEditor = getComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); + if (!comboMarkdownEditor) { + editContentZone.innerHTML = document.querySelector('#issue-comment-editor-template').innerHTML; + const saveButton = querySingleVisibleElem(editContentZone, '.ui.primary.button'); + const cancelButton = querySingleVisibleElem(editContentZone, '.ui.cancel.button'); + comboMarkdownEditor = await initComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); + const syncUiState = () => saveButton.disabled = comboMarkdownEditor.isUploading(); + comboMarkdownEditor.container.addEventListener(ComboMarkdownEditor.EventUploadStateChanged, syncUiState); + cancelButton.addEventListener('click', cancelAndReset); + saveButton.addEventListener('click', saveAndRefresh); + } + // FIXME: ideally here should reload content and attachment list from backend for existing editor, to avoid losing data if (!comboMarkdownEditor.value()) { comboMarkdownEditor.value(rawContent.textContent); diff --git a/web_src/js/utils/dom.test.ts b/web_src/js/utils/dom.test.ts index 5c235795fd..d7e3a4939e 100644 --- a/web_src/js/utils/dom.test.ts +++ b/web_src/js/utils/dom.test.ts @@ -1,4 +1,4 @@ -import {createElementFromAttrs, createElementFromHTML} from './dom.ts'; +import {createElementFromAttrs, createElementFromHTML, querySingleVisibleElem} from './dom.ts'; test('createElementFromHTML', () => { expect(createElementFromHTML('foobar').outerHTML).toEqual('foobar'); @@ -16,3 +16,12 @@ test('createElementFromAttrs', () => { }, 'txt', createElementFromHTML('inner')); expect(el.outerHTML).toEqual(''); }); + +test('querySingleVisibleElem', () => { + let el = createElementFromHTML('
foo
'); + expect(querySingleVisibleElem(el, 'span').textContent).toEqual('foo'); + el = createElementFromHTML('
foobar
'); + expect(querySingleVisibleElem(el, 'span').textContent).toEqual('bar'); + el = createElementFromHTML('
foobar
'); + expect(() => querySingleVisibleElem(el, 'span')).toThrowError('Expected exactly one visible element'); +}); diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index a6e0fe2854..e2a4c60e84 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -269,8 +269,8 @@ export function initSubmitEventPolyfill() { */ export function isElemVisible(element: HTMLElement): boolean { if (!element) return false; - - return Boolean(element.offsetWidth || element.offsetHeight || element.getClientRects().length); + // checking element.style.display is not necessary for browsers, but it is required by some tests with happy-dom because happy-dom doesn't really do layout + return Boolean((element.offsetWidth || element.offsetHeight || element.getClientRects().length) && element.style.display !== 'none'); } // replace selected text in a textarea while preserving editor history, e.g. CTRL-Z works after this @@ -330,3 +330,10 @@ export function animateOnce(el: Element, animationClassName: string): Promise(parent: Element, selector: string): T | null { + const elems = parent.querySelectorAll(selector); + const candidates = Array.from(elems).filter(isElemVisible); + if (candidates.length > 1) throw new Error(`Expected exactly one visible element matching selector "${selector}", but found ${candidates.length}`); + return candidates.length ? candidates[0] as T : null; +} From f64fbd9b74998f3ac8353d2a8344e2e6f0ce1936 Mon Sep 17 00:00:00 2001 From: Bruno Sofiato Date: Wed, 6 Nov 2024 17:51:20 -0300 Subject: [PATCH 2/6] Updated tokenizer to better matching when search for code snippets (#32261) This PR improves the accuracy of Gitea's code search. Currently, Gitea does not consider statements such as `onsole.log("hello")` as hits when the user searches for `log`. The culprit is how both ES and Bleve are tokenizing the file contents (in both cases, `console.log` is a whole token). In ES' case, we changed the tokenizer to [simple_pattern_split](https://www.elastic.co/guide/en/elasticsearch/reference/current/analysis-simplepatternsplit-tokenizer.html#:~:text=The%20simple_pattern_split%20tokenizer%20uses%20a,the%20tokenization%20is%20generally%20faster.). In such a case, tokens are words formed by digits and letters. In Bleve's case, it employs a [letter](https://blevesearch.com/docs/Tokenizers/) tokenizer. Resolves #32220 --------- Signed-off-by: Bruno Sofiato --- modules/indexer/code/bleve/bleve.go | 5 +- .../code/elasticsearch/elasticsearch.go | 13 ++++- modules/indexer/code/indexer_test.go | 49 ++++++++++++++++++ modules/indexer/internal/bleve/util.go | 9 ++-- modules/indexer/internal/bleve/util_test.go | 8 +++ .../org42/search-by-path.git/description | 5 +- .../org42/search-by-path.git/info/refs | 2 +- .../objects/info/commit-graph | Bin 1772 -> 0 bytes .../search-by-path.git/objects/info/packs | 2 +- ...29256bc27cb2ec73898507df710be7a3cf5.bitmap | Bin 674 -> 0 bytes ...3dc29256bc27cb2ec73898507df710be7a3cf5.idx | Bin 2080 -> 0 bytes ...3dc29256bc27cb2ec73898507df710be7a3cf5.rev | Bin 196 -> 0 bytes ...76cf6e2b46bc816936ab69306fb10aea571.bitmap | Bin 0 -> 678 bytes ...bef76cf6e2b46bc816936ab69306fb10aea571.idx | Bin 0 -> 2108 bytes ...f76cf6e2b46bc816936ab69306fb10aea571.pack} | Bin 6714 -> 6545 bytes ...bef76cf6e2b46bc816936ab69306fb10aea571.rev | Bin 0 -> 200 bytes .../org42/search-by-path.git/packed-refs | 2 +- 17 files changed, 83 insertions(+), 12 deletions(-) delete mode 100644 tests/gitea-repositories-meta/org42/search-by-path.git/objects/info/commit-graph delete mode 100644 tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-393dc29256bc27cb2ec73898507df710be7a3cf5.bitmap delete mode 100644 tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-393dc29256bc27cb2ec73898507df710be7a3cf5.idx delete mode 100644 tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-393dc29256bc27cb2ec73898507df710be7a3cf5.rev create mode 100644 tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-a7bef76cf6e2b46bc816936ab69306fb10aea571.bitmap create mode 100644 tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-a7bef76cf6e2b46bc816936ab69306fb10aea571.idx rename tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/{pack-393dc29256bc27cb2ec73898507df710be7a3cf5.pack => pack-a7bef76cf6e2b46bc816936ab69306fb10aea571.pack} (74%) create mode 100644 tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-a7bef76cf6e2b46bc816936ab69306fb10aea571.rev diff --git a/modules/indexer/code/bleve/bleve.go b/modules/indexer/code/bleve/bleve.go index 90e5e62bcb..772317fa59 100644 --- a/modules/indexer/code/bleve/bleve.go +++ b/modules/indexer/code/bleve/bleve.go @@ -31,6 +31,7 @@ import ( "github.com/blevesearch/bleve/v2/analysis/token/camelcase" "github.com/blevesearch/bleve/v2/analysis/token/lowercase" "github.com/blevesearch/bleve/v2/analysis/token/unicodenorm" + "github.com/blevesearch/bleve/v2/analysis/tokenizer/letter" "github.com/blevesearch/bleve/v2/analysis/tokenizer/unicode" "github.com/blevesearch/bleve/v2/mapping" "github.com/blevesearch/bleve/v2/search/query" @@ -69,7 +70,7 @@ const ( filenameIndexerAnalyzer = "filenameIndexerAnalyzer" filenameIndexerTokenizer = "filenameIndexerTokenizer" repoIndexerDocType = "repoIndexerDocType" - repoIndexerLatestVersion = 7 + repoIndexerLatestVersion = 8 ) // generateBleveIndexMapping generates a bleve index mapping for the repo indexer @@ -105,7 +106,7 @@ func generateBleveIndexMapping() (mapping.IndexMapping, error) { } else if err := mapping.AddCustomAnalyzer(repoIndexerAnalyzer, map[string]any{ "type": analyzer_custom.Name, "char_filters": []string{}, - "tokenizer": unicode.Name, + "tokenizer": letter.Name, "token_filters": []string{unicodeNormalizeName, camelcase.Name, lowercase.Name}, }); err != nil { return nil, err diff --git a/modules/indexer/code/elasticsearch/elasticsearch.go b/modules/indexer/code/elasticsearch/elasticsearch.go index 669a1bafcc..1c4dd39eff 100644 --- a/modules/indexer/code/elasticsearch/elasticsearch.go +++ b/modules/indexer/code/elasticsearch/elasticsearch.go @@ -30,7 +30,7 @@ import ( ) const ( - esRepoIndexerLatestVersion = 2 + esRepoIndexerLatestVersion = 3 // multi-match-types, currently only 2 types are used // Reference: https://www.elastic.co/guide/en/elasticsearch/reference/7.0/query-dsl-multi-match-query.html#multi-match-types esMultiMatchTypeBestFields = "best_fields" @@ -60,6 +60,10 @@ const ( "settings": { "analysis": { "analyzer": { + "content_analyzer": { + "tokenizer": "content_tokenizer", + "filter" : ["lowercase"] + }, "filename_path_analyzer": { "tokenizer": "path_tokenizer" }, @@ -68,6 +72,10 @@ const ( } }, "tokenizer": { + "content_tokenizer": { + "type": "simple_pattern_split", + "pattern": "[^a-zA-Z0-9]" + }, "path_tokenizer": { "type": "path_hierarchy", "delimiter": "/" @@ -104,7 +112,8 @@ const ( "content": { "type": "text", "term_vector": "with_positions_offsets", - "index": true + "index": true, + "analyzer": "content_analyzer" }, "commit_id": { "type": "keyword", diff --git a/modules/indexer/code/indexer_test.go b/modules/indexer/code/indexer_test.go index 5b33528dcd..020ccc72f8 100644 --- a/modules/indexer/code/indexer_test.go +++ b/modules/indexer/code/indexer_test.go @@ -181,6 +181,55 @@ func testIndexer(name string, t *testing.T, indexer internal.Indexer) { }, }, }, + // Search for matches on the contents of files regardless of case. + { + RepoIDs: nil, + Keyword: "dESCRIPTION", + Langs: 1, + Results: []codeSearchResult{ + { + Filename: "README.md", + Content: "# repo1\n\nDescription for repo1", + }, + }, + }, + // Search for an exact match on the filename within the repo '62' (case insenstive). + // This scenario yields a single result (the file avocado.md on the repo '62') + { + RepoIDs: []int64{62}, + Keyword: "AVOCADO.MD", + Langs: 1, + Results: []codeSearchResult{ + { + Filename: "avocado.md", + Content: "# repo1\n\npineaple pie of cucumber juice", + }, + }, + }, + // Search for matches on the contents of files when the criteria is a expression. + { + RepoIDs: []int64{62}, + Keyword: "console.log", + Langs: 1, + Results: []codeSearchResult{ + { + Filename: "example-file.js", + Content: "console.log(\"Hello, World!\")", + }, + }, + }, + // Search for matches on the contents of files when the criteria is part of a expression. + { + RepoIDs: []int64{62}, + Keyword: "log", + Langs: 1, + Results: []codeSearchResult{ + { + Filename: "example-file.js", + Content: "console.log(\"Hello, World!\")", + }, + }, + }, } for _, kw := range keywords { diff --git a/modules/indexer/internal/bleve/util.go b/modules/indexer/internal/bleve/util.go index b426b39bc2..a0c3dc4ad4 100644 --- a/modules/indexer/internal/bleve/util.go +++ b/modules/indexer/internal/bleve/util.go @@ -6,12 +6,13 @@ package bleve import ( "errors" "os" + "unicode" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" "github.com/blevesearch/bleve/v2" - "github.com/blevesearch/bleve/v2/analysis/tokenizer/unicode" + unicode_tokenizer "github.com/blevesearch/bleve/v2/analysis/tokenizer/unicode" "github.com/blevesearch/bleve/v2/index/upsidedown" "github.com/ethantkoenig/rupture" ) @@ -57,7 +58,7 @@ func openIndexer(path string, latestVersion int) (bleve.Index, int, error) { // may be different on two string and they still be considered equivalent. // Given a phrasse, its shortest word determines its fuzziness. If a phrase uses CJK (eg: `갃갃갃` `啊啊啊`), the fuzziness is zero. func GuessFuzzinessByKeyword(s string) int { - tokenizer := unicode.NewUnicodeTokenizer() + tokenizer := unicode_tokenizer.NewUnicodeTokenizer() tokens := tokenizer.Tokenize([]byte(s)) if len(tokens) > 0 { @@ -77,8 +78,10 @@ func guessFuzzinessByKeyword(s string) int { // according to https://github.com/blevesearch/bleve/issues/1563, the supported max fuzziness is 2 // magic number 4 was chosen to determine the levenshtein distance per each character of a keyword // BUT, when using CJK (eg: `갃갃갃` `啊啊啊`), it mismatches a lot. + // Likewise, queries whose terms contains characters that are *not* letters should not use fuzziness + for _, r := range s { - if r >= 128 { + if r >= 128 || !unicode.IsLetter(r) { return 0 } } diff --git a/modules/indexer/internal/bleve/util_test.go b/modules/indexer/internal/bleve/util_test.go index ae0b12c08d..8f7844464e 100644 --- a/modules/indexer/internal/bleve/util_test.go +++ b/modules/indexer/internal/bleve/util_test.go @@ -35,6 +35,14 @@ func TestBleveGuessFuzzinessByKeyword(t *testing.T) { Input: "갃갃갃", Fuzziness: 0, }, + { + Input: "repo1", + Fuzziness: 0, + }, + { + Input: "avocado.md", + Fuzziness: 0, + }, } for _, scenario := range scenarios { diff --git a/tests/gitea-repositories-meta/org42/search-by-path.git/description b/tests/gitea-repositories-meta/org42/search-by-path.git/description index 382e2d7f10..ffc40a9c48 100644 --- a/tests/gitea-repositories-meta/org42/search-by-path.git/description +++ b/tests/gitea-repositories-meta/org42/search-by-path.git/description @@ -4,5 +4,6 @@ This repository will be used to test code search. The snippet below shows its di ├── avocado.md ├── cucumber.md ├── ham.md -└── potato - └── ham.md +├── potato +| └── ham.md +└── example-file.js \ No newline at end of file diff --git a/tests/gitea-repositories-meta/org42/search-by-path.git/info/refs b/tests/gitea-repositories-meta/org42/search-by-path.git/info/refs index 6b948c96a8..4adf83dda3 100644 --- a/tests/gitea-repositories-meta/org42/search-by-path.git/info/refs +++ b/tests/gitea-repositories-meta/org42/search-by-path.git/info/refs @@ -3,7 +3,7 @@ 65f1bf27bc3bf70f64657658635e66094edbcb4d refs/heads/develop 65f1bf27bc3bf70f64657658635e66094edbcb4d refs/heads/feature/1 78fb907e3a3309eae4fe8fef030874cebbf1cd5e refs/heads/home-md-img-check -3731fe53b763859aaf83e703ee731f6b9447ff1e refs/heads/master +9f894b61946fd2f7b8b9d8e370e4d62f915522f5 refs/heads/master 62fb502a7172d4453f0322a2cc85bddffa57f07a refs/heads/pr-to-update 4649299398e4d39a5c09eb4f534df6f1e1eb87cc refs/heads/sub-home-md-img-check 3fa2f829675543ecfc16b2891aebe8bf0608a8f4 refs/notes/commits diff --git a/tests/gitea-repositories-meta/org42/search-by-path.git/objects/info/commit-graph b/tests/gitea-repositories-meta/org42/search-by-path.git/objects/info/commit-graph deleted file mode 100644 index b38715bb92b034596ebd83954969d4ac88da755e..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1772 zcmZ>E5Aa}QWMT04ba7*V02d(J2f}1=advSGfv{N>++7@vAZ)fZ5E?|X-9WI1C5sX0 zD}0y1Lcr03y@C@%nCFIhS`8@7-k2uVVDERrWWP|nng>@1vDQY<_5}-;z1e) z=7#@*weZfl*icC+h}t;hc+{SMG7EV|-q&#biQOzYnJzrsIMrGDJ6zQ_7I ze@bduL~>jjr{C? zX|=y*#4|JAUiy5q_lvZ~ITxcY-}uV$?#j-(IxEjjTmj7K`=`1r|2(gEdustOF@i7< zu%t78Y`wlWy61NWzZ;+Vw!byseKh=7CrTizN#wg5B$jMsak*fL9mPsBt z5f9YQ0X2swg)9BuovQAI>i=F8+|4i+IG4uzFVfgb8mNXHs)i#@c&~{5)1>B@thCv+_BT)s3tWxDhV5GVvTg-MU3vm6^Gu?zFDi5EezgrG4^_hw zaqeCH=U@CW$EW}CVOqFj-^qL43Ppcz-$!;&3d0Otj=*iQ^LrM&d}b#8P?CK{--~XU zWyoq$A6iRKnWQH)^;m^k?L$fH>Xx-#969PgQQb4^IFL^?n4!sWr&=?L$zMr$c~6w; J6d~FCUjTa)2z>wm diff --git a/tests/gitea-repositories-meta/org42/search-by-path.git/objects/info/packs b/tests/gitea-repositories-meta/org42/search-by-path.git/objects/info/packs index b2af8c8378..9774923d2e 100644 --- a/tests/gitea-repositories-meta/org42/search-by-path.git/objects/info/packs +++ b/tests/gitea-repositories-meta/org42/search-by-path.git/objects/info/packs @@ -1,2 +1,2 @@ -P pack-393dc29256bc27cb2ec73898507df710be7a3cf5.pack +P pack-a7bef76cf6e2b46bc816936ab69306fb10aea571.pack diff --git a/tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-393dc29256bc27cb2ec73898507df710be7a3cf5.bitmap b/tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-393dc29256bc27cb2ec73898507df710be7a3cf5.bitmap deleted file mode 100644 index 1fdef225e830cf65fa66e30e13f64bcc31b5feb8..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 674 zcmZ?r4Dn@PWME}rVBog2Jv1q7kNRo7;}$alYQGEYtFrkD5(i=?C!2TaXGl&Ce zRJr;eP$>o#G&%nN4KOJ%&4|Kd0K0*K!2u|WVj_Ek1Wcs}SO=g`C wvVrC%>~g5o%UlPOhtVzm3=C5o7#PYeFQr{vDB+MdI5Or2 z@_DF$d4XoYFdvW&!~8%t3=06+=vWXaMh+GN>O;oDKt3)k0+b^aivjf@V{srK8A|~9 zzYYU}c~9mPsBt5wE2$>sCN_^G9q`r_}2g z``I{Fd~vWXJH~Q4Y_f8jAKQZJ)+=d~Jeb@(H7Cz_a(Pw^=WG99-)|ouzHUF`Wm;l( zQ+4af5()W_S~FcO?jBu~>Fp7?Ht>E{?R?dfl_rsuuUuk*hF8|rQ2F3t=;UpWb5(2NxuWM3X86|+A}LHI@7xM{;%*4RjD8M ztM9S?&YzN67Lgp6#_4zav~Skpr50g=M_+fgot!2$$7H3=uC>1_eov^gGUj~s{aW-Tex-}GCn)u zAmelq)!8b)8w+J!#Mbajq}BeKaOXnpg7S;c&n6gc)s?SYryVc3Y04x$p{d6z)M_6} zT35HM?c&H$|2ZR`neq10=bOD>q&3dD7-uStP>^fRCO;@|M#Nc zZicbIxisE?k;YcicOy%5_|u9e%4?^bURo%1fOYa)qsPX*%cF!kBYh1u^nEKoc%Cu) zWBU0Qf6Vdee|(r0?$~$op0`5LpWFA70zO2gaZY}vV5_?=vVPaU`Qf*lyX{I`wrskk zz{_m0o{z1O|MJhvZ@*r(T-SY}>FmvFk(Wmn*6b>I*t$14jdkOk#^apJdsSHGJ4CL@ zZ(Fj);Kl7bR$DAxoXwB->Xq2O-5wut@8Ofn&t6LY>AEOjd~)6;k+X`1C#v$E9Z_8O zo?+K1t0n7i)GvS7<=SUclMrwMSoB>67Qeq37(^cdi(M}u4a)z(dPEc`$6N-KuLM@F z!19tW7sw7}VBj_eivI>yo$GeE+Wg?F|5;{u!wN diff --git a/tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-a7bef76cf6e2b46bc816936ab69306fb10aea571.bitmap b/tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-a7bef76cf6e2b46bc816936ab69306fb10aea571.bitmap new file mode 100644 index 0000000000000000000000000000000000000000..39c02c29877a8809da1e67f7cb474efab663d5cc GIT binary patch literal 678 zcmZ?r4Dn@PWME}rVBlW9?|aU-M_aN_h)vGgHks|Wz`CV{AaNjOf?^;A0_^`GG=n&h zMwP4o0hMAa;VhHT!*Z-#h-y;iUR{f&D&QS9G0B;)$#VY;i3t~Gku?h G76SlJfIs~J literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-a7bef76cf6e2b46bc816936ab69306fb10aea571.idx b/tests/gitea-repositories-meta/org42/search-by-path.git/objects/pack/pack-a7bef76cf6e2b46bc816936ab69306fb10aea571.idx new file mode 100644 index 0000000000000000000000000000000000000000..38d0e6b72265f254a87cf9777741ddcead7f60da GIT binary patch literal 2108 zcmb`IYc$k(7{~w2#f+Ab+=kpql(8A)S|N^0L$j3JA~dyBZj*9bsn}zrb!kgw4I0!g zV?$HyDr?J_n2s7-ggO(;FxgC7&ARSBUO30;9B0|>#pnFL-{6hI~Gv@y@(>{MF^ljMhR;F zg|#rZ6lL(su@2r=iz>`4MGgE?)WI)@2D~jo1ichZ@LDTiJ@hP~y$W=occth;?`qJ4 z{zddbe~bat441(O>i-90m|q>o6$j*=I)`@2u~F19yHr_&+s!yCN1J)BY;43vT}YHG zJ}=5$U!|OhVJFhAw6Qu}j;q>Ewukt?VyH!0+6v-W&Qp00{YWkstBChI!ptnR-QyA% zBqemIRq(TlB=_8L&bAxsz5Z4jUM=^9NhB6i8(+e3*k19M>7hNYuV>Zjb2P=Gdsy7{ zcRQSt+R>eRiwv`Uuw|)PJ+wj(S$9v)}p7rCE? z_v!DXL|?14-MjXII47HTQM1JAhGTQ>%*mPjuk6YA@!|Pz-k@;wM?XpU`_m{AReutO zi@d!=EzOU8>R3{YKJ$!FKAGi?vM0%&GcPMW7sYSsagZC9f6V-|B$?6TXiy3TBtz<`Q8ERGG zi4L9oCPtckeK6YWnCa2$;w#kwt>Y0ok==8#zxtIQpW_lsL$!a*#$Fa59^@nQQbRUx zov*o9kWqd3zJ&Ng8ZLEfP{({nTzNgg>4Poz08r}=%UDr5J{)0*kRtd>X`=4MGI7hj!cgl6sttclHL z)qF1MAFyw<+v&QMn@6HMO>|}LK-SfG&E5{%_d+-<7;&9T0nF5W|}wI_=<_iXbXqfQForJq2;u4QCm0L*WGI z8U&Gdg3}7O57h)G;0{D|A-s!V2C`bE0si@X*k=v(Aq2rrfrtB|@*LvS6o`fR_kb^C z>DqaypN3wzGs*(ct}p|6v=HCQtm?bcBOhKh#&oC^MRSUe2G|7Mzw;p5K zOfk?}NmB6m7CeC45AXM?3KOP)fmx@Ja-K8dqDypMbY|;mahYTgM@EkR+$$VStA(57 zT(vcO*N<%PRj>2DjbJG;4}l`X1qsodbt}E=KNnqM@}7qg)A`ex$E)-rEBprL3%XP^ zbF+p4xDW!X3zI++O=nUo5_1c3QgzcZb5ixPiWwF?e!IN*Sf%*pv}yJ0Jf$mbOc?{A zN;493L54jx?p+=w)EVh(sG;v$`N8vy*&kCA10YZ+$S+AO$!FNRKYaGjYUa5q?-{wG$xqi%@<`3e$=6W`&o9bJQB=|d i0FxgHVza3geFPMzzV~eQ;zhXZIIn=sz4Nl@8>Dz{-o^;KMo}ANkQhw zSedp8R=pc!L`t5@{TzA!d--M10t?3hfKS+M%ZQ0WP|IgQT;whk{Zb1cNX z0B{x%P8HSD^&x7%UQcPKJm(4A>YzOMUm1WM3tfl_3f|fWKdz3Ef3c9b5CWhHlS&ay zKr#|@K`I^__b!hT>WuU?)X?{>{NQ=U?2oC50T3t@k+yb%{eFPLaJ;IV!yeG>p$2gc!efJQ)dOY>^ZbB?plWV}z3+Po;MC>9KArWI2i@EN=s5^tj+r*bV z`KEU|8P9|ei9ee!BMSpEbmU+`gMvI9G!eH+4A4Oj1(+zJixgTYp^84zkf0)iHsV## oKn?YM@L9f}m&M)k=T~3;8poZh9M-`j3bobdH9ViT(&%P-Klc Date: Wed, 6 Nov 2024 13:34:32 -0800 Subject: [PATCH 3/6] Include file extension checks in attachment API (#32151) From testing, I found that issue posters and users with repository write access are able to edit attachment names in a way that circumvents the instance-level file extension restrictions using the edit attachment APIs. This snapshot adds checks for these endpoints. --- routers/api/v1/repo/issue_attachment.go | 13 ++++-- .../api/v1/repo/issue_comment_attachment.go | 13 ++++-- routers/api/v1/repo/release_attachment.go | 13 ++++-- services/attachment/attachment.go | 9 +++++ services/context/upload/upload.go | 23 ++++++++--- templates/swagger/v1_json.tmpl | 9 +++++ .../api_comment_attachment_test.go | 23 ++++++++++- .../integration/api_issue_attachment_test.go | 22 +++++++++- .../api_releases_attachment_test.go | 40 +++++++++++++++++++ 9 files changed, 148 insertions(+), 17 deletions(-) create mode 100644 tests/integration/api_releases_attachment_test.go diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 27c7af2282..d0bcadde37 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -12,7 +12,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" - "code.gitea.io/gitea/services/attachment" + attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/convert" @@ -181,7 +181,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { filename = query } - attachment, err := attachment.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ + attachment, err := attachment_service.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -247,6 +247,8 @@ func EditIssueAttachment(ctx *context.APIContext) { // "$ref": "#/responses/Attachment" // "404": // "$ref": "#/responses/error" + // "422": + // "$ref": "#/responses/validationError" // "423": // "$ref": "#/responses/repoArchivedError" @@ -261,8 +263,13 @@ func EditIssueAttachment(ctx *context.APIContext) { attachment.Name = form.Name } - if err := repo_model.UpdateAttachment(ctx, attachment); err != nil { + if err := attachment_service.UpdateAttachment(ctx, setting.Attachment.AllowedTypes, attachment); err != nil { + if upload.IsErrFileTypeForbidden(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + return + } ctx.Error(http.StatusInternalServerError, "UpdateAttachment", err) + return } ctx.JSON(http.StatusCreated, convert.ToAPIAttachment(ctx.Repo.Repository, attachment)) diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 0863ebd182..a556a803e5 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -14,7 +14,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" - "code.gitea.io/gitea/services/attachment" + attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/convert" @@ -189,7 +189,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { filename = query } - attachment, err := attachment.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ + attachment, err := attachment_service.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -263,6 +263,8 @@ func EditIssueCommentAttachment(ctx *context.APIContext) { // "$ref": "#/responses/Attachment" // "404": // "$ref": "#/responses/error" + // "422": + // "$ref": "#/responses/validationError" // "423": // "$ref": "#/responses/repoArchivedError" attach := getIssueCommentAttachmentSafeWrite(ctx) @@ -275,8 +277,13 @@ func EditIssueCommentAttachment(ctx *context.APIContext) { attach.Name = form.Name } - if err := repo_model.UpdateAttachment(ctx, attach); err != nil { + if err := attachment_service.UpdateAttachment(ctx, setting.Attachment.AllowedTypes, attach); err != nil { + if upload.IsErrFileTypeForbidden(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + return + } ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) + return } ctx.JSON(http.StatusCreated, convert.ToAPIAttachment(ctx.Repo.Repository, attach)) } diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index 4a2371e012..ed6cc8e1ea 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -13,7 +13,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" - "code.gitea.io/gitea/services/attachment" + attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/convert" @@ -234,7 +234,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) { } // Create a new attachment and save the file - attach, err := attachment.UploadAttachment(ctx, content, setting.Repository.Release.AllowedTypes, size, &repo_model.Attachment{ + attach, err := attachment_service.UploadAttachment(ctx, content, setting.Repository.Release.AllowedTypes, size, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -291,6 +291,8 @@ func EditReleaseAttachment(ctx *context.APIContext) { // responses: // "201": // "$ref": "#/responses/Attachment" + // "422": + // "$ref": "#/responses/validationError" // "404": // "$ref": "#/responses/notFound" @@ -322,8 +324,13 @@ func EditReleaseAttachment(ctx *context.APIContext) { attach.Name = form.Name } - if err := repo_model.UpdateAttachment(ctx, attach); err != nil { + if err := attachment_service.UpdateAttachment(ctx, setting.Repository.Release.AllowedTypes, attach); err != nil { + if upload.IsErrFileTypeForbidden(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + return + } ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) + return } ctx.JSON(http.StatusCreated, convert.ToAPIAttachment(ctx.Repo.Repository, attach)) } diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index 0fd51e4fa5..ccb97c66c8 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -50,3 +50,12 @@ func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, return NewAttachment(ctx, attach, io.MultiReader(bytes.NewReader(buf), file), fileSize) } + +// UpdateAttachment updates an attachment, verifying that its name is among the allowed types. +func UpdateAttachment(ctx context.Context, allowedTypes string, attach *repo_model.Attachment) error { + if err := upload.Verify(nil, attach.Name, allowedTypes); err != nil { + return err + } + + return repo_model.UpdateAttachment(ctx, attach) +} diff --git a/services/context/upload/upload.go b/services/context/upload/upload.go index 7123420e99..cefd13ebb6 100644 --- a/services/context/upload/upload.go +++ b/services/context/upload/upload.go @@ -28,12 +28,13 @@ func IsErrFileTypeForbidden(err error) bool { } func (err ErrFileTypeForbidden) Error() string { - return "This file extension or type is not allowed to be uploaded." + return "This file cannot be uploaded or modified due to a forbidden file extension or type." } var wildcardTypeRe = regexp.MustCompile(`^[a-z]+/\*$`) -// Verify validates whether a file is allowed to be uploaded. +// Verify validates whether a file is allowed to be uploaded. If buf is empty, it will just check if the file +// has an allowed file extension. func Verify(buf []byte, fileName, allowedTypesStr string) error { allowedTypesStr = strings.ReplaceAll(allowedTypesStr, "|", ",") // compat for old config format @@ -56,21 +57,31 @@ func Verify(buf []byte, fileName, allowedTypesStr string) error { return ErrFileTypeForbidden{Type: fullMimeType} } extension := strings.ToLower(path.Ext(fileName)) + isBufEmpty := len(buf) <= 1 // https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#Unique_file_type_specifiers for _, allowEntry := range allowedTypes { if allowEntry == "*/*" { return nil // everything allowed - } else if strings.HasPrefix(allowEntry, ".") && allowEntry == extension { + } + if strings.HasPrefix(allowEntry, ".") && allowEntry == extension { return nil // extension is allowed - } else if mimeType == allowEntry { + } + if isBufEmpty { + continue // skip mime type checks if buffer is empty + } + if mimeType == allowEntry { return nil // mime type is allowed - } else if wildcardTypeRe.MatchString(allowEntry) && strings.HasPrefix(mimeType, allowEntry[:len(allowEntry)-1]) { + } + if wildcardTypeRe.MatchString(allowEntry) && strings.HasPrefix(mimeType, allowEntry[:len(allowEntry)-1]) { return nil // wildcard match, e.g. image/* } } - log.Info("Attachment with type %s blocked from upload", fullMimeType) + if !isBufEmpty { + log.Info("Attachment with type %s blocked from upload", fullMimeType) + } + return ErrFileTypeForbidden{Type: fullMimeType} } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 92a8888f32..01b12460ac 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7706,6 +7706,9 @@ "404": { "$ref": "#/responses/error" }, + "422": { + "$ref": "#/responses/validationError" + }, "423": { "$ref": "#/responses/repoArchivedError" } @@ -8328,6 +8331,9 @@ "404": { "$ref": "#/responses/error" }, + "422": { + "$ref": "#/responses/validationError" + }, "423": { "$ref": "#/responses/repoArchivedError" } @@ -13474,6 +13480,9 @@ }, "404": { "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" } } } diff --git a/tests/integration/api_comment_attachment_test.go b/tests/integration/api_comment_attachment_test.go index 0ec950d4c2..623467938a 100644 --- a/tests/integration/api_comment_attachment_test.go +++ b/tests/integration/api_comment_attachment_test.go @@ -151,7 +151,7 @@ func TestAPICreateCommentAttachmentWithUnallowedFile(t *testing.T) { func TestAPIEditCommentAttachment(t *testing.T) { defer tests.PrepareTestEnv(t)() - const newAttachmentName = "newAttachmentName" + const newAttachmentName = "newAttachmentName.txt" attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 6}) comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: attachment.CommentID}) @@ -173,6 +173,27 @@ func TestAPIEditCommentAttachment(t *testing.T) { unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: apiAttachment.ID, CommentID: comment.ID, Name: apiAttachment.Name}) } +func TestAPIEditCommentAttachmentWithUnallowedFile(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 6}) + comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: attachment.CommentID}) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: comment.IssueID}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID}) + repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue) + + filename := "file.bad" + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d/assets/%d", + repoOwner.Name, repo.Name, comment.ID, attachment.ID) + req := NewRequestWithValues(t, "PATCH", urlStr, map[string]string{ + "name": filename, + }).AddTokenAuth(token) + + session.MakeRequest(t, req, http.StatusUnprocessableEntity) +} + func TestAPIDeleteCommentAttachment(t *testing.T) { defer tests.PrepareTestEnv(t)() diff --git a/tests/integration/api_issue_attachment_test.go b/tests/integration/api_issue_attachment_test.go index b4196ec6db..6806d27df2 100644 --- a/tests/integration/api_issue_attachment_test.go +++ b/tests/integration/api_issue_attachment_test.go @@ -126,7 +126,7 @@ func TestAPICreateIssueAttachmentWithUnallowedFile(t *testing.T) { func TestAPIEditIssueAttachment(t *testing.T) { defer tests.PrepareTestEnv(t)() - const newAttachmentName = "newAttachmentName" + const newAttachmentName = "hello_world.txt" attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 1}) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: attachment.RepoID}) @@ -147,6 +147,26 @@ func TestAPIEditIssueAttachment(t *testing.T) { unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: apiAttachment.ID, IssueID: issue.ID, Name: apiAttachment.Name}) } +func TestAPIEditIssueAttachmentWithUnallowedFile(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 1}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: attachment.RepoID}) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: attachment.IssueID}) + repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue) + + filename := "file.bad" + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/assets/%d", repoOwner.Name, repo.Name, issue.Index, attachment.ID) + req := NewRequestWithValues(t, "PATCH", urlStr, map[string]string{ + "name": filename, + }).AddTokenAuth(token) + + session.MakeRequest(t, req, http.StatusUnprocessableEntity) +} + func TestAPIDeleteIssueAttachment(t *testing.T) { defer tests.PrepareTestEnv(t)() diff --git a/tests/integration/api_releases_attachment_test.go b/tests/integration/api_releases_attachment_test.go new file mode 100644 index 0000000000..5df3042437 --- /dev/null +++ b/tests/integration/api_releases_attachment_test.go @@ -0,0 +1,40 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "fmt" + "net/http" + "testing" + + auth_model "code.gitea.io/gitea/models/auth" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/tests" +) + +func TestAPIEditReleaseAttachmentWithUnallowedFile(t *testing.T) { + // Limit the allowed release types (since by default there is no restriction) + defer test.MockVariableValue(&setting.Repository.Release.AllowedTypes, ".exe")() + defer tests.PrepareTestEnv(t)() + + attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 9}) + release := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ID: attachment.ReleaseID}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: attachment.RepoID}) + repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + filename := "file.bad" + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets/%d", repoOwner.Name, repo.Name, release.ID, attachment.ID) + req := NewRequestWithValues(t, "PATCH", urlStr, map[string]string{ + "name": filename, + }).AddTokenAuth(token) + + session.MakeRequest(t, req, http.StatusUnprocessableEntity) +} From 913be9e8ac1b745c9eb6dda06146e090166c8b79 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 6 Nov 2024 14:04:48 -0800 Subject: [PATCH 4/6] Add new index for action to resolve the performance problem (#32333) Fix #32224 --- models/activities/action.go | 5 +++- models/migrations/migrations.go | 1 + models/migrations/v1_23/v308.go | 52 +++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 models/migrations/v1_23/v308.go diff --git a/models/activities/action.go b/models/activities/action.go index 9b4ffd7725..c83dba9d46 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -171,7 +171,10 @@ func (a *Action) TableIndices() []*schemas.Index { cudIndex := schemas.NewIndex("c_u_d", schemas.IndexType) cudIndex.AddColumn("created_unix", "user_id", "is_deleted") - indices := []*schemas.Index{actUserIndex, repoIndex, cudIndex} + cuIndex := schemas.NewIndex("c_u", schemas.IndexType) + cuIndex.AddColumn("user_id", "is_deleted") + + indices := []*schemas.Index{actUserIndex, repoIndex, cudIndex, cuIndex} return indices } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 41f337b838..0333e7e204 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -365,6 +365,7 @@ func prepareMigrationTasks() []*migration { newMigration(305, "Add Repository Licenses", v1_23.AddRepositoryLicenses), newMigration(306, "Add BlockAdminMergeOverride to ProtectedBranch", v1_23.AddBlockAdminMergeOverrideBranchProtection), newMigration(307, "Fix milestone deadline_unix when there is no due date", v1_23.FixMilestoneNoDueDate), + newMigration(308, "Add index(user_id, is_deleted) for action table", v1_23.AddNewIndexForUserDashboard), } return preparedMigrations } diff --git a/models/migrations/v1_23/v308.go b/models/migrations/v1_23/v308.go new file mode 100644 index 0000000000..1e8a9b0af2 --- /dev/null +++ b/models/migrations/v1_23/v308.go @@ -0,0 +1,52 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" + "xorm.io/xorm/schemas" +) + +type improveActionTableIndicesAction struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"INDEX"` // Receiver user id. + OpType int + ActUserID int64 // Action user id. + RepoID int64 + CommentID int64 `xorm:"INDEX"` + IsDeleted bool `xorm:"NOT NULL DEFAULT false"` + RefName string + IsPrivate bool `xorm:"NOT NULL DEFAULT false"` + Content string `xorm:"TEXT"` + CreatedUnix timeutil.TimeStamp `xorm:"created"` +} + +// TableName sets the name of this table +func (*improveActionTableIndicesAction) TableName() string { + return "action" +} + +func (a *improveActionTableIndicesAction) TableIndices() []*schemas.Index { + repoIndex := schemas.NewIndex("r_u_d", schemas.IndexType) + repoIndex.AddColumn("repo_id", "user_id", "is_deleted") + + actUserIndex := schemas.NewIndex("au_r_c_u_d", schemas.IndexType) + actUserIndex.AddColumn("act_user_id", "repo_id", "created_unix", "user_id", "is_deleted") + + cudIndex := schemas.NewIndex("c_u_d", schemas.IndexType) + cudIndex.AddColumn("created_unix", "user_id", "is_deleted") + + cuIndex := schemas.NewIndex("c_u", schemas.IndexType) + cuIndex.AddColumn("user_id", "is_deleted") + + indices := []*schemas.Index{actUserIndex, repoIndex, cudIndex, cuIndex} + + return indices +} + +func AddNewIndexForUserDashboard(x *xorm.Engine) error { + return x.Sync(new(improveActionTableIndicesAction)) +} From 276500c314db1c0ef360088753861ffc010a99da Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 6 Nov 2024 19:28:11 -0800 Subject: [PATCH 5/6] Move AddCollabrator and CreateRepositoryByExample to service layer (#32419) - [x] Move `CreateRepositoryByExample` to service layer - [x] Move `AddCollabrator` to service layer - [x] Add a new parameter for `AddCollabrator` so that changing mode immediately after that will become unnecessary. --- models/perm/access_mode.go | 3 + modules/repository/collaborator.go | 48 ---- modules/repository/collaborator_test.go | 280 ---------------------- modules/repository/create.go | 143 ----------- modules/repository/main_test.go | 1 + routers/api/v1/api.go | 2 +- routers/api/v1/repo/collaborators.go | 27 +-- routers/web/repo/setting/collaboration.go | 5 +- services/feed/action_test.go | 1 + services/issue/main_test.go | 1 + services/mailer/main_test.go | 1 + services/repository/adopt.go | 2 +- services/repository/collaboration.go | 49 ++++ services/repository/collaboration_test.go | 16 ++ services/repository/create.go | 141 ++++++++++- services/repository/fork.go | 2 +- services/repository/generate.go | 2 +- services/repository/transfer.go | 6 +- templates/swagger/v1_json.tmpl | 2 +- 19 files changed, 232 insertions(+), 500 deletions(-) delete mode 100644 modules/repository/collaborator.go delete mode 100644 modules/repository/collaborator_test.go diff --git a/models/perm/access_mode.go b/models/perm/access_mode.go index 0364191e2e..6baeb5531a 100644 --- a/models/perm/access_mode.go +++ b/models/perm/access_mode.go @@ -60,3 +60,6 @@ func ParseAccessMode(permission string, allowed ...AccessMode) AccessMode { } return util.Iif(slices.Contains(allowed, m), m, AccessModeNone) } + +// ErrInvalidAccessMode is returned when an invalid access mode is used +var ErrInvalidAccessMode = util.NewInvalidArgumentErrorf("Invalid access mode") diff --git a/modules/repository/collaborator.go b/modules/repository/collaborator.go deleted file mode 100644 index f71c58fbdf..0000000000 --- a/modules/repository/collaborator.go +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package repository - -import ( - "context" - - "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/perm" - access_model "code.gitea.io/gitea/models/perm/access" - repo_model "code.gitea.io/gitea/models/repo" - user_model "code.gitea.io/gitea/models/user" - - "xorm.io/builder" -) - -func AddCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_model.User) error { - if err := repo.LoadOwner(ctx); err != nil { - return err - } - - if user_model.IsUserBlockedBy(ctx, u, repo.OwnerID) || user_model.IsUserBlockedBy(ctx, repo.Owner, u.ID) { - return user_model.ErrBlockedUser - } - - return db.WithTx(ctx, func(ctx context.Context) error { - has, err := db.Exist[repo_model.Collaboration](ctx, builder.Eq{ - "repo_id": repo.ID, - "user_id": u.ID, - }) - if err != nil { - return err - } else if has { - return nil - } - - if err = db.Insert(ctx, &repo_model.Collaboration{ - RepoID: repo.ID, - UserID: u.ID, - Mode: perm.AccessModeWrite, - }); err != nil { - return err - } - - return access_model.RecalculateUserAccess(ctx, repo, u.ID) - }) -} diff --git a/modules/repository/collaborator_test.go b/modules/repository/collaborator_test.go deleted file mode 100644 index 622f6abce4..0000000000 --- a/modules/repository/collaborator_test.go +++ /dev/null @@ -1,280 +0,0 @@ -// Copyright 2019 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package repository - -import ( - "testing" - - "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/organization" - perm_model "code.gitea.io/gitea/models/perm" - access_model "code.gitea.io/gitea/models/perm/access" - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" - "code.gitea.io/gitea/models/unittest" - user_model "code.gitea.io/gitea/models/user" - - "github.com/stretchr/testify/assert" -) - -func TestRepository_AddCollaborator(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - testSuccess := func(repoID, userID int64) { - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repoID}) - assert.NoError(t, repo.LoadOwner(db.DefaultContext)) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID}) - assert.NoError(t, AddCollaborator(db.DefaultContext, repo, user)) - unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repoID}, &user_model.User{ID: userID}) - } - testSuccess(1, 4) - testSuccess(1, 4) - testSuccess(3, 4) -} - -func TestRepoPermissionPublicNonOrgRepo(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - // public non-organization repo - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) - assert.NoError(t, repo.LoadUnits(db.DefaultContext)) - - // plain user - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - perm, err := access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.False(t, perm.CanWrite(unit.Type)) - } - - // change to collaborator - assert.NoError(t, AddCollaborator(db.DefaultContext, repo, user)) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } - - // collaborator - collaborator := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, collaborator) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } - - // owner - owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, owner) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } - - // admin - admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, admin) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } -} - -func TestRepoPermissionPrivateNonOrgRepo(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - // private non-organization repo - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - assert.NoError(t, repo.LoadUnits(db.DefaultContext)) - - // plain user - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) - perm, err := access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.False(t, perm.CanRead(unit.Type)) - assert.False(t, perm.CanWrite(unit.Type)) - } - - // change to collaborator to default write access - assert.NoError(t, AddCollaborator(db.DefaultContext, repo, user)) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } - - assert.NoError(t, repo_model.ChangeCollaborationAccessMode(db.DefaultContext, repo, user.ID, perm_model.AccessModeRead)) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.False(t, perm.CanWrite(unit.Type)) - } - - // owner - owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, owner) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } - - // admin - admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, admin) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } -} - -func TestRepoPermissionPublicOrgRepo(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - // public organization repo - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 32}) - assert.NoError(t, repo.LoadUnits(db.DefaultContext)) - - // plain user - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) - perm, err := access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.False(t, perm.CanWrite(unit.Type)) - } - - // change to collaborator to default write access - assert.NoError(t, AddCollaborator(db.DefaultContext, repo, user)) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } - - assert.NoError(t, repo_model.ChangeCollaborationAccessMode(db.DefaultContext, repo, user.ID, perm_model.AccessModeRead)) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.False(t, perm.CanWrite(unit.Type)) - } - - // org member team owner - owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, owner) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } - - // org member team tester - member := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 15}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, member) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - } - assert.True(t, perm.CanWrite(unit.TypeIssues)) - assert.False(t, perm.CanWrite(unit.TypeCode)) - - // admin - admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, admin) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } -} - -func TestRepoPermissionPrivateOrgRepo(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - // private organization repo - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 24}) - assert.NoError(t, repo.LoadUnits(db.DefaultContext)) - - // plain user - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) - perm, err := access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.False(t, perm.CanRead(unit.Type)) - assert.False(t, perm.CanWrite(unit.Type)) - } - - // change to collaborator to default write access - assert.NoError(t, AddCollaborator(db.DefaultContext, repo, user)) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } - - assert.NoError(t, repo_model.ChangeCollaborationAccessMode(db.DefaultContext, repo, user.ID, perm_model.AccessModeRead)) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, user) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.False(t, perm.CanWrite(unit.Type)) - } - - // org member team owner - owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 15}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, owner) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } - - // update team information and then check permission - team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 5}) - err = organization.UpdateTeamUnits(db.DefaultContext, team, nil) - assert.NoError(t, err) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, owner) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } - - // org member team tester - tester := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, tester) - assert.NoError(t, err) - assert.True(t, perm.CanWrite(unit.TypeIssues)) - assert.False(t, perm.CanWrite(unit.TypeCode)) - assert.False(t, perm.CanRead(unit.TypeCode)) - - // org member team reviewer - reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 20}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, reviewer) - assert.NoError(t, err) - assert.False(t, perm.CanRead(unit.TypeIssues)) - assert.False(t, perm.CanWrite(unit.TypeCode)) - assert.True(t, perm.CanRead(unit.TypeCode)) - - // admin - admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - perm, err = access_model.GetUserRepoPermission(db.DefaultContext, repo, admin) - assert.NoError(t, err) - for _, unit := range repo.Units { - assert.True(t, perm.CanRead(unit.Type)) - assert.True(t, perm.CanWrite(unit.Type)) - } -} diff --git a/modules/repository/create.go b/modules/repository/create.go index 4f18b9b3fa..b4f7033bd7 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -11,160 +11,17 @@ import ( "path/filepath" "strings" - "code.gitea.io/gitea/models" activities_model "code.gitea.io/gitea/models/activities" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" - "code.gitea.io/gitea/models/organization" - "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" - user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/models/webhook" issue_indexer "code.gitea.io/gitea/modules/indexer/issues" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" ) -// CreateRepositoryByExample creates a repository for the user/organization. -func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository, overwriteOrAdopt, isFork bool) (err error) { - if err = repo_model.IsUsableRepoName(repo.Name); err != nil { - return err - } - - has, err := repo_model.IsRepositoryModelExist(ctx, u, repo.Name) - if err != nil { - return fmt.Errorf("IsRepositoryExist: %w", err) - } else if has { - return repo_model.ErrRepoAlreadyExist{ - Uname: u.Name, - Name: repo.Name, - } - } - - repoPath := repo_model.RepoPath(u.Name, repo.Name) - isExist, err := util.IsExist(repoPath) - if err != nil { - log.Error("Unable to check if %s exists. Error: %v", repoPath, err) - return err - } - if !overwriteOrAdopt && isExist { - log.Error("Files already exist in %s and we are not going to adopt or delete.", repoPath) - return repo_model.ErrRepoFilesAlreadyExist{ - Uname: u.Name, - Name: repo.Name, - } - } - - if err = db.Insert(ctx, repo); err != nil { - return err - } - if err = repo_model.DeleteRedirect(ctx, u.ID, repo.Name); err != nil { - return err - } - - // insert units for repo - defaultUnits := unit.DefaultRepoUnits - if isFork { - defaultUnits = unit.DefaultForkRepoUnits - } - units := make([]repo_model.RepoUnit, 0, len(defaultUnits)) - for _, tp := range defaultUnits { - if tp == unit.TypeIssues { - units = append(units, repo_model.RepoUnit{ - RepoID: repo.ID, - Type: tp, - Config: &repo_model.IssuesConfig{ - EnableTimetracker: setting.Service.DefaultEnableTimetracking, - AllowOnlyContributorsToTrackTime: setting.Service.DefaultAllowOnlyContributorsToTrackTime, - EnableDependencies: setting.Service.DefaultEnableDependencies, - }, - }) - } else if tp == unit.TypePullRequests { - units = append(units, repo_model.RepoUnit{ - RepoID: repo.ID, - Type: tp, - Config: &repo_model.PullRequestsConfig{ - AllowMerge: true, AllowRebase: true, AllowRebaseMerge: true, AllowSquash: true, AllowFastForwardOnly: true, - DefaultMergeStyle: repo_model.MergeStyle(setting.Repository.PullRequest.DefaultMergeStyle), - AllowRebaseUpdate: true, - }, - }) - } else if tp == unit.TypeProjects { - units = append(units, repo_model.RepoUnit{ - RepoID: repo.ID, - Type: tp, - Config: &repo_model.ProjectsConfig{ProjectsMode: repo_model.ProjectsModeAll}, - }) - } else { - units = append(units, repo_model.RepoUnit{ - RepoID: repo.ID, - Type: tp, - }) - } - } - - if err = db.Insert(ctx, units); err != nil { - return err - } - - // Remember visibility preference. - u.LastRepoVisibility = repo.IsPrivate - if err = user_model.UpdateUserCols(ctx, u, "last_repo_visibility"); err != nil { - return fmt.Errorf("UpdateUserCols: %w", err) - } - - if err = user_model.IncrUserRepoNum(ctx, u.ID); err != nil { - return fmt.Errorf("IncrUserRepoNum: %w", err) - } - u.NumRepos++ - - // Give access to all members in teams with access to all repositories. - if u.IsOrganization() { - teams, err := organization.FindOrgTeams(ctx, u.ID) - if err != nil { - return fmt.Errorf("FindOrgTeams: %w", err) - } - for _, t := range teams { - if t.IncludesAllRepositories { - if err := models.AddRepository(ctx, t, repo); err != nil { - return fmt.Errorf("AddRepository: %w", err) - } - } - } - - if isAdmin, err := access_model.IsUserRepoAdmin(ctx, repo, doer); err != nil { - return fmt.Errorf("IsUserRepoAdmin: %w", err) - } else if !isAdmin { - // Make creator repo admin if it wasn't assigned automatically - if err = AddCollaborator(ctx, repo, doer); err != nil { - return fmt.Errorf("AddCollaborator: %w", err) - } - if err = repo_model.ChangeCollaborationAccessMode(ctx, repo, doer.ID, perm.AccessModeAdmin); err != nil { - return fmt.Errorf("ChangeCollaborationAccessModeCtx: %w", err) - } - } - } else if err = access_model.RecalculateAccesses(ctx, repo); err != nil { - // Organization automatically called this in AddRepository method. - return fmt.Errorf("RecalculateAccesses: %w", err) - } - - if setting.Service.AutoWatchNewRepos { - if err = repo_model.WatchRepo(ctx, doer, repo, true); err != nil { - return fmt.Errorf("WatchRepo: %w", err) - } - } - - if err = webhook.CopyDefaultWebhooksToRepo(ctx, repo.ID); err != nil { - return fmt.Errorf("CopyDefaultWebhooksToRepo: %w", err) - } - - return nil -} - const notRegularFileMode = os.ModeSymlink | os.ModeNamedPipe | os.ModeSocket | os.ModeDevice | os.ModeCharDevice | os.ModeIrregular // getDirectorySize returns the disk consumption for a given path diff --git a/modules/repository/main_test.go b/modules/repository/main_test.go index f81dfcdafb..799e8c17c3 100644 --- a/modules/repository/main_test.go +++ b/modules/repository/main_test.go @@ -8,6 +8,7 @@ import ( "code.gitea.io/gitea/models/unittest" + _ "code.gitea.io/gitea/models" _ "code.gitea.io/gitea/models/actions" ) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index bfc601c835..23f466873b 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1172,7 +1172,7 @@ func Routes() *web.Router { m.Get("", reqAnyRepoReader(), repo.ListCollaborators) m.Group("/{collaborator}", func() { m.Combo("").Get(reqAnyRepoReader(), repo.IsCollaborator). - Put(reqAdmin(), bind(api.AddCollaboratorOption{}), repo.AddCollaborator). + Put(reqAdmin(), bind(api.AddCollaboratorOption{}), repo.AddOrUpdateCollaborator). Delete(reqAdmin(), repo.DeleteCollaborator) m.Get("/permission", repo.GetRepoPermissions) }) diff --git a/routers/api/v1/repo/collaborators.go b/routers/api/v1/repo/collaborators.go index 39c9ba527d..ea9d8b0f37 100644 --- a/routers/api/v1/repo/collaborators.go +++ b/routers/api/v1/repo/collaborators.go @@ -12,7 +12,6 @@ import ( access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" - repo_module "code.gitea.io/gitea/modules/repository" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" @@ -123,11 +122,11 @@ func IsCollaborator(ctx *context.APIContext) { } } -// AddCollaborator add a collaborator to a repository -func AddCollaborator(ctx *context.APIContext) { +// AddOrUpdateCollaborator add or update a collaborator to a repository +func AddOrUpdateCollaborator(ctx *context.APIContext) { // swagger:operation PUT /repos/{owner}/{repo}/collaborators/{collaborator} repository repoAddCollaborator // --- - // summary: Add a collaborator to a repository + // summary: Add or Update a collaborator to a repository // produces: // - application/json // parameters: @@ -177,20 +176,18 @@ func AddCollaborator(ctx *context.APIContext) { return } - if err := repo_module.AddCollaborator(ctx, ctx.Repo.Repository, collaborator); err != nil { - if errors.Is(err, user_model.ErrBlockedUser) { - ctx.Error(http.StatusForbidden, "AddCollaborator", err) - } else { - ctx.Error(http.StatusInternalServerError, "AddCollaborator", err) - } - return + p := perm.AccessModeWrite + if form.Permission != nil { + p = perm.ParseAccessMode(*form.Permission) } - if form.Permission != nil { - if err := repo_model.ChangeCollaborationAccessMode(ctx, ctx.Repo.Repository, collaborator.ID, perm.ParseAccessMode(*form.Permission)); err != nil { - ctx.Error(http.StatusInternalServerError, "ChangeCollaborationAccessMode", err) - return + if err := repo_service.AddOrUpdateCollaborator(ctx, ctx.Repo.Repository, collaborator, p); err != nil { + if errors.Is(err, user_model.ErrBlockedUser) { + ctx.Error(http.StatusForbidden, "AddOrUpdateCollaborator", err) + } else { + ctx.Error(http.StatusInternalServerError, "AddOrUpdateCollaborator", err) } + return } ctx.Status(http.StatusNoContent) diff --git a/routers/web/repo/setting/collaboration.go b/routers/web/repo/setting/collaboration.go index 31f9f76d0f..18ecff8250 100644 --- a/routers/web/repo/setting/collaboration.go +++ b/routers/web/repo/setting/collaboration.go @@ -14,7 +14,6 @@ import ( unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" - repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/mailer" @@ -100,12 +99,12 @@ func CollaborationPost(ctx *context.Context) { } } - if err = repo_module.AddCollaborator(ctx, ctx.Repo.Repository, u); err != nil { + if err = repo_service.AddOrUpdateCollaborator(ctx, ctx.Repo.Repository, u, perm.AccessModeWrite); err != nil { if errors.Is(err, user_model.ErrBlockedUser) { ctx.Flash.Error(ctx.Tr("repo.settings.add_collaborator.blocked_user")) ctx.Redirect(ctx.Repo.RepoLink + "/settings/collaboration") } else { - ctx.ServerError("AddCollaborator", err) + ctx.ServerError("AddOrUpdateCollaborator", err) } return } diff --git a/services/feed/action_test.go b/services/feed/action_test.go index e1b071d8f6..60cf7fbb49 100644 --- a/services/feed/action_test.go +++ b/services/feed/action_test.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + _ "code.gitea.io/gitea/models" _ "code.gitea.io/gitea/models/actions" "github.com/stretchr/testify/assert" diff --git a/services/issue/main_test.go b/services/issue/main_test.go index 5dac54183b..819c5d98c3 100644 --- a/services/issue/main_test.go +++ b/services/issue/main_test.go @@ -8,6 +8,7 @@ import ( "code.gitea.io/gitea/models/unittest" + _ "code.gitea.io/gitea/models" _ "code.gitea.io/gitea/models/actions" ) diff --git a/services/mailer/main_test.go b/services/mailer/main_test.go index f803c736ca..5591bea02b 100644 --- a/services/mailer/main_test.go +++ b/services/mailer/main_test.go @@ -8,6 +8,7 @@ import ( "code.gitea.io/gitea/models/unittest" + _ "code.gitea.io/gitea/models" _ "code.gitea.io/gitea/models/actions" ) diff --git a/services/repository/adopt.go b/services/repository/adopt.go index 3d6fe71a09..615f4d482c 100644 --- a/services/repository/adopt.go +++ b/services/repository/adopt.go @@ -66,7 +66,7 @@ func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateR } } - if err := repo_module.CreateRepositoryByExample(ctx, doer, u, repo, true, false); err != nil { + if err := CreateRepositoryByExample(ctx, doer, u, repo, true, false); err != nil { return err } diff --git a/services/repository/collaboration.go b/services/repository/collaboration.go index 4a43ae2a28..abe0489fc5 100644 --- a/services/repository/collaboration.go +++ b/services/repository/collaboration.go @@ -9,11 +9,60 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + + "xorm.io/builder" ) +func AddOrUpdateCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_model.User, mode perm.AccessMode) error { + // only allow valid access modes, read, write and admin + if mode < perm.AccessModeRead || mode > perm.AccessModeAdmin { + return perm.ErrInvalidAccessMode + } + + if err := repo.LoadOwner(ctx); err != nil { + return err + } + + if user_model.IsUserBlockedBy(ctx, u, repo.OwnerID) || user_model.IsUserBlockedBy(ctx, repo.Owner, u.ID) { + return user_model.ErrBlockedUser + } + + return db.WithTx(ctx, func(ctx context.Context) error { + collaboration, has, err := db.Get[repo_model.Collaboration](ctx, builder.Eq{ + "repo_id": repo.ID, + "user_id": u.ID, + }) + if err != nil { + return err + } else if has { + if collaboration.Mode == mode { + return nil + } + if _, err = db.GetEngine(ctx). + Where("repo_id=?", repo.ID). + And("user_id=?", u.ID). + Cols("mode"). + Update(&repo_model.Collaboration{ + Mode: mode, + }); err != nil { + return err + } + } else if err = db.Insert(ctx, &repo_model.Collaboration{ + RepoID: repo.ID, + UserID: u.ID, + Mode: mode, + }); err != nil { + return err + } + + return access_model.RecalculateUserAccess(ctx, repo, u.ID) + }) +} + // DeleteCollaboration removes collaboration relation between the user and repository. func DeleteCollaboration(ctx context.Context, repo *repo_model.Repository, collaborator *user_model.User) (err error) { collaboration := &repo_model.Collaboration{ diff --git a/services/repository/collaboration_test.go b/services/repository/collaboration_test.go index a2eb06b81a..2b9a5d0b8b 100644 --- a/services/repository/collaboration_test.go +++ b/services/repository/collaboration_test.go @@ -7,6 +7,7 @@ import ( "testing" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -14,6 +15,21 @@ import ( "github.com/stretchr/testify/assert" ) +func TestRepository_AddCollaborator(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + testSuccess := func(repoID, userID int64) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repoID}) + assert.NoError(t, repo.LoadOwner(db.DefaultContext)) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID}) + assert.NoError(t, AddOrUpdateCollaborator(db.DefaultContext, repo, user, perm.AccessModeWrite)) + unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repoID}, &user_model.User{ID: userID}) + } + testSuccess(1, 4) + testSuccess(1, 4) + testSuccess(3, 4) +} + func TestRepository_DeleteCollaboration(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/services/repository/create.go b/services/repository/create.go index 282b2d3e58..261ac7fccc 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -12,9 +12,15 @@ import ( "strings" "time" + "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/perm" + access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" @@ -243,7 +249,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt var rollbackRepo *repo_model.Repository if err := db.WithTx(ctx, func(ctx context.Context) error { - if err := repo_module.CreateRepositoryByExample(ctx, doer, u, repo, false, false); err != nil { + if err := CreateRepositoryByExample(ctx, doer, u, repo, false, false); err != nil { return err } @@ -335,3 +341,136 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt return repo, nil } + +// CreateRepositoryByExample creates a repository for the user/organization. +func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository, overwriteOrAdopt, isFork bool) (err error) { + if err = repo_model.IsUsableRepoName(repo.Name); err != nil { + return err + } + + has, err := repo_model.IsRepositoryModelExist(ctx, u, repo.Name) + if err != nil { + return fmt.Errorf("IsRepositoryExist: %w", err) + } else if has { + return repo_model.ErrRepoAlreadyExist{ + Uname: u.Name, + Name: repo.Name, + } + } + + repoPath := repo_model.RepoPath(u.Name, repo.Name) + isExist, err := util.IsExist(repoPath) + if err != nil { + log.Error("Unable to check if %s exists. Error: %v", repoPath, err) + return err + } + if !overwriteOrAdopt && isExist { + log.Error("Files already exist in %s and we are not going to adopt or delete.", repoPath) + return repo_model.ErrRepoFilesAlreadyExist{ + Uname: u.Name, + Name: repo.Name, + } + } + + if err = db.Insert(ctx, repo); err != nil { + return err + } + if err = repo_model.DeleteRedirect(ctx, u.ID, repo.Name); err != nil { + return err + } + + // insert units for repo + defaultUnits := unit.DefaultRepoUnits + if isFork { + defaultUnits = unit.DefaultForkRepoUnits + } + units := make([]repo_model.RepoUnit, 0, len(defaultUnits)) + for _, tp := range defaultUnits { + if tp == unit.TypeIssues { + units = append(units, repo_model.RepoUnit{ + RepoID: repo.ID, + Type: tp, + Config: &repo_model.IssuesConfig{ + EnableTimetracker: setting.Service.DefaultEnableTimetracking, + AllowOnlyContributorsToTrackTime: setting.Service.DefaultAllowOnlyContributorsToTrackTime, + EnableDependencies: setting.Service.DefaultEnableDependencies, + }, + }) + } else if tp == unit.TypePullRequests { + units = append(units, repo_model.RepoUnit{ + RepoID: repo.ID, + Type: tp, + Config: &repo_model.PullRequestsConfig{ + AllowMerge: true, AllowRebase: true, AllowRebaseMerge: true, AllowSquash: true, AllowFastForwardOnly: true, + DefaultMergeStyle: repo_model.MergeStyle(setting.Repository.PullRequest.DefaultMergeStyle), + AllowRebaseUpdate: true, + }, + }) + } else if tp == unit.TypeProjects { + units = append(units, repo_model.RepoUnit{ + RepoID: repo.ID, + Type: tp, + Config: &repo_model.ProjectsConfig{ProjectsMode: repo_model.ProjectsModeAll}, + }) + } else { + units = append(units, repo_model.RepoUnit{ + RepoID: repo.ID, + Type: tp, + }) + } + } + + if err = db.Insert(ctx, units); err != nil { + return err + } + + // Remember visibility preference. + u.LastRepoVisibility = repo.IsPrivate + if err = user_model.UpdateUserCols(ctx, u, "last_repo_visibility"); err != nil { + return fmt.Errorf("UpdateUserCols: %w", err) + } + + if err = user_model.IncrUserRepoNum(ctx, u.ID); err != nil { + return fmt.Errorf("IncrUserRepoNum: %w", err) + } + u.NumRepos++ + + // Give access to all members in teams with access to all repositories. + if u.IsOrganization() { + teams, err := organization.FindOrgTeams(ctx, u.ID) + if err != nil { + return fmt.Errorf("FindOrgTeams: %w", err) + } + for _, t := range teams { + if t.IncludesAllRepositories { + if err := models.AddRepository(ctx, t, repo); err != nil { + return fmt.Errorf("AddRepository: %w", err) + } + } + } + + if isAdmin, err := access_model.IsUserRepoAdmin(ctx, repo, doer); err != nil { + return fmt.Errorf("IsUserRepoAdmin: %w", err) + } else if !isAdmin { + // Make creator repo admin if it wasn't assigned automatically + if err = AddOrUpdateCollaborator(ctx, repo, doer, perm.AccessModeAdmin); err != nil { + return fmt.Errorf("AddCollaborator: %w", err) + } + } + } else if err = access_model.RecalculateAccesses(ctx, repo); err != nil { + // Organization automatically called this in AddRepository method. + return fmt.Errorf("RecalculateAccesses: %w", err) + } + + if setting.Service.AutoWatchNewRepos { + if err = repo_model.WatchRepo(ctx, doer, repo, true); err != nil { + return fmt.Errorf("WatchRepo: %w", err) + } + } + + if err = webhook.CopyDefaultWebhooksToRepo(ctx, repo.ID); err != nil { + return fmt.Errorf("CopyDefaultWebhooksToRepo: %w", err) + } + + return nil +} diff --git a/services/repository/fork.go b/services/repository/fork.go index e114555679..5b24015a03 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -134,7 +134,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork }() err = db.WithTx(ctx, func(txCtx context.Context) error { - if err = repo_module.CreateRepositoryByExample(txCtx, doer, owner, repo, false, true); err != nil { + if err = CreateRepositoryByExample(txCtx, doer, owner, repo, false, true); err != nil { return err } diff --git a/services/repository/generate.go b/services/repository/generate.go index 2b95bbcd4d..f2280de8b2 100644 --- a/services/repository/generate.go +++ b/services/repository/generate.go @@ -343,7 +343,7 @@ func generateRepository(ctx context.Context, doer, owner *user_model.User, templ ObjectFormatName: templateRepo.ObjectFormatName, } - if err = repo_module.CreateRepositoryByExample(ctx, doer, owner, generateRepo, false, false); err != nil { + if err = CreateRepositoryByExample(ctx, doer, owner, generateRepo, false, false); err != nil { return nil, err } diff --git a/services/repository/transfer.go b/services/repository/transfer.go index 7ad6b46fa4..301d895337 100644 --- a/services/repository/transfer.go +++ b/services/repository/transfer.go @@ -20,7 +20,6 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/globallock" "code.gitea.io/gitea/modules/log" - repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/util" notify_service "code.gitea.io/gitea/services/notify" ) @@ -419,10 +418,7 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use return err } if !hasAccess { - if err := repo_module.AddCollaborator(ctx, repo, newOwner); err != nil { - return err - } - if err := repo_model.ChangeCollaborationAccessMode(ctx, repo, newOwner.ID, perm.AccessModeRead); err != nil { + if err := AddOrUpdateCollaborator(ctx, repo, newOwner, perm.AccessModeRead); err != nil { return err } } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 01b12460ac..b9fb1910a3 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -5095,7 +5095,7 @@ "tags": [ "repository" ], - "summary": "Add a collaborator to a repository", + "summary": "Add or Update a collaborator to a repository", "operationId": "repoAddCollaborator", "parameters": [ { From 145e26698791221b007c7dd460fb506cb0237235 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 7 Nov 2024 11:57:07 +0800 Subject: [PATCH 6/6] Support quote selected comments to reply (#32431) Many existing tests were quite hacky, these could be improved later.
![image](https://github.com/user-attachments/assets/93aebb4f-9de5-4cb8-910b-50c64cbcd25a)
--- modules/markup/html.go | 5 +- modules/markup/html_codepreview_test.go | 2 +- modules/markup/html_internal_test.go | 12 +- modules/markup/html_test.go | 16 ++- modules/markup/markdown/markdown_test.go | 12 +- modules/markup/sanitizer_default.go | 1 + modules/templates/util_render_test.go | 16 +-- routers/api/v1/misc/markup_test.go | 6 +- templates/repo/diff/comments.tmpl | 2 +- templates/repo/issue/view_content.tmpl | 2 +- .../repo/issue/view_content/comments.tmpl | 4 +- .../repo/issue/view_content/context_menu.tmpl | 18 +-- .../repo/issue/view_content/conversation.tmpl | 2 +- web_src/js/features/repo-issue-edit.ts | 84 ++++++++----- web_src/js/markup/html2markdown.test.ts | 24 ++++ web_src/js/markup/html2markdown.ts | 119 ++++++++++++++++++ 16 files changed, 255 insertions(+), 70 deletions(-) create mode 100644 web_src/js/markup/html2markdown.test.ts create mode 100644 web_src/js/markup/html2markdown.ts diff --git a/modules/markup/html.go b/modules/markup/html.go index a9c3dc9ba2..e2eefefc4b 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -442,7 +442,10 @@ func createLink(href, content, class string) *html.Node { a := &html.Node{ Type: html.ElementNode, Data: atom.A.String(), - Attr: []html.Attribute{{Key: "href", Val: href}}, + Attr: []html.Attribute{ + {Key: "href", Val: href}, + {Key: "data-markdown-generated-content"}, + }, } if class != "" { diff --git a/modules/markup/html_codepreview_test.go b/modules/markup/html_codepreview_test.go index d33630d040..a90de278f5 100644 --- a/modules/markup/html_codepreview_test.go +++ b/modules/markup/html_codepreview_test.go @@ -30,5 +30,5 @@ func TestRenderCodePreview(t *testing.T) { assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } test("http://localhost:3000/owner/repo/src/commit/0123456789/foo/bar.md#L10-L20", "

code preview

") - test("http://other/owner/repo/src/commit/0123456789/foo/bar.md#L10-L20", `

http://other/owner/repo/src/commit/0123456789/foo/bar.md#L10-L20

`) + test("http://other/owner/repo/src/commit/0123456789/foo/bar.md#L10-L20", `

http://other/owner/repo/src/commit/0123456789/foo/bar.md#L10-L20

`) } diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 74089cffdd..8f516751b0 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -33,11 +33,9 @@ func numericIssueLink(baseURL, class string, index int, marker string) string { // link an HTML link func link(href, class, contents string) string { - if class != "" { - class = " class=\"" + class + "\"" - } - - return fmt.Sprintf("%s", href, class, contents) + extra := ` data-markdown-generated-content=""` + extra += util.Iif(class != "", ` class="`+class+`"`, "") + return fmt.Sprintf(`%s`, href, extra, contents) } var numericMetas = map[string]string{ @@ -353,7 +351,9 @@ func TestRender_FullIssueURLs(t *testing.T) { Metas: localMetas, }, []processor{fullIssuePatternProcessor}, strings.NewReader(input), &result) assert.NoError(t, err) - assert.Equal(t, expected, result.String()) + actual := result.String() + actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") + assert.Equal(t, expected, actual) } test("Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6", "Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6") diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 32858dbd6b..82aded4407 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -116,7 +116,9 @@ func TestRender_CrossReferences(t *testing.T) { Metas: localMetas, }, input) assert.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) + actual := strings.TrimSpace(buffer) + actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") + assert.Equal(t, strings.TrimSpace(expected), actual) } test( @@ -156,7 +158,9 @@ func TestRender_links(t *testing.T) { }, }, input) assert.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) + actual := strings.TrimSpace(buffer) + actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") + assert.Equal(t, strings.TrimSpace(expected), actual) } oldCustomURLSchemes := setting.Markdown.CustomURLSchemes @@ -267,7 +271,9 @@ func TestRender_email(t *testing.T) { }, }, input) assert.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res)) + actual := strings.TrimSpace(res) + actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") + assert.Equal(t, strings.TrimSpace(expected), actual) } // Text that should be turned into email link @@ -616,7 +622,9 @@ func TestPostProcess_RenderDocument(t *testing.T) { Metas: localMetas, }, strings.NewReader(input), &res) assert.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res.String())) + actual := strings.TrimSpace(res.String()) + actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") + assert.Equal(t, strings.TrimSpace(expected), actual) } // Issue index shouldn't be post processing in a document. diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index cfb821ab19..ad38e7a088 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -311,7 +311,8 @@ func TestTotal_RenderWiki(t *testing.T) { IsWiki: true, }, sameCases[i]) assert.NoError(t, err) - assert.Equal(t, template.HTML(answers[i]), line) + actual := strings.ReplaceAll(string(line), ` data-markdown-generated-content=""`, "") + assert.Equal(t, answers[i], actual) } testCases := []string{ @@ -336,7 +337,8 @@ func TestTotal_RenderWiki(t *testing.T) { IsWiki: true, }, testCases[i]) assert.NoError(t, err) - assert.Equal(t, template.HTML(testCases[i+1]), line) + actual := strings.ReplaceAll(string(line), ` data-markdown-generated-content=""`, "") + assert.EqualValues(t, testCases[i+1], actual) } } @@ -356,7 +358,8 @@ func TestTotal_RenderString(t *testing.T) { Metas: localMetas, }, sameCases[i]) assert.NoError(t, err) - assert.Equal(t, template.HTML(answers[i]), line) + actual := strings.ReplaceAll(string(line), ` data-markdown-generated-content=""`, "") + assert.Equal(t, answers[i], actual) } testCases := []string{} @@ -996,7 +999,8 @@ space

for i, c := range cases { result, err := markdown.RenderString(&markup.RenderContext{Ctx: context.Background(), Links: c.Links, IsWiki: c.IsWiki}, input) assert.NoError(t, err, "Unexpected error in testcase: %v", i) - assert.Equal(t, c.Expected, string(result), "Unexpected result in testcase %v", i) + actual := strings.ReplaceAll(string(result), ` data-markdown-generated-content=""`, "") + assert.Equal(t, c.Expected, actual, "Unexpected result in testcase %v", i) } } diff --git a/modules/markup/sanitizer_default.go b/modules/markup/sanitizer_default.go index 669dc24eae..476ae5e26f 100644 --- a/modules/markup/sanitizer_default.go +++ b/modules/markup/sanitizer_default.go @@ -107,6 +107,7 @@ func (st *Sanitizer) createDefaultPolicy() *bluemonday.Policy { "start", "summary", "tabindex", "target", "title", "type", "usemap", "valign", "value", "vspace", "width", "itemprop", + "data-markdown-generated-content", } generalSafeElements := []string{ diff --git a/modules/templates/util_render_test.go b/modules/templates/util_render_test.go index 3e4ea04c63..0a6e89c5f2 100644 --- a/modules/templates/util_render_test.go +++ b/modules/templates/util_render_test.go @@ -129,18 +129,18 @@ com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit @mention-user test #123 space` - - assert.EqualValues(t, expected, newTestRenderUtils().RenderCommitBody(testInput(), testMetas)) + actual := strings.ReplaceAll(string(newTestRenderUtils().RenderCommitBody(testInput(), testMetas)), ` data-markdown-generated-content=""`, "") + assert.EqualValues(t, expected, actual) } func TestRenderCommitMessage(t *testing.T) { - expected := `space @mention-user ` + expected := `space @mention-user ` assert.EqualValues(t, expected, newTestRenderUtils().RenderCommitMessage(testInput(), testMetas)) } func TestRenderCommitMessageLinkSubject(t *testing.T) { - expected := `space @mention-user` + expected := `space @mention-user` assert.EqualValues(t, expected, newTestRenderUtils().RenderCommitMessageLinkSubject(testInput(), "https://example.com/link", testMetas)) } @@ -168,7 +168,8 @@ mail@domain.com space ` expected = strings.ReplaceAll(expected, "", " ") - assert.EqualValues(t, expected, newTestRenderUtils().RenderIssueTitle(testInput(), testMetas)) + actual := strings.ReplaceAll(string(newTestRenderUtils().RenderIssueTitle(testInput(), testMetas)), ` data-markdown-generated-content=""`, "") + assert.EqualValues(t, expected, actual) } func TestRenderMarkdownToHtml(t *testing.T) { @@ -193,7 +194,8 @@ com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit #123 space

` - assert.Equal(t, expected, string(newTestRenderUtils().MarkdownToHtml(testInput()))) + actual := strings.ReplaceAll(string(newTestRenderUtils().MarkdownToHtml(testInput())), ` data-markdown-generated-content=""`, "") + assert.Equal(t, expected, actual) } func TestRenderLabels(t *testing.T) { @@ -211,5 +213,5 @@ func TestRenderLabels(t *testing.T) { func TestUserMention(t *testing.T) { rendered := newTestRenderUtils().MarkdownToHtml("@no-such-user @mention-user @mention-user") - assert.EqualValues(t, `

@no-such-user @mention-user @mention-user

`, strings.TrimSpace(string(rendered))) + assert.EqualValues(t, `

@no-such-user @mention-user @mention-user

`, strings.TrimSpace(string(rendered))) } diff --git a/routers/api/v1/misc/markup_test.go b/routers/api/v1/misc/markup_test.go index e2ab7141b7..abffdf3516 100644 --- a/routers/api/v1/misc/markup_test.go +++ b/routers/api/v1/misc/markup_test.go @@ -38,7 +38,8 @@ func testRenderMarkup(t *testing.T, mode string, wiki bool, filePath, text, expe ctx, resp := contexttest.MockAPIContext(t, "POST /api/v1/markup") web.SetForm(ctx, &options) Markup(ctx) - assert.Equal(t, expectedBody, resp.Body.String()) + actual := strings.ReplaceAll(resp.Body.String(), ` data-markdown-generated-content=""`, "") + assert.Equal(t, expectedBody, actual) assert.Equal(t, expectedCode, resp.Code) resp.Body.Reset() } @@ -58,7 +59,8 @@ func testRenderMarkdown(t *testing.T, mode string, wiki bool, text, responseBody ctx, resp := contexttest.MockAPIContext(t, "POST /api/v1/markdown") web.SetForm(ctx, &options) Markdown(ctx) - assert.Equal(t, responseBody, resp.Body.String()) + actual := strings.ReplaceAll(resp.Body.String(), ` data-markdown-generated-content=""`, "") + assert.Equal(t, responseBody, actual) assert.Equal(t, responseCode, resp.Code) resp.Body.Reset() } diff --git a/templates/repo/diff/comments.tmpl b/templates/repo/diff/comments.tmpl index 0ed231b07e..2d716688b9 100644 --- a/templates/repo/diff/comments.tmpl +++ b/templates/repo/diff/comments.tmpl @@ -49,7 +49,7 @@ {{end}} {{end}} {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.root.RepoLink .ID)}} - {{template "repo/issue/view_content/context_menu" dict "ctxData" $.root "item" . "delete" true "issue" false "diff" true "IsCommentPoster" (and $.root.IsSigned (eq $.root.SignedUserID .PosterID))}} + {{template "repo/issue/view_content/context_menu" dict "item" . "delete" true "issue" false "diff" true "IsCommentPoster" (and $.root.IsSigned (eq $.root.SignedUserID .PosterID))}}
diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index 74440c5ef2..1f9bbd86aa 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -48,7 +48,7 @@ {{if not $.Repository.IsArchived}} {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index)}} {{end}} - {{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" .Issue "delete" false "issue" true "diff" false "IsCommentPoster" $.IsIssuePoster}} + {{template "repo/issue/view_content/context_menu" dict "item" .Issue "delete" false "issue" true "diff" false "IsCommentPoster" $.IsIssuePoster}}
diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 3047533154..477b6b33c6 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -55,7 +55,7 @@ {{if not $.Repository.IsArchived}} {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} {{end}} - {{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" . "delete" true "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} + {{template "repo/issue/view_content/context_menu" dict "item" . "delete" true "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}}
@@ -430,7 +430,7 @@ {{template "repo/issue/view_content/show_role" dict "ShowRole" .ShowRole}} {{if not $.Repository.IsArchived}} {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} - {{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" . "delete" false "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} + {{template "repo/issue/view_content/context_menu" dict "item" . "delete" false "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} {{end}}
diff --git a/templates/repo/issue/view_content/context_menu.tmpl b/templates/repo/issue/view_content/context_menu.tmpl index 9e38442c36..749a2fa0dd 100644 --- a/templates/repo/issue/view_content/context_menu.tmpl +++ b/templates/repo/issue/view_content/context_menu.tmpl @@ -5,29 +5,29 @@
{{$referenceUrl := ""}} {{if .issue}} - {{$referenceUrl = printf "%s#%s" .ctxData.Issue.Link .item.HashTag}} + {{$referenceUrl = printf "%s#%s" ctx.RootData.Issue.Link .item.HashTag}} {{else}} - {{$referenceUrl = printf "%s/files#%s" .ctxData.Issue.Link .item.HashTag}} + {{$referenceUrl = printf "%s/files#%s" ctx.RootData.Issue.Link .item.HashTag}} {{end}}
{{ctx.Locale.Tr "repo.issues.context.copy_link"}}
- {{if .ctxData.IsSigned}} + {{if ctx.RootData.IsSigned}} {{$needDivider := false}} - {{if not .ctxData.Repository.IsArchived}} + {{if not ctx.RootData.Repository.IsArchived}} {{$needDivider = true}}
{{ctx.Locale.Tr "repo.issues.context.quote_reply"}}
{{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}}
{{ctx.Locale.Tr "repo.issues.context.reference_issue"}}
{{end}} - {{if or .ctxData.Permission.IsAdmin .IsCommentPoster .ctxData.HasIssuesOrPullsWritePermission}} + {{if or ctx.RootData.Permission.IsAdmin .IsCommentPoster ctx.RootData.HasIssuesOrPullsWritePermission}}
{{ctx.Locale.Tr "repo.issues.context.edit"}}
{{if .delete}} -
{{ctx.Locale.Tr "repo.issues.context.delete"}}
+
{{ctx.Locale.Tr "repo.issues.context.delete"}}
{{end}} {{end}} {{end}} - {{$canUserBlock := call .ctxData.CanBlockUser .ctxData.SignedUser .item.Poster}} - {{$canOrgBlock := and .ctxData.Repository.Owner.IsOrganization (call .ctxData.CanBlockUser .ctxData.Repository.Owner .item.Poster)}} + {{$canUserBlock := call ctx.RootData.CanBlockUser ctx.RootData.SignedUser .item.Poster}} + {{$canOrgBlock := and ctx.RootData.Repository.Owner.IsOrganization (call ctx.RootData.CanBlockUser ctx.RootData.Repository.Owner .item.Poster)}} {{if or $canOrgBlock $canUserBlock}} {{if $needDivider}}
@@ -36,7 +36,7 @@
{{ctx.Locale.Tr "user.block.block.user"}}
{{end}} {{if $canOrgBlock}} -
{{ctx.Locale.Tr "user.block.block.org"}}
+
{{ctx.Locale.Tr "user.block.block.org"}}
{{end}} {{end}} {{end}} diff --git a/templates/repo/issue/view_content/conversation.tmpl b/templates/repo/issue/view_content/conversation.tmpl index fd657fb66a..14803298b8 100644 --- a/templates/repo/issue/view_content/conversation.tmpl +++ b/templates/repo/issue/view_content/conversation.tmpl @@ -83,7 +83,7 @@ {{template "repo/issue/view_content/show_role" dict "ShowRole" .ShowRole}} {{if not $.Repository.IsArchived}} {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} - {{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} + {{template "repo/issue/view_content/context_menu" dict "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} {{end}}
diff --git a/web_src/js/features/repo-issue-edit.ts b/web_src/js/features/repo-issue-edit.ts index af97ee4eab..9d146951bd 100644 --- a/web_src/js/features/repo-issue-edit.ts +++ b/web_src/js/features/repo-issue-edit.ts @@ -1,4 +1,3 @@ -import $ from 'jquery'; import {handleReply} from './repo-issue.ts'; import {getComboMarkdownEditor, initComboMarkdownEditor, ComboMarkdownEditor} from './comp/ComboMarkdownEditor.ts'; import {POST} from '../modules/fetch.ts'; @@ -7,11 +6,14 @@ import {hideElem, querySingleVisibleElem, showElem} from '../utils/dom.ts'; import {attachRefIssueContextPopup} from './contextpopup.ts'; import {initCommentContent, initMarkupContent} from '../markup/content.ts'; import {triggerUploadStateChanged} from './comp/EditorUpload.ts'; +import {convertHtmlToMarkdown} from '../markup/html2markdown.ts'; -async function onEditContent(event) { - event.preventDefault(); +async function tryOnEditContent(e) { + const clickTarget = e.target.closest('.edit-content'); + if (!clickTarget) return; - const segment = this.closest('.header').nextElementSibling; + e.preventDefault(); + const segment = clickTarget.closest('.header').nextElementSibling; const editContentZone = segment.querySelector('.edit-content-zone'); const renderContent = segment.querySelector('.render-content'); const rawContent = segment.querySelector('.raw-content'); @@ -102,33 +104,53 @@ async function onEditContent(event) { triggerUploadStateChanged(comboMarkdownEditor.container); } +function extractSelectedMarkdown(container: HTMLElement) { + const selection = window.getSelection(); + if (!selection.rangeCount) return ''; + const range = selection.getRangeAt(0); + if (!container.contains(range.commonAncestorContainer)) return ''; + + // todo: if commonAncestorContainer parent has "[data-markdown-original-content]" attribute, use the parent's markdown content + // otherwise, use the selected HTML content and respect all "[data-markdown-original-content]/[data-markdown-generated-content]" attributes + const contents = selection.getRangeAt(0).cloneContents(); + const el = document.createElement('div'); + el.append(contents); + return convertHtmlToMarkdown(el); +} + +async function tryOnQuoteReply(e) { + const clickTarget = (e.target as HTMLElement).closest('.quote-reply'); + if (!clickTarget) return; + + e.preventDefault(); + const contentToQuoteId = clickTarget.getAttribute('data-target'); + const targetRawToQuote = document.querySelector(`#${contentToQuoteId}.raw-content`); + const targetMarkupToQuote = targetRawToQuote.parentElement.querySelector('.render-content.markup'); + let contentToQuote = extractSelectedMarkdown(targetMarkupToQuote); + if (!contentToQuote) contentToQuote = targetRawToQuote.textContent; + const quotedContent = `${contentToQuote.replace(/^/mg, '> ')}\n`; + + let editor; + if (clickTarget.classList.contains('quote-reply-diff')) { + const replyBtn = clickTarget.closest('.comment-code-cloud').querySelector('button.comment-form-reply'); + editor = await handleReply(replyBtn); + } else { + // for normal issue/comment page + editor = getComboMarkdownEditor(document.querySelector('#comment-form .combo-markdown-editor')); + } + + if (editor.value()) { + editor.value(`${editor.value()}\n\n${quotedContent}`); + } else { + editor.value(quotedContent); + } + editor.focus(); + editor.moveCursorToEnd(); +} + export function initRepoIssueCommentEdit() { - // Edit issue or comment content - $(document).on('click', '.edit-content', onEditContent); - - // Quote reply - $(document).on('click', '.quote-reply', async function (event) { - event.preventDefault(); - const target = this.getAttribute('data-target'); - const quote = document.querySelector(`#${target}`).textContent.replace(/\n/g, '\n> '); - const content = `> ${quote}\n\n`; - - let editor; - if (this.classList.contains('quote-reply-diff')) { - const replyBtn = this.closest('.comment-code-cloud').querySelector('button.comment-form-reply'); - editor = await handleReply(replyBtn); - } else { - // for normal issue/comment page - editor = getComboMarkdownEditor($('#comment-form .combo-markdown-editor')); - } - if (editor) { - if (editor.value()) { - editor.value(`${editor.value()}\n\n${content}`); - } else { - editor.value(content); - } - editor.focus(); - editor.moveCursorToEnd(); - } + document.addEventListener('click', (e) => { + tryOnEditContent(e); // Edit issue or comment content + tryOnQuoteReply(e); // Quote reply to the comment editor }); } diff --git a/web_src/js/markup/html2markdown.test.ts b/web_src/js/markup/html2markdown.test.ts new file mode 100644 index 0000000000..99a63956a0 --- /dev/null +++ b/web_src/js/markup/html2markdown.test.ts @@ -0,0 +1,24 @@ +import {convertHtmlToMarkdown} from './html2markdown.ts'; +import {createElementFromHTML} from '../utils/dom.ts'; + +const h = createElementFromHTML; + +test('convertHtmlToMarkdown', () => { + expect(convertHtmlToMarkdown(h(`

h

`))).toBe('# h'); + expect(convertHtmlToMarkdown(h(`txt`))).toBe('**txt**'); + expect(convertHtmlToMarkdown(h(`txt`))).toBe('_txt_'); + expect(convertHtmlToMarkdown(h(`txt`))).toBe('~~txt~~'); + + expect(convertHtmlToMarkdown(h(`txt`))).toBe('[txt](link)'); + expect(convertHtmlToMarkdown(h(`https://link`))).toBe('https://link'); + + expect(convertHtmlToMarkdown(h(``))).toBe('![image](link)'); + expect(convertHtmlToMarkdown(h(`name`))).toBe('![name](link)'); + expect(convertHtmlToMarkdown(h(``))).toBe('image'); + + expect(convertHtmlToMarkdown(h(`

txt

`))).toBe('txt\n'); + expect(convertHtmlToMarkdown(h(`
a\nb
`))).toBe('> a\n> b\n'); + + expect(convertHtmlToMarkdown(h(`
  1. a
    • b
`))).toBe('1. a\n * b\n\n'); + expect(convertHtmlToMarkdown(h(`
  1. a
`))).toBe('1. [x] a\n'); +}); diff --git a/web_src/js/markup/html2markdown.ts b/web_src/js/markup/html2markdown.ts new file mode 100644 index 0000000000..c690e0c8b1 --- /dev/null +++ b/web_src/js/markup/html2markdown.ts @@ -0,0 +1,119 @@ +import {htmlEscape} from 'escape-goat'; + +type Processors = { + [tagName: string]: (el: HTMLElement) => string | HTMLElement | void; +} + +type ProcessorContext = { + elementIsFirst: boolean; + elementIsLast: boolean; + listNestingLevel: number; +} + +function prepareProcessors(ctx:ProcessorContext): Processors { + const processors = { + H1(el) { + const level = parseInt(el.tagName.slice(1)); + el.textContent = `${'#'.repeat(level)} ${el.textContent.trim()}`; + }, + STRONG(el) { + return `**${el.textContent}**`; + }, + EM(el) { + return `_${el.textContent}_`; + }, + DEL(el) { + return `~~${el.textContent}~~`; + }, + + A(el) { + const text = el.textContent || 'link'; + const href = el.getAttribute('href'); + if (/^https?:/.test(text) && text === href) { + return text; + } + return href ? `[${text}](${href})` : text; + }, + IMG(el) { + const alt = el.getAttribute('alt') || 'image'; + const src = el.getAttribute('src'); + const widthAttr = el.hasAttribute('width') ? ` width="${htmlEscape(el.getAttribute('width') || '')}"` : ''; + const heightAttr = el.hasAttribute('height') ? ` height="${htmlEscape(el.getAttribute('height') || '')}"` : ''; + if (widthAttr || heightAttr) { + return `${htmlEscape(alt)}`; + } + return `![${alt}](${src})`; + }, + + P(el) { + el.textContent = `${el.textContent}\n`; + }, + BLOCKQUOTE(el) { + el.textContent = `${el.textContent.replace(/^/mg, '> ')}\n`; + }, + + OL(el) { + const preNewLine = ctx.listNestingLevel ? '\n' : ''; + el.textContent = `${preNewLine}${el.textContent}\n`; + }, + LI(el) { + const parent = el.parentNode; + const bullet = parent.tagName === 'OL' ? `1. ` : '* '; + const nestingIdentLevel = Math.max(0, ctx.listNestingLevel - 1); + el.textContent = `${' '.repeat(nestingIdentLevel * 4)}${bullet}${el.textContent}${ctx.elementIsLast ? '' : '\n'}`; + return el; + }, + INPUT(el) { + return el.checked ? '[x] ' : '[ ] '; + }, + + CODE(el) { + const text = el.textContent; + if (el.parentNode && el.parentNode.tagName === 'PRE') { + el.textContent = `\`\`\`\n${text}\n\`\`\`\n`; + return el; + } + if (text.includes('`')) { + return `\`\` ${text} \`\``; + } + return `\`${text}\``; + }, + }; + processors['UL'] = processors.OL; + for (let level = 2; level <= 6; level++) { + processors[`H${level}`] = processors.H1; + } + return processors; +} + +function processElement(ctx :ProcessorContext, processors: Processors, el: HTMLElement) { + if (el.hasAttribute('data-markdown-generated-content')) return el.textContent; + if (el.tagName === 'A' && el.children.length === 1 && el.children[0].tagName === 'IMG') { + return processElement(ctx, processors, el.children[0] as HTMLElement); + } + + const isListContainer = el.tagName === 'OL' || el.tagName === 'UL'; + if (isListContainer) ctx.listNestingLevel++; + for (let i = 0; i < el.children.length; i++) { + ctx.elementIsFirst = i === 0; + ctx.elementIsLast = i === el.children.length - 1; + processElement(ctx, processors, el.children[i] as HTMLElement); + } + if (isListContainer) ctx.listNestingLevel--; + + if (processors[el.tagName]) { + const ret = processors[el.tagName](el); + if (ret && ret !== el) { + el.replaceWith(typeof ret === 'string' ? document.createTextNode(ret) : ret); + } + } +} + +export function convertHtmlToMarkdown(el: HTMLElement): string { + const div = document.createElement('div'); + div.append(el); + const ctx = {} as ProcessorContext; + ctx.listNestingLevel = 0; + processElement(ctx, prepareProcessors(ctx), el); + return div.textContent; +}