Skip to content

Ignite 12088#6884

Open
kcmvp wants to merge 5 commits intoapache:masterfrom
kcmvp:IGNITE-12088
Open

Ignite 12088#6884
kcmvp wants to merge 5 commits intoapache:masterfrom
kcmvp:IGNITE-12088

Conversation

@kcmvp
Copy link
Contributor

@kcmvp kcmvp commented Sep 19, 2019

Cache or template name should be validated before attempt to start, max valid name length is 235 characters (20 is reserved for internal prefixes, suffixes or extensions).

@kcmvp
Copy link
Contributor Author

kcmvp commented Sep 19, 2019

Copy link
Member

@Jokser Jokser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall fix looks good. Please resolve minor code style issues I've addressed in PR.

*/
public static void validateCacheName(String name) throws IllegalArgumentException {
A.ensure(name != null && !name.isEmpty(), "Cache name must not be null or empty.");
A.ensure(name.length() <= MAX_FILE_NAME_LENGTH, "Length of cache name can not exceed "+ MAX_FILE_NAME_LENGTH +".");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style issue - there should be whitespace between " and +

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

*/
public static void validateCacheGroupName(String name) throws IllegalArgumentException {
if (name != null)
A.ensure(name.length() <= MAX_FILE_NAME_LENGTH, "Length of cache group name can not exceed "+ MAX_FILE_NAME_LENGTH +".");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style issue - there should be whitespace between " and +

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

* @throws Exception If failed
*/
@Test
public void testGetorCreateLongCacheNameExceed235() throws Exception{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest renaming the test to smth. like testCreateLongCacheName and move it to org.apache.ignite.internal.processors.cache.GridCacheConfigurationConsistencySelfTest test class.

* @throws Exception If failed
*/
@Test
public void testGetorCreateLongCacheGroupNameExceed235() throws Exception{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest renaming the test to smth. like testCreateLongCacheGroupName and move it to org.apache.ignite.internal.processors.cache.GridCacheConfigurationConsistencySelfTest test class.

* @throws Exception If failed
*/
@Test
public void testGetorCreateLongCacheNameLessThan235() throws Exception{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think this test is needed


/** */
@Test
public void testGetorCreateLongCacheNameExceed235() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following 2 tests duplicate others. Should be removed.

@kcmvp
Copy link
Contributor Author

kcmvp commented Sep 19, 2019

fixed the review comments.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants