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

Save only the relative path of the filename for StaticDirStorage #1990

Merged
merged 23 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@ devenv.local.nix
result*

.idea

# Test folders
static/Test.FileStorage.ControllerFunctionsSpec
12 changes: 9 additions & 3 deletions IHP/FileStorage/ControllerFunctions.hs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ storeFileWithOptions fileInfo options = do
|> LBS.writeFile (cs destPath)

let frameworkConfig = ?context.frameworkConfig
pure $ frameworkConfig.baseUrl <> "/" <> objectPath
pure $ objectPath
S3Storage { connectInfo, bucket, baseUrl } -> do
let payload = fileInfo
|> (.fileContent)
Expand Down Expand Up @@ -226,7 +226,13 @@ createTemporaryDownloadUrlFromPathWithExpiredAt validInSeconds objectPath = do
case storage of
StaticDirStorage -> do
let frameworkConfig = ?context.frameworkConfig
let url = frameworkConfig.baseUrl <> "/" <> objectPath
let urlSchemes = ["http://", "https://"]

let url = if any (`isPrefixOf` objectPath) urlSchemes
-- BC, before we saved only the relative path of a file, we saved the full URL. So use it as is.
then objectPath
-- We have the relative path, so add the baseUrl.
else frameworkConfig.baseUrl <> "/" <> objectPath

pure TemporaryDownloadUrl { url = cs url, expiredAt = publicUrlExpiredAt }
S3Storage { connectInfo, bucket} -> do
Expand Down Expand Up @@ -409,4 +415,4 @@ storage = ?context.frameworkConfig.appConfig
storagePrefix :: (?context :: ControllerContext) => Text
storagePrefix = case storage of
StaticDirStorage -> "static/"
_ -> ""
_ -> ""
70 changes: 70 additions & 0 deletions Test/FileStorage/ControllerFunctionsSpec.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
module Test.FileStorage.ControllerFunctionsSpec where

import Test.Hspec
import IHP.Prelude
import IHP.FileStorage.ControllerFunctions
import IHP.Controller.Context
import IHP.FrameworkConfig
import Network.Wai as Wai (defaultRequest)
import Network.Wai.Parse (FileInfo(..))
import IHP.Controller.RequestContext
import IHP.FileStorage.Types
import IHP.FileStorage.Config

tests :: Spec
tests = describe "IHP.FileStorage.ControllerFunctions" $ do

let config :: ConfigBuilder
config = do
initStaticDirStorage

let withFrameworkConfig = IHP.FrameworkConfig.withFrameworkConfig config

describe "storeFileWithOptions" $ do
it "returns the objectPath without the baseUrl" $ do
withFrameworkConfig \frameworkConfig -> do
context <- createControllerContext frameworkConfig
let ?context = context

let fileInfo = FileInfo
{ fileName = "test.txt"
, fileContentType = "text/plain"
, fileContent = "Hello, world!"
}

-- We pass the UUID that will be used as the filename, so we can easily assert the objectPath.
let options :: StoreFileOptions = def
{ fileName = Just "4c55dac2-e411-45ac-aa10-b957b01221df"
, directory = "Test.FileStorage.ControllerFunctionsSpec"
}

result <- storeFileWithOptions fileInfo options

result.url `shouldBe` ("Test.FileStorage.ControllerFunctionsSpec/4c55dac2-e411-45ac-aa10-b957b01221df")
mpscholten marked this conversation as resolved.
Show resolved Hide resolved

describe "createTemporaryDownloadUrlFromPath" $ do
it "returns baseUrl concatenated with objectPath when objectPath does not start with http:// or https://" $ do
withFrameworkConfig \frameworkConfig -> do
context <- createControllerContext frameworkConfig
let ?context = context
let objectPath = "static/test.txt"
temporaryDownloadUrl <- createTemporaryDownloadUrlFromPath objectPath

temporaryDownloadUrl.url `shouldBe` "http://localhost:8000/static/test.txt"

it "returns the objectPath when objectPath starts with 'http://' or 'https://'" $ do
withFrameworkConfig \frameworkConfig -> do
context <- createControllerContext frameworkConfig
let ?context = context
let objectPath = "https://example.com/static/test.txt"
temporaryDownloadUrl <- createTemporaryDownloadUrlFromPath objectPath

temporaryDownloadUrl.url `shouldBe` "https://example.com/static/test.txt"

createControllerContext frameworkConfig = do
let
requestBody = FormBody { params = [], files = [] }
request = Wai.defaultRequest
requestContext = RequestContext { request, respond = error "respond", requestBody, frameworkConfig = frameworkConfig }
let ?requestContext = requestContext
newControllerContext
2 changes: 2 additions & 0 deletions Test/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import qualified Test.RouterSupportSpec
import qualified Test.ViewSupportSpec
import qualified Test.ServerSideComponent.HtmlParserSpec
import qualified Test.ServerSideComponent.HtmlDiffSpec
import qualified Test.FileStorage.ControllerFunctionsSpec
import qualified Test.FileStorage.MimeTypesSpec
import qualified Test.DataSync.DynamicQueryCompiler
import qualified Test.IDE.CodeGeneration.MigrationGenerator
Expand Down Expand Up @@ -78,6 +79,7 @@ main = hspec do
Test.ViewSupportSpec.tests
Test.ServerSideComponent.HtmlParserSpec.tests
Test.ServerSideComponent.HtmlDiffSpec.tests
Test.FileStorage.ControllerFunctionsSpec.tests
Test.FileStorage.MimeTypesSpec.tests
Test.DataSync.DynamicQueryCompiler.tests
Test.IDE.SchemaDesigner.SchemaOperationsSpec.tests
Expand Down