Use ranges

C++20 brought us std::ranges for a lot of algorithms. Most of these
conversions were done using comby, similar to:

```
comby -verbose 'std::lower_bound(:[a].begin(),:[b].end(),:[c])' 's

Use ranges

C++20 brought us std::ranges for a lot of algorithms. Most of these
conversions were done using comby, similar to:

```
comby -verbose 'std::lower_bound(:[a].begin(),:[b].end(),:[c])' 'std::ranges::lower_bound(:[a], :[c])' $(git ls-files | grep "\.[hc]\(pp\)\?$") -in-place
```

Change-Id: I0c99c04e9368312555c08147d474ca93a5959e8d
Signed-off-by: Ed Tanous <edtanous@google.com>

show more ...

Fix bugprone-unchecked-optional-access findings

Clang-tidy has the aforementioned check, which shows a few places in the
core where we ignored the required optional checks. Fix all uses.
Note, we c

Fix bugprone-unchecked-optional-access findings

Clang-tidy has the aforementioned check, which shows a few places in the
core where we ignored the required optional checks. Fix all uses.
Note, we cannot enable the check that this time because of some weird
code in health.hpp that crashes tidy[1]. That will need to be a future
improvement.

There are tests that call something like
ASSERT(optional)
EXPECT(optional->foo())

While this isn't an actual violation, clang-tidy doesn't seem to be
smart enough to deal with it, so add some explicit checks.

[1] https://github.com/llvm/llvm-project/issues/55530

Tested: Redfish service validator passes.

Change-Id: Ied579cd0b957efc81aff5d5d1091a740a7a2d7e3
Signed-off-by: Ed Tanous <edtanous@google.com>

show more ...

Replace logging with std::format

std::format is a much more modern logging solution, and gives us a lot
more flexibility, and better compile times when doing logging.

Unfortunately, given its level

Replace logging with std::format

std::format is a much more modern logging solution, and gives us a lot
more flexibility, and better compile times when doing logging.

Unfortunately, given its level of compile time checks, it needs to be a
method, instead of the stream style logging we had before. This
requires a pretty substantial change. Fortunately, this change can be
largely automated, via the script included in this commit under
scripts/replace_logs.py. This is to aid people in moving their
patchsets over to the new form in the short period where old patches
will be based on the old logging. The intention is that this script
eventually goes away.

The old style logging (stream based) looked like.

BMCWEB_LOG_DEBUG << "Foo " << foo;

The new equivalent of the above would be:
BMCWEB_LOG_DEBUG("Foo {}", foo);

In the course of doing this, this also cleans up several ignored linter
errors, including macro usage, and array to pointer deconstruction.

Note, This patchset does remove the timestamp from the log message. In
practice, this was duplicated between journald and bmcweb, and there's
no need for both to exist.

One design decision of note is the addition of logPtr. Because the
compiler can't disambiguate between const char* and const MyThing*, it's
necessary to add an explicit cast to void*. This is identical to how
fmt handled it.

Tested: compiled with logging meson_option enabled, and launched bmcweb

Saw the usual logging, similar to what was present before:
```
[Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled
[Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800
[Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist
[Info src/webserver_main.cpp:59] Starting webserver on port 18080
[Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file.
[Info src/webserver_main.cpp:137] Start Hostname Monitor Service...
```
Signed-off-by: Ed Tanous <ed@tanous.net>

Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8

show more ...

json utils: add getEstimatedJsonSize

Add a utility function which estimates the size of the JSON tree.

It is used in the children change to limit the reponse size of expand
query.

Tested:
1. unit

json utils: add getEstimatedJsonSize

Add a utility function which estimates the size of the JSON tree.

It is used in the children change to limit the reponse size of expand
query.

Tested:
1. unit test passed;
2. tested on hardware, the following are real sizes and estimation
```
Real payload size, Estimation, query
15.69 KB, 10.21 KB, redfish/v1?$expand=.($levels=1)
95.76 KB, 62.11 KB, redfish/v1?$expand=.($levels=2)
117.14 KB, 72.71 KB, redfish/v1?$expand=.($levels=3)
127.65 KB, 77.64 KB, redfish/v1?$expand=.($levels=4)
```

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Iae26d6732a6ec63ecc59eacf657b4bf33c07c046

show more ...

Make propertyValueTypeError more typesafe

Similar to the prior patchset in this series, propertyValueTypeError can
be moved to safer constructs. This ensures that we are minimizing how
many places

Make propertyValueTypeError more typesafe

Similar to the prior patchset in this series, propertyValueTypeError can
be moved to safer constructs. This ensures that we are minimizing how
many places we are calling dump() from, and allows us to reduce the
amount of code written for error handling.

Tested:
PATCH /redfish/v1/SessionService {"SessionTimeout": "foo"}

Returns PropertyValueTypeError in the same behavior as prior to this
patch.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iddff4b787f35c49bf923663d61bba156687f358c

show more ...

Make propertyValueNotInList typesafe

The error codes for this function accept a string_view, which has caused
a number of cases of users of this function to call dump() to_string()
and all manner of

Make propertyValueNotInList typesafe

The error codes for this function accept a string_view, which has caused
a number of cases of users of this function to call dump() to_string()
and all manner of other conversions. Considering that dump() is
something that's difficult to call correctly, and overly wordy, it would
be ideal if the message code just handled that for us.

Therefore, this commit changes the prototype to include a nlohmann::json
object as an argument instead of string_view, then audits the codebase
for all uses, and moves them to a more normalized usage, which allows
the calling code to call "dump" for them.

Tested: PATCH /redfish/v1/SessionService {"SessionTimeout": 1}

Returns the PropertyValueNotInList error as it did before.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If62909072db1f067ad1f8aa590bb716c84181219

show more ...

json utility: add sort

This commit adds a utility function |sortJsonArrayByKey|. It can sort an
json array by value of a given key of each element.

Use cases includes:
1. sort the MemberCollection

json utility: add sort

This commit adds a utility function |sortJsonArrayByKey|. It can sort an
json array by value of a given key of each element.

Use cases includes:
1. sort the MemberCollection by @odata.id

Tested:
1. unit test passed;

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Idc175fab3af5c6102a5a3439b712b659ecb76468

show more ...

Fix some includes

System includes should be included with <>, in-tree includes should be
included with "". This was found manually, with the help of the
following grep statement[1].

git grep -o -h

Fix some includes

System includes should be included with <>, in-tree includes should be
included with "". This was found manually, with the help of the
following grep statement[1].

git grep -o -h "#include .*" | sort | uniq

Tested:
Code compiles

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I1a6b2a5ba35ccbbb61c67b7c4b036a2d7b3a36a3

show more ...

Switched bmcweb to use new telemetry service API

Added support for multiple MetricProperties. Added support for new
parameters: CollectionTimeScope, CollectionDuration. ReadingParameters
was not yet

Switched bmcweb to use new telemetry service API

Added support for multiple MetricProperties. Added support for new
parameters: CollectionTimeScope, CollectionDuration. ReadingParameters
was not yet changed in telemetry backend, instead temporary property
ReadingParametersFutureVersion was introduced. Once bmcweb is adapted to
use ReadingParametersFutureVersion this property will be renamed in
backend to ReadingParameters. Then bmcweb will change to use
ReadingParameters. Then ReadingParametersFutureVersion will be removed
from backend and everything will be exactly like described in
phosphor-dbus-interfaces without introducing breaking changes.

Related change in phosphor-dbus-interfaces [1], [2]. This change needs
to be bumped together with [3].

Tested:
- It is possible to create MetricReportDefinitions with multiple
MetricProperties.
- Stub values for new parameters are correctly passed to telemetry
service.
- All existing telemetry service functionalities remain unchanged.

[1]: https://github.com/openbmc/phosphor-dbus-interfaces/commit/4f9c09144b60edc015291d2c120fc5b33aa0bec2
[2]: https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/60750
[3]: https://gerrit.openbmc.org/c/openbmc/telemetry/+/58229

Change-Id: I2cd17069e3ea015c8f5571c29278f1d50536272a
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Signed-off-by: Lukasz Kazmierczak <lukasz.kazmierczak@intel.com>

show more ...

clang-format: copy latest and re-format

clang-format-16 has some backwards incompatible changes that require
additional settings for best compatibility and re-running the formatter.
Copy the latest

clang-format: copy latest and re-format

clang-format-16 has some backwards incompatible changes that require
additional settings for best compatibility and re-running the formatter.
Copy the latest .clang-format from the docs repository and reformat the
repository.

Change-Id: I75f89d2959b0f1338c20d72ad669fbdc1d720835
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

show more ...

Change readJson from is_object to get_ptr

While is_object is useful in contexts where we don't need the underlying
value, in cases where we do, nlohmann::json::begin() can throw when
generating the

Change readJson from is_object to get_ptr

While is_object is useful in contexts where we don't need the underlying
value, in cases where we do, nlohmann::json::begin() can throw when
generating the nlohmann::iterator_proxy object. This causes unwind
tables to be generated, so as a general rule, we should be prefering
get_ptr in these cases.

Tested: Good unit test coverage for readJson. Unit tests only.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I5ecb2dca95cc06246e983f9e4190b2dd46d1a6b9

show more ...

clang-tidy: fix misc warnings

The following error reports have started to be reported by clang-tidy:

* readability-qualified-auto - add 'const' to `auto&` iterators
* bugprone-use-after-move -

clang-tidy: fix misc warnings

The following error reports have started to be reported by clang-tidy:

* readability-qualified-auto - add 'const' to `auto&` iterators
* bugprone-use-after-move - add break in loop after element is found

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I5314559f62f58aa032d4c74946b8e3e4ce6be808

show more ...

json utils: remove forward_declare

It was accidentally missed in previous round of iwyu clean up.

Tested: build.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I6857409848a99594d5c911

json utils: remove forward_declare

It was accidentally missed in previous round of iwyu clean up.

Tested: build.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I6857409848a99594d5c91168cf4a760e456e8c90

show more ...

test treewide: iwyu

These changes are done by running iwyu manually under clang14.

Suppressed some obvious impl or details headers. Kept the recommended
public headers.

IWYU can increase readabili

test treewide: iwyu

These changes are done by running iwyu manually under clang14.

Suppressed some obvious impl or details headers. Kept the recommended
public headers.

IWYU can increase readability, make maintenance easier, and avoid errors
in some cases. See details in
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md.

This commit also uses its best effort to correct obvious errors through
iwyu pragma.
See reference here:
https://github.com/include-what-you-use/include-what-you-use#how-to-correct-iwyu-mistakes

Tested: unit test passed.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I983b6f75601707cbb0f2f04546c3362ff4ba7fee

show more ...

Handle NTPServers list per the specification

The Redfish specification for PATCH of arrays defines a number of
requirements.
- Setting a value to null, should remove it from the list.
- Setting a va

Handle NTPServers list per the specification

The Redfish specification for PATCH of arrays defines a number of
requirements.
- Setting a value to null, should remove it from the list.
- Setting a value to empty object "{}" should leave the value unmodified
- Values at indexes larger than whats included in the PATCH request
shall be removed.

This commit attempts to fix this behavior for NTPServers and make it
correct. It does this by first getting the list of NTP servers, then
walking the list in parallel with the list given in the patch, and
either modifying or changing the list as the spec requires before
setting the setting across the system.

It also turns out that the current behavior of unpacking nlohmann::json
objects requires an object to be an array, object, or null, which
doesn't allow unpacking the strings required in this case, so that check
is removed. A quick inspection shows that we don't unpack nlohmann
objects very often, and this should have no impact.

Tested:
Redfish-protocol-validator tests for NTPServers now pass

'''
curl -vvvv --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/Managers/bmc/NetworkProtocol -X PATCH -d '{"NTP": {"NTPServers": []}}'
'''
Used to patch values succeeds with various "good" values;
["time-a-b.nist.gov", "time-b-b.nist.gov"]
[{}, {}]
["time-a-b.nist.gov", null]
[]

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I23a8febde34817bb0b934e46e2b77ff391b52a57

show more ...

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels and to use OuterScope."

bmcweb is very callback heavy code. Try to enable it and see if that
improves things. There are many cases where the length of a lambda call
will change, and reindent the entire lambda function. This is really
bad for code reviews, as it's difficult to see the lines changed. This
commit should resolve it. This does have the downside of reindenting a
lot of functions, which is unfortunate, but probably worth it in the
long run.

All changes except for the .clang-format file were made by the robot.

Tested: Code compiles, whitespace changes only.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib4aa2f1391fada981febd25b67dcdb9143827f43

show more ...

Implement odata annotations ignoring

From the quoted section of the spec in the patchset, we should be
ignoring odata annotations on PATCH requests. This commit implements a
preliminary loop throug

Implement odata annotations ignoring

From the quoted section of the spec in the patchset, we should be
ignoring odata annotations on PATCH requests. This commit implements a
preliminary loop through the json object, and removes the odata items
before processing begins.

Tested:

curl -vvvv --insecure --user root:0penBmc -X PATCH -d '{"@odata.etag":
"my_etag"}' https://192.168.7.2/redfish/v1/AccountService/Accounts/root
returns: Base.1.11.0.NoOperation

Redfish protocol validator now passes the REQ_PATCH_ODATA_PROPS test.

Included unit tests passing.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I62be75342681d147b8536fd122bbc793eeaa3788

show more ...

Enable readability checks

clang-tidy readability checks are overall a good thing, and help us to
write consistent and readable code, even if it doesn't change the
result.

All changes done by the ro

Enable readability checks

clang-tidy readability checks are overall a good thing, and help us to
write consistent and readable code, even if it doesn't change the
result.

All changes done by the robot.

Tested: Code compiles, inspection only (changes made by robot)

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iee4a0c74a11eef9f158f0044eae675ebc518b549

show more ...

json_utils: Add support for multiple level json read

Added support for multiple level direct read. For example, we can now
access `abc/xyz` directly instead of getting `abc` and then abc[`xyz`].

Fo

json_utils: Add support for multiple level json read

Added support for multiple level direct read. For example, we can now
access `abc/xyz` directly instead of getting `abc` and then abc[`xyz`].

For extra element error, it will only be triggered if the element at the
root level is not a parent of any of the requested elements.

For example,

{
"abc": {
"xyz": 12
}
}

Getting "abc/xyz" will satisfy the condition so it does not throw an
error.

This is accomplished in a reasonable way by moving the previously
variadic templated code to a std::span<variant> that contains all
possible types. This is a trick learned from the fmt library to reduce
compile sizes, as the majority of the code doesn't get duplicated at
template level, and is instead operating on the fixed variant type.

This commit drops 7316 bytes (about half a percent of total) from the
bmcweb binary size from the reduction in template usage. Later patches
build on this patchset to simplify call sites even more and reduce the
binary size further, but as is, this is still a win.

Note: now that the UnpackVariant lists all possible unpack types, it was
found that readJson would fail to compile for vector<bool>. This is a
well known C++ deficiency in the std::vector<bool> type when compared to
all other types, and will need special handling in the future. The two
types for vector<bool> are left commented out in the typelist.


Tested: Unit tests passing with reasonable coverage. Functional use in
next commit in this series.

Change-Id: Ifb247c9121c41ad8f1de26eb4bfc3d71484e6bd6
Signed-off-by: Willy Tu <wltu@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>

show more ...

json_utils: Add support jsonRead Patch/Action

Added support for readJson for Patch and Action. The only difference is
that Patch does not allow empty json input while Action does. Action with
empty

json_utils: Add support jsonRead Patch/Action

Added support for readJson for Patch and Action. The only difference is
that Patch does not allow empty json input while Action does. Action with
empty input will use the default value based on the implementation and
return 200 OK response code.

readJsonPatch will replace the existing readJson and be used for path
requests. It will not allow empty json input and all requested
keys are required in the json input.

readJsonAction will be used for Action requests where it is possible for
all of the properties to be optional and allow empty request.
The optional properties are determined by the requested values type.

All current Action readJson are replaced with readJsonAction. It does
not change the existing behavior since it needs `std::optional`.
This will have to be updated later as we define the default behavior.

Tested:
Added unit tests and readJsonAction allows empty empty json object.

No Change to Redfish Tree.

Change-Id: Ia5e1f81695c528a20f1dc985aee19c920d8adaea
Signed-off-by: Willy Tu <wltu@google.com>

show more ...

Enable readability-named-parameter checks

We don't have too many violations here, probably because we don't have
many optional parameters. Fix the existing instances, and enable the
check.

Signed-

Enable readability-named-parameter checks

We don't have too many violations here, probably because we don't have
many optional parameters. Fix the existing instances, and enable the
check.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4d512f0ec90b060fb60a42fe3cd6ba72fb6c6bcb

show more ...

Enable pointer devolution checks

Enable cpp core guidelines checks for pointer deevolution. For the
moment, simply ignore the uses, although ideally these should be cleaned
up at some point.

Signe

Enable pointer devolution checks

Enable cpp core guidelines checks for pointer deevolution. For the
moment, simply ignore the uses, although ideally these should be cleaned
up at some point.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9a8aae94cc7a59529eab89225a37e89628c17597

show more ...

Fix nlohmann::json::dump calls

The nlohmann::json::dump call needs to be called with specific arguments
to avoid throwing in failure cases. http connection already does this
properly, but a bunch o

Fix nlohmann::json::dump calls

The nlohmann::json::dump call needs to be called with specific arguments
to avoid throwing in failure cases. http connection already does this
properly, but a bunch of code has snuck in (mostly in redfish) that
ignores this, and calls it incorrectly. This can potentially lead to a
crash if the wrong thing throws on invalid UTF8 characters.

This audits the whole codebase, and replaces every dump() call with the
correct dump(2, ' ', true, nlohmann::json::error_handler_t::replace)
call. For correct output, the callers should expect no change, and in
practice, this would require injecting non-utf8 characters into the
BMC.

Tested:
Ran several of the endpoints/error conditions in question, including
some of the error cases. Observed correct responses. I don't know of a
security issue that would allow injecting invalid utf8 into the BMC, but
in theory if it were possible, this would prevent a crash.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4a15b8e260e3db129bc20484ade4ed5449f75ad0

show more ...

Omit using subscripting operator in json utils

Using operator[] as a result of expression is a common mistake in c++.
First it creates a key in container with empty value than execute an
expression

Omit using subscripting operator in json utils

Using operator[] as a result of expression is a common mistake in c++.
First it creates a key in container with empty value than execute an
expression and assign result from it to the key in container. The result
of it is that object is created two times.

Fixes https://github.com/openbmc/bmcweb/issues/139

Tested:
Used:
curl -vvv -X POST -d "{
\"EventType\": \"Alert\",
\"EventId\": \"TestEventId\",
\"EventTimestamp\": \"2017-08-08T08:24:00Z\",
\"Severity\": \"TestSeverity\",
\"Message\": \"TestMessage\",
\"MessageId\": \"TestMessageId\",
\"MessageArgs\": [ \"TestMessageArg\" ],
\"OriginOfCondition\": \"/redfish/v1/\"
}" --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/EventService/Actions/EventService.SubmitTestEven

To send test event. Call returned 204 as expected.

Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Idf44829bfb25daf216003f591d354df65ccecb18

show more ...

fix include names

cppcheck isn't smart enough to recognize these are c++ headers, not c
headers. Considering we're already inconsistent about our naming, it's
easier to just be consistent, and move

fix include names

cppcheck isn't smart enough to recognize these are c++ headers, not c
headers. Considering we're already inconsistent about our naming, it's
easier to just be consistent, and move the last few files to use .hpp
instead of .h.

Tested:
Code builds, no changes.

Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Ic348d695f8527fa4a0ded53f433e1558c319db40

show more ...