benbjohnson/hashfs

Configure location of hash in filename.

Closed this issue · 3 comments

c9845 commented

Hi Ben,
I know there hasn't been any development on this package in years and not sure if you have much interest in it still. Just need to know if you would be interested in accepting pull-requests or if I should just fork this repo.


As of now, the hash is inserted at the first period in a file's name.
I.e.:

  • fontawesome-6.2.1.min.css becomes
  • fontawesome-6-a1b2c3...d4e5f6.2.1.min.css.

This doesn't break anything, but it is a bit ugly. It would be nice if we could configure the hash to be at the start of the filename instead of in the middle.

I am suggesting a package-level config such as:

type config struct {
    hashLocation string
}
var cfg = config{""}

Where hashLocation could be:

  • "" (empty string) meaning the current formatting is used,
  • "start" where the hash is prepended to the filename (separated with a dash, i.e.: a1b2c3...d4e5f6-fontawesome-6.2.1.min.css).
  • "end" could also be an option with the hash being placed at the end of the filename and the extension being copied after the hash (fontawesome-6.2.1.min.css-a1b2c3...d4e5f6.css).

FormatName() would then be rewritten with a switch to handle each of the hash locations.

The config file option could be set via something like SetHashLocation(s string) or a second parameter (or functional options [1]) in the call to NewFS().


Why a struct instead of just a var? Seems like a nicer way to organize, especially for future expansion of other options (cache control age?)

[1] https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis

c9845 commented

So, I worked on this a bit but got hung up around the exported funcs and if they all actually need to be exported. Do FormatName() and ParseName() really need to be exported? My issue is that, with a configurable hash location, another argument is needed for these funcs (they aren't methods on FS) which is a pain to keep track of (the below hashLocation... consts then need be exported). Regardless of this, I don't really see a reason either func needs to be exported, (*FS)HashName() or (*FS)ParseName() should be used instead regardless.

Here is what I have so far:

type FS struct {
    //existing fields...

    hl hashLocation
}

//Define consts for hash location in filename...
type hashLocation int

const (
	hashLocationStart       hashLocation = iota //script.min.js -> a1b2c3...d4e5f6.script.min.js
	hashLocationFirstPeriod                     //script.min.js -> script-a1b2c3...d4e5f6.min.js; original designed hash location
	hashLocationEnd                             //script.min.js -> script.min.a1b2c3...d4e5f6.js

	hashLocationDefault = hashLocationFirstPeriod
)

//NewFS is rewritten to accept functional options...
func NewFS(fsys fs.FS, options ...optionFunc) *FS {
	f := &FS{
		fsys: fsys,
		m:    make(map[string]string),
		r:    make(map[string][2]string),
		hl:   hashLocationDefault,
	}

	for _, option := range options {
		option(f)
	}

	return f
}

//Functional options are defined for each hash location...
func HashLocationStart() optionFunc {
	return func(f *FS) {
		f.hl = hashLocationStart
	}
}

//FormatName and ParseName are rewritten with an additional argument, the hash location, which is used in a switch to build/deconstruct the filename with/without the hash.  (FormatName and ParseName could be unexported in this change, otherwise this gets a bit messy [having to pass around the hash location]).
func FormatName(filename, hash string, hl hashLocation) string {
        //...

        switch hl {
	case hashLocationFirstPeriod:
		if i := strings.Index(base, "."); i != -1 {
			return path.Join(dir, fmt.Sprintf("%s-%s%s", base[:i], hash, base[i:]))
		}
		return path.Join(dir, fmt.Sprintf("%s-%s", base, hash))
	case hashLocationStart:
		return path.Join(dir, fmt.Sprintf("%s.%s", hash, base))
	case hashLocationEnd:
		return path.Join(dir, fmt.Sprintf("%s.%s%s", base, hash, path.Ext(base)))
	default:
		return ""
	}
}
c9845 commented

Functional options may seem like overkill here, they are only being used to add one feature, but doing so (1) doesn't break existing NewFS() calls, and (2) allows for future features to be added easily (random thoughts...using MD5 instead of SHA256 [AWS does this], changing the Max-Age header).

Closing this. I forked your repo and added the functionality (and some else). https://github.com/c9845/hashfs