Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LogViewSet: Accessing request.body in create() and timestamp_str in parse_log() #34

Closed
gonmachado opened this issue Jul 8, 2024 · 3 comments

Comments

@gonmachado
Copy link
Contributor

Hello again!

Has anyone encountered issues with the LogViewSet in handling JSON data and parsing timestamps with AM/PM format? I've encountered the following issues and would like to gather more insights or solutions if available.

The LogViewSet attempts to handle incoming JSON data, but i encounter some issues with both accessing request.body and parsing timestamps that include AM/PM format.

Testing Scenario: Testing of the LogViewSet endpoint.

  • "What changed?" Response from Apple
    • Attempted to force a pass update without actually making changes to the pass instance (This was executed using pass_instance.push_notification() on a pass instance with no modified data.) triggering Apple to POST to the log endpoint. The message was "What changed? ........" since the pass had no modifications since the last updated_at.
  1. Accessing request.body:

    The LogViewSet's create method uses json.loads(request.body) to parse incoming JSON data. However, this lead me to a RawPostDataException because request.body can only be read once in Django's request lifecycle.

    • Note: I've noticed a difference in how request.body is handled between different views within the library. For instance, the RegisterPassViewSet successfully uses request.body to parse JSON data without issues. However, in the LogViewSet, direct access to request.body leads to a RawPostDataException. Am i the only one experiencing this error?
  2. Parsing Timestamps with AM/PM Format:

    Another issue arises in Log.parse_log(log, message), specifically with parsing timestamps provided by Apple servers that include AM/PM format (%p). The current implementation of datetime.datetime.strptime(timestamp_str, "%Y-%m-%d %H:%M:%S %z") does not account for %p, leading to parsing errors when timestamps contain AM/PM indicators.

Proposed Solutions:

  1. Handling request.body:

    Refactor LogViewSet.create to utilize DRF's request parsing mechanisms (request.data) instead of directly accessing request.body. This approach ensures compatibility with Django's request handling and avoids RawPostDataException.

    class LogViewSet(viewsets.ViewSet):
        """
        Logs messages from devices
        """
        permission_classes = (AllowAny, )
    
        def create(self, request):
            for message in request.data.get('logs', []):  # Use DRF's request parsing
                log = Log(message=message)
                Log.parse_log(log, message)
            return Response({}, status=status.HTTP_200_OK)
  2. Parsing Timestamps:

    Modify the timestamp parsing logic in Log.parse_log to include %p in the format string ("%Y-%m-%d %H:%M:%S %p %z") when using datetime.datetime.strptime. This adjustment ensures correct parsing of timestamps that include AM/PM indicators.

      class Log(models.Model):
          # model fields
    
          @classmethod
          def parse_log(cls, log, message):
              # method code
              log.created_at = datetime.datetime.strptime(timestamp_str, "%Y-%m-%d %H:%M:%S %p %z")  # Include %p to parse AM/PM

Here's a screenshot showing the request from Apple with the data in request.data and the message containing the AM/PM formatted datetime.
error_1

@patroqueeet
Copy link
Collaborator

lgtm. can you send a PR w/ tests?

@GonzaloMachado
Copy link
Contributor

Hi @patroqueeet

Sorry for the account mix-ups earlier.

I've submitted a pull request that includes the necessary tests.

@alexandernst
Copy link
Member

Closed by #35

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

No branches or pull requests

4 participants