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

Add support for filesystem adapters #25

Closed
ejunker opened this issue Apr 29, 2016 · 11 comments
Closed

Add support for filesystem adapters #25

ejunker opened this issue Apr 29, 2016 · 11 comments

Comments

@ejunker
Copy link

ejunker commented Apr 29, 2016

This is based on discussion in #24 where I want to be able to write the sitemaps and index to Amazon S3.

I think a very simple interface like this may work:

interface FilesystemAdapterInterface
{
    /**
     * Create a file or update if exists.
     *
     * @param string $path     The path to the file.
     * @param string $contents The file contents.
     *
     * @return bool True on success, false on failure.
     */
    public function put($path, $contents);
}

And then a FilesystemAdapter could be passed into the constructor and used when writing files.

@samdark
Copy link
Owner

samdark commented Apr 29, 2016

No, it won't. If there are many URLs keeping it all in memory and then writing at once will cause huge memory usage.

@ejunker
Copy link
Author

ejunker commented Apr 29, 2016

Ok, adding an append() method would be useful

interface FilesystemAdapterInterface
{
    /**
     * Create a file or update if exists.
     *
     * @param string $path     The path to the file.
     * @param string $contents The file contents.
     *
     * @return bool True on success, false on failure.
     */
    public function put($path, $contents);

    /**
     * Append content to an existing file.
     *
     * @param string $path     The path to the file.
     * @param string $content The content to append.
     *
     * @return bool True on success, false on failure.
     */
    public function append($path, $content);
}

@samdark
Copy link
Owner

samdark commented Apr 29, 2016

How to handle overwriting of existing files?

@ejunker
Copy link
Author

ejunker commented Apr 29, 2016

put() is essentially file_put_contents() without the FILE_APPEND flag set. If path does not exist, the file is created. Otherwise, the existing file is overwritten.

@samdark
Copy link
Owner

samdark commented Apr 29, 2016

OK. Makes sense. @WinterSilence what do you think?

@WinterSilence
Copy link
Contributor

WinterSilence commented Apr 30, 2016

@samdark Maybe - need tests.
@ejunker Few changes:
name: FilesystemAdapterInterface to StorageInterface,
methods: put, append to write(mixed $data),read() or get\set + add __construct(array $config = []).

Class implemented StorageInterface contain all logic to working with data.
StorageInterface::__construct(array $config = []):

$sitemap = new Sitemap(new Storage\File([$filePath = '...', $maxUrls = 1000, $bufferSize = 1000]));
$sitemap = new Sitemap(new Storage\MyCache([$key = '...', $ttl = 3600]));

@samdark
Copy link
Owner

samdark commented Apr 30, 2016

How to get a list of file names written then?

@WinterSilence
Copy link
Contributor

WinterSilence commented Apr 30, 2016

@samdark
1.Use method read()
2. Add method getFiles() for Storage\File (alias for read()?)
3. Auto create sitemapindex if Sitemap::fileCount > 1

@samdark
Copy link
Owner

samdark commented Apr 30, 2016

That would do, yes.

@WinterSilence
Copy link
Contributor

@samdark What's next?

@samdark
Copy link
Owner

samdark commented Apr 30, 2016

If you have extra time to burn you may implement refactoring. Along with https://github.com/samdark/sitemap/tree/refactroring it could be released as a new major version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants