Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/master' into default-bids-cache-ttl
Browse files Browse the repository at this point in the history
  • Loading branch information
CTMBNara committed Dec 3, 2024
2 parents ea43526 + 5fa096a commit db519ab
Show file tree
Hide file tree
Showing 295 changed files with 11,484 additions and 1,664 deletions.
18 changes: 6 additions & 12 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,40 +1,34 @@
### 🔧 Type of changes
- [ ] new bid adapter
- [ ] update bid adapter
- [ ] bid adapter update
- [ ] new feature
- [ ] new analytics adapter
- [ ] new module
- [ ] module update
- [ ] bugfix
- [ ] documentation
- [ ] configuration
- [ ] dependency update
- [ ] tech debt (test coverage, refactorings, etc.)

### ✨ What's the context?

What's the context for the changes? Are there any

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
- [ ] verify email contact works
- [ ] NO fully dynamic hosts
- [ ] NO fully dynamic hostnames
- [ ] geographic host parameters are NOT required
- [ ] NO direct use of HTTP is prohibited - *implement an existing Bidder interface that will do all the job*
- [ ] direct use of HTTP is prohibited - *implement an existing Bidder interface that will do all the job*
- [ ] if the ORTB is just forwarded to the endpoint, use the generic adapter - *define the new adapter as the alias of the generic adapter*
- [ ] cover an adapter configuration with an integration test


### 🧪 Test plan

How do you know the changes are safe to ship to production?


### 🏎 Quality check

- [ ] Are your changes following [our code style guidelines](https://github.com/prebid/prebid-server-java/blob/master/docs/developers/code-style.md)?
- [ ] Are there any breaking changes in your code?
- [ ] Does your test coverage exceed 90%?
Expand Down
7 changes: 6 additions & 1 deletion docs/application-settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@ There are two ways to configure application settings: database and file. This do
operational warning.
- "enforce": if a bidder returns a creative that's larger in height or width than any of the allowed sizes, reject
the bid and log an operational warning.
- `auction.bidadjustments` - configuration JSON for default bid adjustments
- `auction.bidadjustments.mediatype.{banner, video-instream, video-outstream, audio, native, *}.{<BIDDER>, *}.{<DEAL_ID>, *}[]` - array of bid adjustment to be applied to any bid of the provided mediatype, <BIDDER> and <DEAL_ID> (`*` means ANY)
- `auction.bidadjustments.mediatype.*.*.*[].adjtype` - type of the bid adjustment (cpm, multiplier, static)
- `auction.bidadjustments.mediatype.*.*.*[].value` - value of the bid adjustment
- `auction.bidadjustments.mediatype.*.*.*[].currency` - currency of the bid adjustment
- `auction.events.enabled` - enables events for account if true
- `auction.price-floors.enabeled` - enables price floors for account if true. Defaults to true.
- `auction.price-floors.enabled` - enables price floors for account if true. Defaults to true.
- `auction.price-floors.fetch.enabled`- enables data fetch for price floors for account if true. Defaults to false.
- `auction.price-floors.fetch.url` - url to fetch price floors data from.
- `auction.price-floors.fetch.timeout-ms` - timeout for fetching price floors data. Defaults to 5000.
Expand Down
38 changes: 16 additions & 22 deletions docs/developers/code-reviews.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,21 @@
## Standards
Anyone is free to review and comment on any [open pull requests](https://github.com/prebid/prebid-server-java/pulls).

All pull requests must be reviewed and approved by at least one [core member](https://github.com/orgs/prebid/teams/core/members) before merge.

Very small pull requests may be merged with just one review if they:

1. Do not change the public API.
2. Have low risk of bugs, in the opinion of the reviewer.
3. Introduce no new features, or impact the code architecture.

Larger pull requests must meet at least one of the following two additional requirements.

1. Have a second approval from a core member
2. Be open for 5 business days with no new changes requested.
1. PRs that touch only adapters and modules can be approved by one reviewer before merge.
2. PRs that touch PBS-core must be reviewed and approved by at least two 'core' reviewers before merge.

## Process

New pull requests should be [assigned](https://help.github.com/articles/assigning-issues-and-pull-requests-to-other-github-users/)
to a core member for review within 3 business days of being opened.
That person should either approve the changes or request changes within 4 business days of being assigned.
If they're too busy, they should assign it to someone else who can review it within that timeframe.
New pull requests must be [assigned](https://help.github.com/articles/assigning-issues-and-pull-requests-to-other-github-users/)
to a reviewer within 5 business days of being opened. That person must either approve the changes or request changes within 5 business days of being assigned.

If a reviewer is too busy, they should re-assign it to someone else as soon as possible so that person has enough time to take over the review and still meet the 5-day goal. Please tag the new reviewer in the PR. If you don't know who to assign it to, use the #prebid-server-java-dev Slack channel to ask for help in re-assigning.

If the changes are small, that member can merge the PR once the changes are complete. Otherwise, they should
assign the pull request to another member for a second review.
If a reviewer is going to be unavailable for more than a few days, they should update the notes column of the duty spreadsheet or drop a note about their availability into the Slack channel.

The pull request can then be merged whenever the second reviewer approves, or if 5 business days pass with no farther
changes requested by anybody, whichever comes first.
After the review, if the PR touches PBS-core, it must be assigned to a second reviewer.

## Priorities
## Review Priorities

Code reviews should focus on things which cannot be validated by machines.

Expand All @@ -43,4 +31,10 @@ explaining it. Are there better ways to achieve those goals?
- Does the code use any global, mutable state? [Inject dependencies](https://en.wikipedia.org/wiki/Dependency_injection) instead!
- Can the code be organized into smaller, more modular pieces?
- Is there dead code which can be deleted? Or TODO comments which should be resolved?
- Look for code used by other adapters. Encourage adapter submitter to utilize common code.
- Look for code used by other adapters. Encourage adapter submitter to utilize common code.
- Specific bid adapter rules:
- The email contact must work and be a group, not an individual.
- Host endpoints cannot be fully dynamic. i.e. they can utilize "https://REGION.example.com", but not "https://HOST".
- They cannot _require_ a "region" parameter. Region may be an optional parameter, but must have a default.
- No direct use of HTTP is prohibited - *implement an existing Bidder interface that will do all the job*
- If the ORTB is just forwarded to the endpoint, use the generic adapter - *define the new adapter as the alias of the generic adapter*
56 changes: 28 additions & 28 deletions docs/developers/code-style.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ in `pom.xml` directly.

It is recommended to define version of library to separate property in `pom.xml`:

```
```xml
<project>
<properties>
<caffeine.version>2.6.2</caffeine.version>
Expand All @@ -48,7 +48,7 @@ It is recommended to define version of library to separate property in `pom.xml`

Do not use wildcard in imports because they hide what exactly is required by the class.

```
```java
// bad
import java.util.*;

Expand All @@ -61,7 +61,7 @@ import java.util.Map;

Prefer to use `camelCase` naming convention for variables and methods.

```
```java
// bad
String account_id = "id";

Expand All @@ -71,7 +71,7 @@ String accountId = "id";

Name of variable should be self-explanatory:

```
```java
// bad
String s = resolveParamA();

Expand All @@ -83,7 +83,7 @@ This helps other developers flesh your code out better without additional questi

For `Map`s it is recommended to use `To` between key and value designation:

```
```java
// bad
Map<Imp, ExtImp> map = getData();

Expand All @@ -97,7 +97,7 @@ Make data transfer object(DTO) classes immutable with static constructor.
This can be achieved by using Lombok and `@Value(staticConstructor="of")`. When constructor uses multiple(more than 4) arguments, use builder instead(`@Builder`).
If dto must be modified somewhere, use builders annotation `toBuilder=true` parameter and rebuild instance by calling `toBuilder()` method.

```
```java
// bad
public class MyDto {

Expand Down Expand Up @@ -138,7 +138,7 @@ final MyDto updatedDto = myDto.toBuilder().value("newValue").build();
Although Java supports the `var` keyword at the time of writing this documentation, the maintainers have chosen not to utilize it within the PBS codebase.
Instead, write full variable type.

```
```java
// bad
final var result = getResult();

Expand All @@ -150,7 +150,7 @@ final Data result = getResult();

Enclosing parenthesis should be placed on expression end.

```
```java
// bad
methodCall(
long list of arguments
Expand All @@ -163,7 +163,7 @@ methodCall(

This also applies for nested expressions.

```
```java
// bad
methodCall(
nestedCall(
Expand All @@ -181,7 +181,7 @@ methodCall(

Please, place methods inside a class in call order.

```
```java
// bad
public interface Test {

Expand Down Expand Up @@ -249,7 +249,7 @@ Define interface first method, then all methods that it is calling, then second
Not strict, but methods with long parameters list, that cannot be placed on single line,
should add empty line before body definition.

```
```java
// bad
public static void method(
parameters definitions) {
Expand All @@ -266,7 +266,7 @@ public static void method(

Use collection literals where it is possible to define and initialize collections.

```
```java
// bad
final List<String> foo = new ArrayList();
foo.add("foo");
Expand All @@ -278,7 +278,7 @@ final List<String> foo = List.of("foo", "bar");

Also, use special methods of Collections class for empty or single-value one-line collection creation. This makes developer intention clear and code less error-prone.

```
```java
// bad
return List.of();

Expand All @@ -296,7 +296,7 @@ return Collections.singletonList("foo");

It is recommended to declare variable as `final`- not strict but rather project convention to keep the code safe.

```
```java
// bad
String value = "value";

Expand All @@ -308,7 +308,7 @@ final String value = "value";

Results of long ternary operators should be on separate lines:

```
```java
// bad
boolean result = someVeryVeryLongConditionThatForcesLineWrap ? firstResult
: secondResult;
Expand All @@ -321,7 +321,7 @@ boolean result = someVeryVeryLongConditionThatForcesLineWrap

Not so strict, but short ternary operations should be on one line:

```
```java
// bad
boolean result = someShortCondition
? firstResult
Expand All @@ -335,7 +335,7 @@ boolean result = someShortCondition ? firstResult : secondResult;

Do not rely on operator precedence in boolean logic, use parenthesis instead. This will make code simpler and less error-prone.

```
```java
// bad
final boolean result = a && b || c;

Expand All @@ -347,7 +347,7 @@ final boolean result = (a && b) || c;

Try to avoid hard-readable multiple nested method calls:

```
```java
// bad
int resolvedValue = resolveValue(fetchExternalJson(url, httpClient), populateAdditionalKeys(mainKeys, keyResolver));

Expand All @@ -361,7 +361,7 @@ int resolvedValue = resolveValue(externalJson, additionalKeys);

Try not to retrieve same data more than once:

```
```java
// bad
if (getData() != null) {
final Data resolvedData = resolveData(getData());
Expand All @@ -380,7 +380,7 @@ if (data != null) {

If you're dealing with incoming data, please be sure to check if the nested object is not null before chaining.
```
```java
// bad
final ExtRequestTargeting targeting = bidRequest.getExt().getPrebid().getTargeting();
Expand All @@ -400,7 +400,7 @@ We are trying to get rid of long chains of null checks, which are described in s
Don't leave commented code (don't think about the future).
```
```java
// bad
// String iWillUseThisLater = "never";
```
Expand All @@ -426,7 +426,7 @@ The code should be covered over 90%.
The common way for writing tests has to comply with `given-when-then` style.
```
```java
// given
final BidRequest bidRequest = BidRequest.builder().id("").build();
Expand All @@ -451,7 +451,7 @@ The team decided to use name `target` for class instance under test.
Unit tests should be as granular as possible. Try to split unit tests into smaller ones until this is impossible to do.
```
```java
// bad
@Test
public void testFooBar() {
Expand Down Expand Up @@ -487,7 +487,7 @@ public void testBar() {
This also applies to cases where same method is tested with different arguments inside single unit test.
Note: This represents the replacement we have selected for parameterized testing.
```
```java
// bad
@Test
public void testFooFirstSecond() {
Expand Down Expand Up @@ -527,7 +527,7 @@ It is also recommended to structure test method names with this scheme:
name of method that is being tested, word `should`, what a method should return.
If a method should return something based on a certain condition, add word `when` and description of a condition.
```
```java
// bad
@Test
public void doSomethingTest() {
Expand All @@ -547,7 +547,7 @@ public void processDataShouldReturnResultWhenInputIsData() {
Place data used in test as close as possible to test code. This will make tests easier to read, review and understand.
```
```java
// bad
@Test
public void testFoo() {
Expand Down Expand Up @@ -576,7 +576,7 @@ This point also implies the next one.
Since we are trying to improve test simplicity and readability and place test data close to tests, we decided to avoid usage of top level constants where it is possible.
Instead, just inline constant values.
```
```java
// bad
public class TestClass {
Expand Down Expand Up @@ -609,7 +609,7 @@ public class TestClass {
Don't use real information in tests, like existing endpoint URLs, account IDs, etc.

```
```java
// bad
String ENDPOINT_URL = "https://prebid.org";

Expand Down
Loading

0 comments on commit db519ab

Please sign in to comment.