Remove usage of Option::unwrap() in activitypub code #80

nutomic · 2020-08-10T15:25:48Z

nutomic commented

2020-08-10 15:25:48 +00:00

This removes most usage of Option::unwrap() from activitypub code, and replaces it with context(location_info!())?. If a none value is accessed, it will now throw an error like this:

Error: LemmyError { inner: None value at src/main.rs:63, column 16 }

TODO:

replace remaining usages of unwrap() (which are a bit more complicated)
maybe replace usages of unwrap() outside of activitypub code

This removes most usage of `Option::unwrap()` from activitypub code, and replaces it with `context(location_info!())?`. If a none value is accessed, it will now throw an error like this: ``` Error: LemmyError { inner: None value at src/main.rs:63, column 16 } ``` TODO: - replace remaining usages of unwrap() (which are a bit more complicated) - maybe replace usages of unwrap() outside of activitypub code

nutomic changed title from ~~WIP: Remove usage of Option::unwrap() in activitypub code~~ to Remove usage of Option::unwrap() in activitypub code

2020-08-10 18:14:51 +00:00

nutomic commented

2020-08-10 18:17:29 +00:00

I removed all remaining usage of unwrap() in the apub code so this is good to merge. I also looked into doing the same in other code, but unwrap usages there seem more complicated, so I decided not to touch that.

Note that I came across two ways we can easily simplify the code:

make public_key a string field in the db, instead of an option
storing shared_inbox_url in CommunityFollowerView

Getting the unwrap was really much easier than I thought.

I removed all remaining usage of unwrap() in the apub code so this is good to merge. I also looked into doing the same in other code, but unwrap usages there seem more complicated, so I decided not to touch that. Note that I came across two ways we can easily simplify the code: - make public_key a string field in the db, instead of an option - storing shared_inbox_url in CommunityFollowerView Getting the unwrap was really much easier than I thought.

dessalines reviewed 2020-08-11 00:16:13 +00:00

server/src/apub/mod.rs Outdated

						
				@ -345,0 +349,4 @@

				      PublicKey {

				        id: format!("{}#main-key", self.actor_id_str()),

				        owner: self.actor_id_str(),

				        public_key_pem: self.public_key().context(location_info!())?,

dessalines commented

2020-08-11 00:16:13 +00:00

Nice, this is a pretty easy way to do this.

dessalines reviewed 2020-08-11 00:18:03 +00:00

server/src/apub/mod.rs Outdated

						
				@ -320,0 +317,4 @@

				    let actor_id = self.actor_id()?;

				    let url = format!(

				      "{}://{}{}/inbox",

				      &actor_id.scheme(),

dessalines commented

2020-08-11 00:18:03 +00:00

Cool, so we do get some url functionality out of this.

nutomic commented

2020-08-11 11:36:14 +00:00

What do you mean get some url functionality?

dessalines reviewed 2020-08-11 00:23:31 +00:00

server/src/apub/post.rs Outdated

						
				@ -147,6 +148,37 @@ impl ToApub for Post {

				  }

				}

				type EmbedType = (Option<String>, Option<String>, Option<String>);

dessalines commented

2020-08-11 00:23:31 +00:00

Is it possible to do this as a struct?

struct EmbedType {
  name: Option...
  summary:...
  content:...

Its a preference thing, but I really don't like tuple structs, and want to eliminate any others if we have them. You have to guess what that 2nd or 3rd item is, whereas with the regular structs, its a named thing so you know what it is.

Is it possible to do this as a struct? ``` struct EmbedType { name: Option... summary:... content:... ``` Its a preference thing, but I really don't like tuple structs, and want to eliminate any others if we have them. You have to guess what that 2nd or 3rd item is, whereas with the regular structs, its a named thing so you know what it is.

nutomic commented

2020-08-11 11:36:48 +00:00

Sure I can do that, just added the typedef cause clippy was complaining.