Tests: Add profiles #4056
Tests: Add profiles #4056
Conversation
| import org.prebid.server.functional.model.request.profile.ProfileMergePrecedence | ||
| import org.prebid.server.functional.util.ObjectMapperWrapper | ||
|
|
||
| class ProfileMergePrecedenceConvert implements AttributeConverter<ProfileMergePrecedence, String>, ObjectMapperWrapper { |
There was a problem hiding this comment.
I hope you don't need the ObjectMapperWrapper dependency here
| import org.prebid.server.functional.model.request.profile.ProfileType | ||
| import org.prebid.server.functional.util.ObjectMapperWrapper | ||
|
|
||
| class ProfileTypeConvert implements AttributeConverter<ProfileType, String>, ObjectMapperWrapper { |
There was a problem hiding this comment.
Same here with ObjectMapperWrapper
|
|
||
| @ToString(includeNames = true, ignoreNulls = true) | ||
| @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy) | ||
| class FilesystemAccount { |
There was a problem hiding this comment.
Do we need such a class?(Don't see any use case)
| import org.prebid.server.functional.model.config.AccountConfig | ||
|
|
||
| @ToString(includeNames = true, ignoreNulls = true) | ||
| @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy) |
There was a problem hiding this comment.
Don't need a strategy for now
| PrebidOptions options | ||
|
|
||
| @JsonProperty("profiles") | ||
| List<String> profilesNames |
There was a problem hiding this comment.
Please rename to profileNames
| Boolean copyToContainer(String content, String containerPath) { | ||
| pbsContainer.copyToContainer(content, containerPath) | ||
| } |
| .replace("]", "_") | ||
| } | ||
|
|
||
| PrebidServerContainer copyToContainer(String content, String containerPath) { |
There was a problem hiding this comment.
Looks like we don't need a snippet of this code till to line 165
| final static <T> T decodeYaml(String yamlString, Class<T> clazz) { | ||
| yaml.readValue(yamlString, clazz) | ||
| } | ||
|
|
||
| final static <T> T decodeYaml(String yamlString, TypeReference<T> typeReference) { | ||
| yaml.readValue(yamlString, typeReference) | ||
| } | ||
|
|
||
| final static <T> T decodeYaml(InputStream inputStream, Class<T> clazz) { | ||
| yaml.readValue(inputStream, clazz) | ||
| } |
There was a problem hiding this comment.
I believe we can remove those three methods
| private static final ObjectMapper mapper = new ObjectMapper().setSerializationInclusion(NON_NULL) | ||
| .registerModule(new ZonedDateTimeModule()) | ||
| private static final YAMLMapper yaml = new YAMLMapper().setSerializationInclusion(NON_NULL) | ||
| .registerModule(new ZonedDateTimeModule()) as YAMLMapper |
There was a problem hiding this comment.
.registerModule(new ZonedDateTime Module()) can be removed since we don't use date time in serialisation
| accountId varchar(64) NOT NULL, | ||
| profileId varchar(128) NOT NULL, |
There was a problem hiding this comment.
I'd like to change of accountId varchar to 40 and the same for profileId.
|
|
||
| private static final ObjectMapper mapper = new ObjectMapper().setSerializationInclusion(NON_NULL) | ||
| .registerModule(new ZonedDateTimeModule()) | ||
| private static final YAMLMapper yaml = new YAMLMapper().setSerializationInclusion(NON_NULL) as YAMLMapper |
There was a problem hiding this comment.
Please rename yaml to yamlMapper
|
|
||
| private static final String REJECT_ERROR_MESSAGE = 'replace' | ||
| private static final String LIMIT_ERROR_MESSAGE = 'Profiles exceeded the limit.' | ||
| private static final String INVALID_REQEUST_PREFIX = 'Invalid request format: Error during processing profiles: ' |
There was a problem hiding this comment.
| private static final String INVALID_REQEUST_PREFIX = 'Invalid request format: Error during processing profiles: ' | |
| private static final String INVALID_REQUEST_PREFIX = 'Invalid request format: Error during processing profiles: ' |
| and: "Default account" | ||
| def account = new Account(uuid: bidRequest.accountId, status: ACTIVE) | ||
| accountDao.save(account) |
There was a problem hiding this comment.
We don't need this account for passing test
| ] | ||
|
|
||
| private static final Map<String, String> PROFILES_CONFIG = [ | ||
| 'adapters.openx.enabled' : "true", |
There was a problem hiding this comment.
Looks like we don't need this bidder
| } | ||
| } | ||
|
|
||
| def "PBS should skip invalid request profile from database when merge strategy #mergeStrategy"() { |
There was a problem hiding this comment.
Here, please check that bidder called
| assert !secondBidderImpBanner.btype | ||
| } | ||
|
|
||
| def "PBS should include invalid or missing profiles into limit count"() { |
There was a problem hiding this comment.
Literally can't understand what the test does
| given: "PBS with profiles.fail-on-unknown config" | ||
| def failOnUnknownProfilesConfig = new HashMap<>(PROFILES_CONFIG) | ||
| failOnUnknownProfilesConfig["auction.profiles.fail-on-unknown"] = "true" | ||
| def prebidServerService = pbsServiceFactory.getService(failOnUnknownProfilesConfig) |
There was a problem hiding this comment.
Can be simplified wihout creating a hashMap pbsServiceFactory.getService(PROFILES_CONFIG + ["auction.profiles.fail-on-unknown": true])
| and: "Flash metrics" | ||
| flushMetrics(prebidServerService) |
There was a problem hiding this comment.
Please remove for this case
| class RequestProfile extends Profile<BidRequest> { | ||
|
|
||
| static RequestProfile getProfile(String accountId = PBSUtils.randomString, | ||
| String name = PBSUtils.randomString, |
| and: "Default account" | ||
| def account = new Account(uuid: accountId, status: ACTIVE) | ||
| accountDao.save(account) |
| when: "PBS processes auction request" | ||
| def response = pbsWithStoredProfiles.sendAuctionRequest(bidRequest) | ||
|
|
||
| then: "No errors should be emitted in debug" |
There was a problem hiding this comment.
What does it mean in debug?
🔧 Type of changes
✨ What's the context?
What's the context for the changes?
🧠 Rationale behind the change
Why did you choose to make these changes? Were there any trade-offs you had to consider?
🔎 New Bid Adapter Checklist
🧪 Test plan
How do you know the changes are safe to ship to production?
🏎 Quality check