Skip to content

added new config option to disallow manual specifying UUID#8

Merged
schernysh merged 3 commits intomasterfrom
configuration-to-prevent-UUID-specification
Oct 2, 2018
Merged

added new config option to disallow manual specifying UUID#8
schernysh merged 3 commits intomasterfrom
configuration-to-prevent-UUID-specification

Conversation

@dhutsalo
Copy link
Copy Markdown
Contributor

added new config option to disallow manual specifying UUID
slightly changed tests for more flexibility


private void validateUUID(final PayloadWrapper payload, final SynchronousSink<PayloadWrapper> sink) {
if(payload.isExternalId() && !config.isAllowExternalUUID()) {
sink.error(new InvalidUUIDException("You cant specify UUID manually."));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's better to choose another wording like "Prebid cache host forbids specifying UUID in request"

package org.prebid.cache.handlers;

import com.google.common.collect.ImmutableList;
import org.junit.jupiter.api.Nested;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unused import

@Test
void testExternalUUIDInvalid() {
//given
cacheConfig.setAllowExternalUUID(false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wouldn't do this since Spring Test could cache context and beans between tests so it's better to create new config object just for this test

schernysh
schernysh previously approved these changes Oct 2, 2018
@schernysh schernysh merged commit b1b139b into master Oct 2, 2018
@schernysh schernysh deleted the configuration-to-prevent-UUID-specification branch October 2, 2018 13:29
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.

3 participants