Code review

This commit is contained in:
Asaf Gartner 2022-08-06 00:41:14 +03:00
parent 87a146dfa8
commit 89b1e48e69
6 changed files with 30 additions and 77 deletions

View File

@ -7464,24 +7464,6 @@ article code {
.h1-5 {
height: 1.5rem; }
.gap0 {
gap: 0; }
.gap1 {
gap: 0.25rem; }
.gap2 {
gap: 0.5rem; }
.gap3 {
gap: 1rem; }
.gap4 {
gap: 2rem; }
.gap5 {
gap: 4rem; }
.pre-line {
white-space: pre-line; }

View File

@ -11,7 +11,6 @@ import (
"strings"
"time"
"git.handmade.network/hmn/hmn/src/logging"
"git.handmade.network/hmn/hmn/src/utils"
)
@ -37,6 +36,9 @@ func IsUrlEmbeddable(u string) bool {
func GetEmbeddableFromUrls(ctx context.Context, urls []string, maxSize int, httpTimeout time.Duration, httpMaxAttempts int) (*Embeddable, error) {
embedError := NoEmbedFound
client := &http.Client{
Timeout: httpTimeout,
}
for _, urlStr := range urls {
u, err := url.Parse(urlStr)
if err != nil {
@ -60,7 +62,7 @@ func GetEmbeddableFromUrls(ctx context.Context, urls []string, maxSize int, http
if httpMaxAttempts > 0 {
httpMaxAttempts -= 1
embed, err := FetchEmbed(ctx, urlStr, httpTimeout, maxSize)
embed, err := FetchEmbed(ctx, urlStr, client, maxSize)
if err != nil {
embedError = err
continue
@ -77,34 +79,28 @@ func GetEmbeddableFromUrls(ctx context.Context, urls []string, maxSize int, http
// If the url points to a file, downloads and returns the file.
// If the url points to an html page, parses opengraph and tries to fetch an image/video/audio file according to that.
// maxSize only limits the actual filesize. In the case of html we always fetch up to 100kb even if maxSize is smaller.
func FetchEmbed(ctx context.Context, urlStr string, timeout time.Duration, maxSize int) (*Embed, error) {
logging.ExtractLogger(ctx).Debug().Msg("Fetching embed")
client := &http.Client{
Timeout: timeout,
}
func FetchEmbed(ctx context.Context, urlStr string, httpClient *http.Client, maxSize int) (*Embed, error) {
req, err := http.NewRequestWithContext(ctx, "GET", urlStr, nil)
if err != nil {
return nil, err
}
res, err := client.Do(req)
res, err := httpClient.Do(req)
if err != nil {
return nil, err
}
defer res.Body.Close()
if res.StatusCode < 200 || res.StatusCode > 299 {
return nil, NoEmbedFound
}
contentType := res.Header.Get("Content-Type")
logging.ExtractLogger(ctx).Debug().Str("type", contentType).Msg("Got first result")
if strings.HasPrefix(contentType, "text/html") || strings.HasPrefix(contentType, "application/html") {
var buffer bytes.Buffer
_, err := io.CopyN(&buffer, res.Body, 100*1024) // NOTE(asaf): If the opengraph stuff isn't in the first 100kb, we don't care.
res.Body.Close()
if err != nil && !errors.Is(err, io.EOF) {
return nil, err
}
partialHtml := buffer.Bytes()
urlStr = ExtractEmbedFromOpenGraph(partialHtml)
logging.ExtractLogger(ctx).Debug().Str("url", urlStr).Msg("Got ograph")
if urlStr == "" {
return nil, NoEmbedFound
}
@ -113,10 +109,11 @@ func FetchEmbed(ctx context.Context, urlStr string, timeout time.Duration, maxSi
if err != nil {
return nil, err
}
res, err = client.Do(req)
res, err = httpClient.Do(req)
if err != nil {
return nil, err
}
defer res.Body.Close()
if res.StatusCode < 200 || res.StatusCode > 299 {
return nil, NoEmbedFound
}
@ -125,7 +122,6 @@ func FetchEmbed(ctx context.Context, urlStr string, timeout time.Duration, maxSi
var buffer bytes.Buffer
n, err := io.CopyN(&buffer, res.Body, int64(maxSize+1))
res.Body.Close()
if err != nil && !errors.Is(err, io.EOF) {
return nil, err
}
@ -171,28 +167,27 @@ func ExtractEmbedFromOpenGraph(partialHtml []byte) string {
keyIdx := metaAttrRegex.SubexpIndex("key")
valueIdx := metaAttrRegex.SubexpIndex("value")
html := string(partialHtml)
matches := metaRegex.FindAllStringSubmatch(html, -1)
for _, m := range matches {
if len(m) > 1 {
content := ""
prop := ""
attrs := metaAttrRegex.FindAllStringSubmatch(m[1], -1)
for _, attr := range attrs {
key := attr[keyIdx]
value := attr[valueIdx]
if key == "name" || key == "property" {
for _, ogKey := range OGKeys {
if value == ogKey {
prop = value
}
metaTags := metaRegex.FindAllStringSubmatch(html, -1)
for _, m := range metaTags {
content := ""
relevantProp := false
attrs := metaAttrRegex.FindAllStringSubmatch(m[1], -1)
for _, attr := range attrs {
key := attr[keyIdx]
value := attr[valueIdx]
if key == "name" || key == "property" {
for _, ogKey := range OGKeys {
if value == ogKey {
relevantProp = true
break
}
} else if key == "content" {
content = value
}
} else if key == "content" {
content = value
}
if content != "" && prop != "" {
return content
}
}
if content != "" && relevantProp {
return content
}
}
return ""

View File

@ -337,30 +337,6 @@ article code {
height: 1.5rem;
}
.gap0 {
gap: $spacing-none;
}
.gap1 {
gap: $spacing-extra-small;
}
.gap2 {
gap: $spacing-small;
}
.gap3 {
gap: $spacing-medium;
}
.gap4 {
gap: $spacing-large;
}
.gap5 {
gap: $spacing-extra-large;
}
.pre-line {
white-space: pre-line;
}

View File

@ -30,7 +30,7 @@
<div class="pre-line overflow-auto" data-tmpl="description">
Unknown description
</div>
<div data-tmpl="projects" class="pt2 flex gap2"></div>
<div data-tmpl="projects" class="pt2 flex g2"></div>
<div class="i f7 pt2">
<a data-tmpl="discord_link" target="_blank">View original message on Discord</a>
</div>

View File

@ -28,7 +28,7 @@
</div>
</div>
</div>
<div data-tmpl="projectList" class="flex flex-wrap gap2"></div>
<div data-tmpl="projectList" class="flex flex-wrap g2"></div>
<div class="flex items-center">
<div data-tmpl="errors"></div>
<div class="flex-grow-1"></div>

View File

@ -69,7 +69,7 @@
{{ end }}
{{ with .Projects }}
<div class="mt3 flex gap2 projects">
<div class="mt3 flex g2 projects">
{{ range $i, $proj := . }}
<a data-projid="{{ $proj.ID }}" href="{{ $proj.Url }}" class="flex flex-row items-center bg-theme-dimmer ph2 pv1 br2">
<img src="{{ $proj.Logo }}" class="db mr1 br1 h1-5" />