Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GO-5364 export markdown in memory #2262

Merged
merged 3 commits into from
Mar 28, 2025

Conversation

requilence
Copy link
Contributor

@requilence requilence commented Mar 25, 2025

Internal export API to export a single object into markdown byte slice. To be used in API

@requilence requilence changed the base branch from main to GO-4459-rest-api-docs March 25, 2025 14:15
Copy link

Testomat.io Report 🟢 SMOKE-TEST PASSED
Tests ✔️ 8 tests run
Summary 🟢 8 passed; 🟡 0 skipped
Duration 🕐 13 minutes, 1 seconds
Testomat.io Report 📊 Run #87155bbf
Job 🗂️ Smoke Tests / smoke-test
Operating System 🖥️ Linux X64

✅ Passed Tests (8)

  • Revoking an invite link (3 minutes, 4 seconds)
  • User cancels their join request (1 minute, 23 seconds)
  • User deletes the space and rejoins later (1 minute, 36 seconds)
  • Owner changes the rights of a user from Viewer to Editor (1 minute, 48 seconds)
  • Owner approves a join request with Editor permissions (1 minute, 32 seconds)
  • Owner declines a join request (1 minute, 8 seconds)
  • Owner removes a participant from the space (1 minute, 23 seconds)
  • Sync on staging nodes (1 minute, 4 seconds)

@jmetrikat jmetrikat requested review from jmetrikat and Copilot March 25, 2025 17:42
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements an internal export API to export a single object into a Markdown byte slice for the API. Key changes include:

  • Adding new export entries to the documentation and proto definitions.
  • Creating a new middleware handler (ObjectExport) and its supporting ExportSingleInMemory method.
  • Introducing an InMemoryWriter and deepLinkNamer to support in-memory and deep-linked exports.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
docs/proto.md Added new export-related entries and hyperlinks for RPC calls.
pkg/lib/mill/image_resize.go Introduced IsImageExt to support image extension checks.
core/export.go Added ObjectExport middleware and ExportSingleInMemory for in-memory exports.
pb/protos/commands.proto Added new Export Request/Response messages and updated comments in the proto.
core/block/export/export.go Implemented exportObject for synchronous single-object export using InMemoryWriter.
core/block/import/markdown/anymark/anyblocks.go Replaced hardcoded image extension list with mill.IsImageExt.
pb/protos/service/service.proto Added ObjectExport RPC definition.
core/block/export/writer.go Updated writer interface and added InMemoryWriter and deepLinkNamer for link exports.
Comments suppressed due to low confidence (2)

pb/protos/commands.proto:2879

  • [nitpick] Consider updating the comment to 'ID of document for export' to match the singular field name 'objectId' and improve clarity.
                // ids of documents for export, when empty - will export all available docs

core/block/export/writer.go:203

  • Ensure that the 'ext' parameter is consistently formatted (with or without a leading period) when used in condition checks. For example, if ext is not normalized prior to comparison, deepLinkNamer.Get may produce unexpected results.
if ext == ".md" {

Comment on lines +272 to +279
for _, v := range inMemoryWriter.data {
if e.format == model.Export_Protobuf {
return base64.StdEncoding.EncodeToString(v), nil
}
return string(v), nil
}

return "", fmt.Errorf("failed to find data in writer")
Copy link
Preview

Copilot AI Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop in exportObject returns on the first iteration; if multiple file entries are ever present, only the first will be processed. Consider asserting that only one file entry exists or explicitly handling multiple entries.

Suggested change
for _, v := range inMemoryWriter.data {
if e.format == model.Export_Protobuf {
return base64.StdEncoding.EncodeToString(v), nil
}
return string(v), nil
}
return "", fmt.Errorf("failed to find data in writer")
var result string
for _, v := range inMemoryWriter.data {
if e.format == model.Export_Protobuf {
result += base64.StdEncoding.EncodeToString(v)
} else {
result += string(v)
}
}
if result == "" {
return "", fmt.Errorf("failed to find data in writer")
}
return result, nil

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

@jmetrikat jmetrikat merged commit de031d6 into GO-4459-rest-api-docs Mar 28, 2025
3 checks passed
@jmetrikat jmetrikat deleted the go-5364-export-markdown-in-memory branch March 28, 2025 11:05
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants