add_agf changes and tests

src/GlassCat/generate.jl

@@ -2,19 +2,19 @@
 # Copyright (c) Microsoft Corporation. All rights reserved.
 # See LICENSE in the project root for full license information.
 
-using DelimitedFiles: readdlm # used in sourcefile_to_catalog
+using DelimitedFiles: readdlm # used in agffile_to_catalog
 using StringEncodings
 using StaticArrays
 using Unitful
 import Unitful: Length, Temperature, Quantity, Units
 
 """
-    generate_jls(sourcenames::Vector{<:AbstractString}, mainfile::AbstractString, jldir::AbstractString, sourcedir::AbstractString; test::Bool = false)
+    generate_jls(sourcenames::Vector{<:AbstractString}, mainfile::AbstractString, jldir::AbstractString, agfdir::AbstractString; test::Bool = false)
 
 Generates .jl files in `jldir`: a `mainfile` and several catalog files.
 
 Each catalog file is a module representing a distinct glass catalog (e.g. NIKON, SCHOTT), generated from corresponding
-AGF files in `sourcedir`. These are then included and exported in `mainfile`.
+AGF files in `agfdir`. These are then included and exported in `mainfile`.
 
 In order to avoid re-definition of constants `AGF_GLASS_NAMES` and `AGF_GLASSES` during testing, we have an optional
 `test` argument. If `true`, we generate a .jl file that defines glasses with `GlassType.TEST` to avoid namespace/ID
@@ -24,7 +24,7 @@ function generate_jls(
     sourcenames::Vector{<:AbstractString},
     mainfile::AbstractString,
     jldir::AbstractString,
-    sourcedir::AbstractString;
+    agfdir::AbstractString;
     test::Bool = false
 )
     glasstype = test ? "TEST" : "AGF"
@@ -34,9 +34,9 @@ function generate_jls(
 
     # generate several catalog files (.jl)
     for catalogname in sourcenames
-        # parse the sourcefile (.agf) into a catalog (native Julia dictionary)
-        sourcefile = joinpath(sourcedir, "$(catalogname).agf")
-        catalog = sourcefile_to_catalog(sourcefile)
+        # parse the agffile (.agf) into a catalog (native Julia dictionary)
+        agffile = joinpath(agfdir, "$(catalogname).agf")
+        catalog = agffile_to_catalog(agffile)
 
         # parse the catalog into a module string and write it to a catalog file (.jl)
         id, modstring = catalog_to_modstring(id, catalogname, catalog, glasstype)
@@ -68,7 +68,7 @@ function generate_jls(
 end
 
 """
-Parse a `sourcefile` (.agf) into a native Dict, `catalogdict`, where each `kvp = (glassname, glassinfo)` is a glass.
+Parse a `agffile` (.agf) into a native Dict, `catalogdict`, where each `kvp = (glassname, glassinfo)` is a glass.
 
 | 1  | 2        | 3         | 4     | 5     | 6                     | 7             | 8         | 9         | 10    | 11    |
 |:---|:---------|:----------|:------|:------|:----------------------|:--------------|:----------|:----------|:------|:------|
@@ -80,7 +80,7 @@ Parse a `sourcefile` (.agf) into a native Dict, `catalogdict`, where each `kvp =
 | LD | λmin     | λmax      |
 | IT | T1       | T2        | T3    |
 """
-function sourcefile_to_catalog(sourcefile::AbstractString)
+function agffile_to_catalog(agffile::AbstractString)
     catalogdict = Dict{String,Dict{String}}()
 
     # store persistent variables between loops
@@ -154,9 +154,9 @@ function sourcefile_to_catalog(sourcefile::AbstractString)
         rowbuffer = []
     end
 
-    is_utf8 = isvalid(readuntil(sourcefile, " "))
+    is_utf8 = isvalid(readuntil(agffile, " "))
     # use DelimitedFiles.readdlm to parse the source file conveniently (with type inference)
-    for line in eachrow(readdlm(sourcefile))
+    for line in eachrow(readdlm(agffile))
         for item in line
             if !is_utf8
                 item = decode(Vector{UInt8}(item), "UTF-16")

src/GlassCat/sources.jl

@@ -8,49 +8,65 @@ import ZipFile
 using Pkg
 
 """
-    add_agf(sourcefile::AbstractString; name::Union{Nothing,AbstractString} = nothing, rebuild::Bool = true)
+    add_agf(agffile; agfdir = AGF_DIR, sourcefile = SOURCES_PATH, name = nothing, rebuild = true)
 
-Adds an already downloaded AGF file to the sourcelist at data/sources.txt, generating the SHA256 checksum automatically.
+Copies a downloaded AGF file at `agffile` to `agfdir` and appends a corresponding entry to the source list at
+`sourcefile`.
 
-Optionally provide a `name` for the corresponding module, and `rebuild` AGFGlassCat.jl by default.
-"""
-function add_agf(sourcefile::AbstractString; name::Union{Nothing,AbstractString} = nothing, rebuild::Bool = true)
-    if !isfile(sourcefile)
-        @error "AGF file not found at $sourcefile"
-        return
-    end
+If a `name` is not provided for the catalog, an implicit name is derived from `agffile`.
 
-    # infer catalog name from sourcefile basename (alphabetical only)
+If `rebuild` is true, Pkg.build is called at the end to install the new catalog.
+"""
+function add_agf(
+    agffile::AbstractString;
+    agfdir::AbstractString = AGF_DIR,
+    sourcefile::AbstractString = SOURCES_PATH,
+    name::Union{Nothing, AbstractString} = nothing,
+    rebuild::Bool = true
+)
+    # check name
     if name === nothing
-        name = uppercase(match(r"^([a-zA-Z]+)\.agf$"i, basename(sourcefile))[1])
+        m = match(r"^([a-zA-Z]+)\.(agf|AGF)$", basename(agffile))
+        if m === nothing
+            @error "invalid implicit catalog name \"$(basename(agffile))\". Should be purely alphabetical with a .agf/.AGF extension."
+            return
+        end
+        name = m[1]
+    else
+        if match(r"^([a-zA-Z]+)$", name) === nothing
+            @error "invalid catalog name \"$name\". Should be purely alphabetical."
+        end
     end
-
-    # avoid duplicate catalog names
-    if name ∈ first.(split.(readlines(SOURCES_PATH)))
-        @error "Adding the catalog name \"$name\" would create a duplicate entry in sources.txt"
+    if name ∈ first.(split.(readlines(sourcefile)))
+        @error "adding the catalog name \"$name\" would create a duplicate entry in source file $sourcefile"
         return
     end
 
-    # copy sourcefile to correct location
-    mkpath(AGF_DIR)
-    cp(sourcefile, joinpath(AGF_DIR, name * ".agf"), force=true)
+    # copy agffile to agfdir
+    if !isfile(agffile)
+        @error "file not found at $agffile"
+        return
+    end
+    mkpath(agfdir)
+    cp(agffile, joinpath(agfdir, name * ".agf"), force=true)
 
-    # append a corresponding entry to sources.txt
-    sha256sum = SHA.bytes2hex(SHA.sha256(read(sourcefile)))
-    open(SOURCES_PATH, "a") do io
+    # append a corresponding entry to the source list at sourcefile
+    sha256sum = SHA.bytes2hex(SHA.sha256(read(agffile)))
+    open(sourcefile, "a") do io
         write(io, join([name, sha256sum], ' ') * '\n')
     end
 
+    # optional rebuild
     if rebuild
         @info "Re-building OpticSim.jl"
         Pkg.build("OpticSim"; verbose=true)
     end
 end
 
 """
-    verify_sources!(sources::AbstractVector{<:AbstractVector{<:AbstractString}}, sourcedir::AbstractString)
+    verify_sources!(sources::AbstractVector{<:AbstractVector{<:AbstractString}}, agfdir::AbstractString)
 
-Verify a list of `sources` located in `sourcedir`. If AGF files are missing or invalid, try to download them using the
+Verify a list of `sources` located in `agfdir`. If AGF files are missing or invalid, try to download them using the
 information provided in `sources`.
 
 Each `source ∈ sources` is a collection of strings in the format `name, sha256sum, url [, POST_data]`, where the last
@@ -59,18 +75,18 @@ sources (e.g. Sumita).
 
 Modifies `sources` in-place such that only verified sources remain.
 """
-function verify_sources!(sources::AbstractVector{<:AbstractVector{<:AbstractString}}, sourcedir::AbstractString)
+function verify_sources!(sources::AbstractVector{<:AbstractVector{<:AbstractString}}, agfdir::AbstractString)
     # track missing sources as we go and delete them afterwards to avoid modifying our iterator
     missing_sources = []
 
     for (i, source) in enumerate(sources)
         name, sha256sum = source[1:2]
-        sourcefile = joinpath(sourcedir, "$(name).agf")
-        verified = verify_source(sourcefile, sha256sum)
+        agffile = joinpath(agfdir, "$(name).agf")
+        verified = verify_source(agffile, sha256sum)
         if !verified && length(source) >= 3
             # try downloading and re-verifying the source if download information is provided (sources[3:end])
-            download_source(sourcefile, source[3:end]...)
-            verified = verify_source(sourcefile, sha256sum)
+            download_source(agffile, source[3:end]...)
+            verified = verify_source(agffile, sha256sum)
         end
         if !verified
             push!(missing_sources, i)
@@ -81,29 +97,29 @@ function verify_sources!(sources::AbstractVector{<:AbstractVector{<:AbstractStri
 end
 
 """
-    verify_source(sourcefile::AbstractString, sha256sum::AbstractString)
+    verify_source(agffile::AbstractString, sha256sum::AbstractString)
 
 Verify a source file using SHA256, returning true if successful. Otherwise, remove the file and return false.
 """
-function verify_source(sourcefile::AbstractString, sha256sum::AbstractString)
-    if !isfile(sourcefile)
-        @info "[-] Missing file at $sourcefile"
-    elseif sha256sum == SHA.bytes2hex(SHA.sha256(read(sourcefile)))
-        @info "[✓] Verified file at $sourcefile"
+function verify_source(agffile::AbstractString, sha256sum::AbstractString)
+    if !isfile(agffile)
+        @info "[-] Missing file at $agffile"
+    elseif sha256sum == SHA.bytes2hex(SHA.sha256(read(agffile)))
+        @info "[✓] Verified file at $agffile"
         return true
     else
-        @info "[x] Removing unverified file at $sourcefile"
-        rm(sourcefile)
+        @info "[x] Removing unverified file at $agffile"
+        rm(agffile)
     end
     return false
 end
 
 """
-    download_source(sourcefile::AbstractString, url::AbstractString, POST_data::Union{Nothing,AbstractString} = nothing)
+    download_source(agffile::AbstractString, url::AbstractString, POST_data::Union{Nothing,AbstractString} = nothing)
 
 Download and unzip an AGF glass catalog from a publicly available source. Supports POST requests.
 """
-function download_source(sourcefile::AbstractString, url::AbstractString, POST_data::Union{Nothing,AbstractString} = nothing)
+function download_source(agffile::AbstractString, url::AbstractString, POST_data::Union{Nothing,AbstractString} = nothing)
     @info "Downloading source file from $url"
     try
         headers = ["Content-Type" => "application/x-www-form-urlencoded"]
@@ -116,7 +132,7 @@ function download_source(sourcefile::AbstractString, url::AbstractString, POST_d
             reader = ZipFile.Reader(IOBuffer(resp.body))
             agfdata = read(reader.files[findfirst(f -> endswith(lowercase(f.name), ".agf"), reader.files)])
         end
-        write(sourcefile, agfdata)
+        write(agffile, agfdata)
     catch e
         @error e
     end

test/testsets/GlassCat.jl

@@ -23,6 +23,63 @@ using Unitful.DefaultSymbols
         @test !isnan(NIKON.LLF6.C10)
     end
 
+    @testset "sources.jl" begin
+        @testset "add_agf" begin
+            tmpdir = mktempdir()
+            agfdir = mktempdir(tmpdir)
+            sourcefile, _ = mktemp(tmpdir)
+
+            @testset "bad implicit catalog names" begin
+                for agffile in split("a 1.agf a.Agf")
+                    @test_logs (:error, "invalid implicit catalog name \"$agffile\". Should be purely alphabetical with a .agf/.AGF extension.") add_agf(agffile; agfdir, sourcefile)
+                end
+            end
+
+            @testset "file not found" begin
+                @test_logs (:error, "file not found at nonexistentfile.agf") add_agf("nonexistentfile.agf"; agfdir, sourcefile)
+            end
+
+            @testset "add to source file" begin
+                for name in split("a b")
+                    open(joinpath(tmpdir, "$name.agf"), "w") do io
+                        write(io, "")
+                    end
+                end
+                empty_sha = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
+
+                @test isempty(readlines(sourcefile))
+
+                add_agf(joinpath(tmpdir, "a.agf"); agfdir, sourcefile, rebuild=false)
+                @test length(readlines(sourcefile)) === 1
+                @test readlines(sourcefile)[1] === "a $empty_sha"
+
+                add_agf(joinpath(tmpdir, "b.agf"); agfdir, sourcefile, rebuild=false)
+                @test length(readlines(sourcefile)) === 2
+                @test readlines(sourcefile)[1] === "a $empty_sha"
+                @test readlines(sourcefile)[2] === "b $empty_sha"
+
+                @test_logs (:error, "adding the catalog name \"a\" would create a duplicate entry in source file $sourcefile") add_agf(joinpath(tmpdir, "a.agf"); agfdir, sourcefile)
+            end
+
+            # TODO rebuild=true
+        end
+
+        # integration test
+        @testset "verify_sources!" begin
+            tmpdir = mktempdir()
+            agfdir = mktempdir(tmpdir)
+
+            sources = split.(readlines(GlassCat.SOURCES_PATH))
+            GlassCat.verify_sources!(sources, agfdir)
+
+            @test first.(sources) == first.(split.(readlines(GlassCat.SOURCES_PATH)))
+
+            # TODO missing_sources
+        end
+
+        # TODO unit tests
+    end
+
     @testset "generate.jl" begin
         CATALOG_NAME = "TEST_CAT"
         SOURCE_DIR = joinpath(@__DIR__, "..", "..", "test")
@@ -79,7 +136,7 @@ using Unitful.DefaultSymbols
         FIELDS = names(TEST_CAT_VALUES)[2:end]
 
         @testset "Parsing Tests" begin
-            cat = GlassCat.sourcefile_to_catalog(SOURCE_FILE)
+            cat = GlassCat.agffile_to_catalog(SOURCE_FILE)
 
             for glass in eachrow(TEST_CAT_VALUES)
                 name = glass["name"]