Deny creating subdirectories
Currently if we put a path separator in slug, "reno new" command creates subdirectories under the notes subdirectory. This usually results in the situation where the note file is put to an unexpected place. For example we have seen several cases with reno files placed under releasenote/notes/releasenote/notes directory because the notes subdirectory path is accidentally included in slug, like; $ reno new releasenotes/notes/something-new no configuration file in: ./releasenotes/config.yaml, ./reno.yaml Created new notes file in releasenotes/notes/releasenotes/notes/something-new-2ff053c99583fc69.yaml This change prohibits users from creating such subdirectries under the notes subdirectory. The previous behavior can be restored by the allow_subdirectories config option. Change-Id: I30b5de3d5f35abb3b903e9ed9e4db42671421e88
This commit is contained in:
parent
35c0a43c39
commit
7dc390e6a6
@ -31,6 +31,11 @@ _OPTIONS = [
|
||||
notes live.
|
||||
""")),
|
||||
|
||||
Opt('allow_subdirectories', False,
|
||||
textwrap.dedent("""\
|
||||
Allow creating subdirectories under the notes subdirectory.
|
||||
""")),
|
||||
|
||||
Opt('collapse_pre_releases', True,
|
||||
textwrap.dedent("""\
|
||||
Should pre-release versions be merged into the final release
|
||||
|
@ -64,6 +64,11 @@ def create_cmd(args, conf):
|
||||
# their local git tree, and so there should not be any concurrency
|
||||
# concern.
|
||||
slug = args.slug.replace(' ', '-')
|
||||
|
||||
if not conf.options['allow_subdirectories'] and os.sep in slug:
|
||||
raise ValueError('Slug should not include the path separator (%s)'
|
||||
% os.sep)
|
||||
|
||||
filename = _pick_note_file_name(conf.notespath, slug)
|
||||
encoding = conf.options['encoding']
|
||||
if args.from_template:
|
||||
|
@ -16,6 +16,7 @@ from unittest import mock
|
||||
|
||||
import fixtures
|
||||
import io
|
||||
import os
|
||||
|
||||
from reno import config
|
||||
from reno import create
|
||||
@ -72,7 +73,7 @@ class TestCreate(base.TestCase):
|
||||
args.edit = False
|
||||
conf = mock.create_autospec(config.Config)
|
||||
conf.notespath = self.tmpdir
|
||||
conf.options = {'encoding': None}
|
||||
conf.options = {'allow_subdirectories': False, 'encoding': None}
|
||||
with mock.patch('sys.stdout', new=io.StringIO()) as fake_out:
|
||||
create.create_cmd(args, conf)
|
||||
filename = self._get_file_path_from_output(fake_out.getvalue())
|
||||
@ -87,8 +88,34 @@ class TestCreate(base.TestCase):
|
||||
args.edit = False
|
||||
conf = mock.create_autospec(config.Config)
|
||||
conf.notespath = self.tmpdir
|
||||
conf.options = {'allow_subdirectories': False, 'encoding': None}
|
||||
self.assertRaises(ValueError, create.create_cmd, args, conf)
|
||||
|
||||
def test_create_from_user_template_fails_because_path_separator(self):
|
||||
args = mock.Mock()
|
||||
args.from_template = self._create_user_template('i-am-a-user-template')
|
||||
args.slug = 'the' + os.sep + 'slug'
|
||||
args.edit = False
|
||||
conf = mock.create_autospec(config.Config)
|
||||
conf.notespath = self.tmpdir
|
||||
conf.options = {'allow_subdirectories': False, 'encoding': None}
|
||||
self.assertRaises(ValueError, create.create_cmd, args, conf)
|
||||
|
||||
def test_create_from_template_with_path_separator_allowed(self):
|
||||
args = mock.Mock()
|
||||
args.from_template = self._create_user_template('i-am-a-user-template')
|
||||
args.slug = 'the' + os.sep + 'slug'
|
||||
args.edit = False
|
||||
conf = mock.create_autospec(config.Config)
|
||||
conf.notespath = self.tmpdir
|
||||
conf.options = {'allow_subdirectories': True, 'encoding': None}
|
||||
with mock.patch('sys.stdout', new=io.StringIO()) as fake_out:
|
||||
create.create_cmd(args, conf)
|
||||
filename = self._get_file_path_from_output(fake_out.getvalue())
|
||||
with open(filename, 'r') as f:
|
||||
body = f.read()
|
||||
self.assertEqual('i-am-a-user-template', body)
|
||||
|
||||
def test_edit(self):
|
||||
self.useFixture(fixtures.EnvironmentVariable('EDITOR', 'myeditor'))
|
||||
with mock.patch('subprocess.call') as call_mock:
|
||||
|
Loading…
Reference in New Issue
Block a user